Project

General

Profile

Actions
Copy link

Defect #38992

open

How do I use helper_queries_column_value helper hook?

Added by keineahnung 2345 about 3 years ago. Updated about 20 hours ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Resolution:
Affected version:

Description

In app/helpers/queries_helper.rb, there is:

  def column_value(column, item, value)
    content = ...

    call_hook(:helper_queries_column_value,
              {:content => content, :column => column, :item => item, :value => value})

    content
  end

In my xxx_helper_queries_column_value_hook.rb:

  def helper_queries_column_value(context = { })
    if context[:column].name ==  :action
      context[:content] = "Ushiro" 
    end
  end

I can change the parameter context, but when I checked in app/helpers/queries_helper.rb, I found that the variable content is not updated. So what's the proper way of using this hook? My guess is that this piece of code:

    call_hook(:helper_queries_column_value,
              {:content => content, :column => column, :item => item, :value => value})

    content

should be revised to:

    content = call_hook(:helper_queries_column_value,
              {:content => content, :column => column, :item => item, :value => value})

    content
Actions
Copy link
 #1

Updated by Like HOPPER over 2 years ago

Indeed, what is this hook for? And how should we use it? Does it allow to modify the display of an attribute in a list?

Actions
Copy link
 #2

Updated by Florian Walchshofer about 20 hours ago

The hook is not meant to be used by reassigning context[:content]. call_hook merges your hash into a new one (default_context.merge(context)), so context[:content] = "..." only changes a key in that copy. The local variable content in column_value is never touched.

However, the same String object is passed along, so the intended way is to modify it in place:

class MyPluginHookListener < Redmine::Hook::ViewListener
  def helper_queries_column_value(context = {})
    return unless context[:column].name == :action

    content = context[:content]
    return unless content.is_a?(String) && !content.frozen?

    # content is usually an ActiveSupport::SafeBuffer (html_safe),
    # so escape anything that is not already safe HTML
    content.replace(ERB::Util.html_escape('Ushiro'))
  end
end

Please note that the proposed change

content = call_hook(:helper_queries_column_value, {...})

would break things: in a view/helper context, call_hook returns the return values of all listeners joined as a string. With no plugin registered, this is "", so every column in every list would be empty.

The same pattern applies to helper_queries_column_content (both hooks were introduced in #34743).

Things to keep in mind:
  • content can be nil, a non-String, or frozen depending on the column. Check before calling replace.
  • replace keeps the html_safe flag of the SafeBuffer. Always escape user data (ERB::Util.html_escape / h), otherwise this opens an XSS hole.

Maybe this should be documented in Hooks, and the status could be changed to "Resolved" / "Invalid", since this is more of a usage question than a defect.

Actions
Copy link

Also available in: Atom PDF