https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292009-01-19T21:23:58ZRedmineRedmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=67752009-01-19T21:23:58ZDouglas Manley
<ul></ul><p>It would be sweet if something like this could be done so that we don't need (as plugin developers) to access the "request" variable.<br /><pre>
include ActionController::UrlWriter
default_url_options[:host] = request.env[ 'SERVER_NAME' ]
default_url_options[:port] = request.env[ 'SERVER_PORT' ]
</pre></p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=67942009-01-20T18:01:15ZEric Davis
<ul></ul><p>Douglas Manley wrote:</p>
<blockquote>
<p>It would be sweet if something like this could be done so that we don't need (as plugin developers) to access the "request" variable.</p>
</blockquote>
<p>That would solve the <code>host</code> and <code>port</code> issue (would also need <code>protocol</code>) but <code>request</code> has a lot of other useful information. For example, a plugin might want to sniff the User Agent and resent a mobile view.</p>
<p>I'd also love to be able to use <code>render</code> in a hook but I know it's not trivial. Putting "view" code in a class as a string feels so dirty to me.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=68382009-01-23T08:50:53ZThomas Löber
<ul></ul><p>This is how I would change <code>hook.rb</code>:</p>
<pre>
module Helper
def call_hook(hook, context={})
Redmine::Hook.call_hook(hook, {:project => @project, :request => request}.merge(context))
end
end
</pre>
<p>And Redmine::Hook.call_hook:<br /><pre>
# Calls a hook.
# Returns the listeners response.
def call_hook(hook, context={})
returning "" do |response|
hls = hook_listeners(hook)
if hls.any?
request = context[:request]
if request
default_url_options[:host] = request.env["SERVER_NAME"]
default_url_options[:port] = request.env["SERVER_PORT"]
end
hls.each do |listener|
response << listener.send(hook, context).to_s
end
end
end
end
</pre></p>
<p>This works for me at least. I can use "link_to" in a hook listener and I can query context[:response].</p>
<p>What do you think?</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=68412009-01-23T12:28:36ZThomas Löber
<ul></ul><p>Eric Davis wrote:</p>
<blockquote>
<p>I'd also love to be able to use <code>render</code> in a hook but I know it's not trivial. Putting "view" code in a class as a string feels so dirty to me.</p>
</blockquote>
<p>If the <code>call_hook</code> method is extended like this:<br /><pre>
module Helper
def call_hook(hook, context={})
ctx_init = {:project => @project, :controller => controller, :request => request}
Redmine::Hook.call_hook(hook, ctx_init.merge(context))
end
end
</pre></p>
<p>you can use <code>render_to_string</code> in your hook listener method:<br /><pre>
def view_issues_show_details_bottom(context)
controller = context[:controller]
issue = context[:issue]
controller.send(:render_to_string, :partial => "show_more_data", :locals => {:issue => issue})
end
</pre></p>
<p>It works, but I wonder if this is the correct way to do it. At least the HTML is in a (partial) view again.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=68492009-01-23T15:47:13ZThomas Löber
<ul></ul><p>Actually the above method has to be:<br /><pre>
module Helper
def call_hook(hook, context={})
ctx_init = {:project => @project, :request => request}
ctx_init[:controller] = self.is_a?(ActionController::Base) ? self : controller
Redmine::Hook.call_hook(hook, ctx_init.merge(context))
end
end
</pre><br />Then it will work for controller and view hooks.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=68622009-01-24T02:41:40ZEric Davis
<ul><li><strong>Assignee</strong> set to <i>Eric Davis</i></li></ul><p>Thomas thanks for the suggestions, I'll see what I can do.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=70372009-01-31T23:07:25ZThomas Löber
<ul></ul><p>This is how we can make calling <code>render_to_string</code> more straightforward:</p>
<p>In hook.rb<br /><pre>
class ViewListener < Listener
[...]
def self.render_on(hook, options={})
define_method hook do |context|
context[:controller].send(:render_to_string, {:locals => context}.merge(options))
end
end
end
</pre></p>
<p>Then we can define a hook that renders its result using a partial view like this:<br /><pre>
class MyHook < Redmine::Hook::ViewListener
render_on :view_issues_show_details_bottom, :partial => "show_more_data"
end
</pre></p>
<p>The partial view will have access to <code>issue</code> because it is given as a parameter in the <code>call_hook</code> call.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=70522009-02-02T09:00:47ZThomas Löber
<ul><li><strong>File</strong> <a href="/attachments/1478">hook.rb.diff</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/1478/hook.rb.diff">hook.rb.diff</a> added</li></ul><p>This is the complete patch.</p>
<p>I split the "call_hook" helper into two methods. One for controllers and one for views. The controller method returns an array of results (which is more appropriate if I want to handle the result inside a controller). The view method uses "join" to convert the array into a string.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=70542009-02-02T10:58:06ZThomas Löber
<ul><li><strong>File</strong> <a href="/attachments/1480">hook.rb.revised.diff</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/1480/hook.rb.revised.diff">hook.rb.revised.diff</a> added</li></ul><p>As I just saw it was a bad idea to split the "call_hook" helper. It doesn't work if a controller includes the ApplicationHelper.</p>
<p>So here's a revised patch.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=72262009-02-10T02:09:26ZEric Davis
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>7</i></li><li><strong>Target version</strong> set to <i>0.8.1</i></li></ul><p>Thomas Löber wrote:</p>
<blockquote>
<p>As I just saw it was a bad idea to split the "call_hook" helper. It doesn't work if a controller includes the ApplicationHelper.</p>
<p>So here's a revised patch.</p>
</blockquote>
<p>I'd like to apply this but I'm having a hard time testing it. Could you provide some tests for the different paths of:</p>
<ul>
<li><code>Redmine::Hook.call_hook</code></li>
<li><code>Redmine::Hook::Helper.call_hook</code></li>
<li><code>Redmine::Hook::ViewListener.render_on</code></li>
</ul> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=72272009-02-10T03:15:47ZEric Davis
<ul><li><strong>% Done</strong> changed from <i>0</i> to <i>80</i></li></ul><p>I found a way to test a few of the methods and committed your patch with some modifications in <a class="changeset" title="Added request and controller objects to the hooks by default. The request and controller objects..." href="https://www.redmine.org/projects/redmine/repository/svn/revisions/2429">r2429</a> and <a class="changeset" title="Renamed variables to be more descriptive. #2542" href="https://www.redmine.org/projects/redmine/repository/svn/revisions/2430">r2430</a>. I'd still like to add some tests for <code>Redmine::Hook::Helper.call_hook</code> and <code>Redmine::Hook::ViewListener.render_on</code> before closing this issue. You can find the tests at the bottom of <code>test/unit/lib/redmine/hook_test.rb</code>.</p>
<a name="Commit-details"></a>
<h3 >Commit details<a href="#Commit-details" class="wiki-anchor">¶</a></h3>
<p>Added request and controller objects to the hooks by default.</p>
<p>The request and controller objects are now added to all hook contexts by default. This will also make url_for work better in hooks by setting up the default_url_options :host, :port, and :protocol.</p>
<p>Finally a new helper method <code>render_or</code> has been added to ViewListener. This will let a hook easily render a partial without a full method definition.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=73382009-02-15T08:40:56ZJean-Philippe Langjp_lang@yahoo.fr
<ul><li><strong>Target version</strong> deleted (<del><i>0.8.1</i></del>)</li></ul><p>See <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: Setting up the default_url_options :port in hook will duplicate with the host parameter(which alr... (Closed)" href="https://www.redmine.org/issues/2754">#2754</a></p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75092009-02-21T00:29:25ZEric Davis
<ul><li><strong>Status</strong> changed from <i>7</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>80</i> to <i>100</i></li></ul><p>This should be finished now. Instead of using the request and setting host, port, and protocol directly I made the <code>default_url_options</code> use <code>:only_path</code>. This was a much cleaner implementation and should work for the majority of the plugin cases.</p>
<p>Any non standard case (like Douglas Manley) can set the host, port, and protocol in their plugin itself using the <code>request</code> context that's passed in:</p>
<pre><code class="ruby syntaxhl"><span class="n">context</span><span class="p">[</span><span class="ss">:request</span><span class="p">].</span><span class="nf">env</span><span class="p">[</span><span class="s2">"SERVER_NAME"</span><span class="p">]</span>
<span class="n">context</span><span class="p">[</span><span class="ss">:request</span><span class="p">].</span><span class="nf">env</span><span class="p">[</span><span class="s2">"SERVER_PORT"</span><span class="p">]</span>
</pre>
<p>A big thanks goes to <a href="http://github.com/edavis10/redmine/commit/5b7a5c39a7da667b1a2718d166a80f1f0ae4d434#comments" class="external">Peter Suschlik</a> for the idea.</p>
<p>Implemented in r2489 through r2491</p></code></pre> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75102009-02-21T00:30:29ZEric Davis
<ul><li><strong>Target version</strong> set to <i>0.8.2</i></li></ul><p>Adding to 0.8.2, since I couldn't reproduce <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: Setting up the default_url_options :port in hook will duplicate with the host parameter(which alr... (Closed)" href="https://www.redmine.org/issues/2754">#2754</a> and I don't think <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: Setting up the default_url_options :port in hook will duplicate with the host parameter(which alr... (Closed)" href="https://www.redmine.org/issues/2754">#2754</a> would be valid anymore after my refactoring.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75132009-02-21T07:42:33ZPeter Suschlik
<ul></ul><p>Great, thank you!</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75432009-02-21T21:48:30ZDouglas Manley
<ul></ul><p>This should work just fine; thanks!</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75452009-02-22T04:10:29ZChaoqun Zou
<ul></ul><code class="ruby syntaxhl"><span class="n">default_url_options</span><span class="p">[</span><span class="ss">:only_path</span><span class="p">]</span> <span class="o">||=</span> <span class="kp">true</span>
</code>
<p>This will make the default_url_options[:only_path] to be true permanently. So the link in the notification mail will lost it's protocal, host and port.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75462009-02-22T04:14:53ZEric Davis
<ul></ul><p>Chaoqun Zou wrote:</p>
<blockquote>
<code class="ruby syntaxhl"><span class="o">></span> <span class="n">default_url_options</span><span class="p">[</span><span class="ss">:only_path</span><span class="p">]</span> <span class="o">||=</span> <span class="kp">true</span>
<span class="o">></span> </code>
<p>This will make the default_url_options[:only_path] to be true permanently. So the link in the notification mail will lost it's protocal, host and port.</p>
</blockquote>
<p>Can you provide a test case showing the incorrect behavior? The email urls were being generated correctly for me (see <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: Setting up the default_url_options :port in hook will duplicate with the host parameter(which alr... (Closed)" href="https://www.redmine.org/issues/2754">#2754</a>).</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=75472009-02-22T07:32:33ZChaoqun Zou
<ul><li><strong>File</strong> <a href="/attachments/1563">hook_test_diff_for_link_test.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/1563/hook_test_diff_for_link_test.patch">hook_test_diff_for_link_test.patch</a> added</li></ul><p>Here is a test (in hook_test unit test) for :only_path :</p>
<pre>
<code class="ruby syntaxhl"> <span class="n">fixtures</span> <span class="ss">:issues</span>
<span class="k">def</span> <span class="nf">test_call_hook_will_break_issue_link_in_mail</span>
<span class="n">issue</span> <span class="o">=</span> <span class="no">Issue</span><span class="p">.</span><span class="nf">find</span><span class="p">(</span><span class="mi">1</span><span class="p">)</span>
<span class="no">ActionMailer</span><span class="o">::</span><span class="no">Base</span><span class="p">.</span><span class="nf">deliveries</span><span class="p">.</span><span class="nf">clear</span>
<span class="no">Mailer</span><span class="p">.</span><span class="nf">deliver_issue_add</span><span class="p">(</span><span class="n">issue</span><span class="p">)</span>
<span class="n">mail</span> <span class="o">=</span> <span class="no">ActionMailer</span><span class="o">::</span><span class="no">Base</span><span class="p">.</span><span class="nf">deliveries</span><span class="p">.</span><span class="nf">last</span>
<span class="nb">puts</span> <span class="n">mail</span><span class="p">.</span><span class="nf">body</span>
<span class="vi">@hook_module</span><span class="p">.</span><span class="nf">add_listener</span><span class="p">(</span><span class="no">TestLinkToHook</span><span class="p">)</span>
<span class="vi">@hook_helper</span><span class="p">.</span><span class="nf">call_hook</span><span class="p">(</span><span class="ss">:view_layouts_base_html_head</span><span class="p">)</span>
<span class="no">ActionMailer</span><span class="o">::</span><span class="no">Base</span><span class="p">.</span><span class="nf">deliveries</span><span class="p">.</span><span class="nf">clear</span>
<span class="no">Mailer</span><span class="p">.</span><span class="nf">deliver_issue_add</span><span class="p">(</span><span class="n">issue</span><span class="p">)</span>
<span class="n">mail2</span> <span class="o">=</span> <span class="no">ActionMailer</span><span class="o">::</span><span class="no">Base</span><span class="p">.</span><span class="nf">deliveries</span><span class="p">.</span><span class="nf">last</span>
<span class="nb">puts</span> <span class="n">mail2</span><span class="p">.</span><span class="nf">body</span>
<span class="n">assert_equal</span> <span class="n">mail</span><span class="p">.</span><span class="nf">body</span><span class="p">,</span> <span class="n">mail2</span><span class="p">.</span><span class="nf">body</span>
<span class="k">end</span>
</code><br /></pre>
<p>This test will fail before the line 11-12 be commented out.</p>
<p>A diff patch file is attatched.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=76222009-02-25T07:24:52ZEric Davis
<ul></ul><p>I believe I fixed the Plugin and Mailer default_url_options in <a class="changeset" title="Fixing Plugin and Mailer default_url_options. Both the plugin hooks and Mailer were setting defa..." href="https://www.redmine.org/projects/redmine/repository/svn/revisions/2522">r2522</a>. It turned out to be a tricky bug that was caused by using <code>default_url_options</code> incorrectly.</p>
<p>Both the plugin hooks and Mailer were setting <code>default_url_options</code> incorrectly and causing <code>ActionContoller::UrlWritter</code> to cache the settings on the module (<code>mattr_accessor</code>) causing several url generators to fail in either the plugin hooks or the Mailer. I found a good description of why this was happening on <a href="http://stackoverflow.com/questions/185573/what-is-mattraccessor-in-a-rails-module/188915#188915" class="external">Stackoverflow</a>.</p>
<p>Thanks for reporting the bug and providing the test case, it helped debugging.</p> Redmine - Feature #2542: Plugin hooks should have access to the "request" variablehttps://www.redmine.org/issues/2542?journal_id=78652009-03-07T11:41:29ZJean-Philippe Langjp_lang@yahoo.fr
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Merged in 0.8-stable in <a class="changeset" title="Backported r2429, r2430, r248 to r2491 and r2522 from trunk (request and controller objects added..." href="https://www.redmine.org/projects/redmine/repository/svn/revisions/2558">r2558</a>.</p>