Defect #10251

Description diff link in note details is relative when received by email

Added by Etienne Massip almost 6 years ago. Updated over 5 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Email notifications
Target version:1.4.0
Resolution:Fixed Affected version:

Description

In the HTML email notification received after an issue description has been updated, the "diff" link is present and relative so clicking on it leads to some http://journals/diff/<...>?detail_id=<...> URL.

description_diff_in_email_notification.PNG (14.8 KB) Etienne Massip, 2012-02-16 09:25

10251_1.diff Magnifier (670 Bytes) Sridhar P, 2012-02-21 14:36


Related issues

Related to Redmine - Feature #6386: Issue mail should render the HTML version of the issue de... Closed 2010-09-14

Associated revisions

Revision 9022
Added by Etienne Massip over 5 years ago

Override #url_for in AM to force generation of absolute links (#10251).

Revision 9023
Added by Jean-Philippe Lang over 5 years ago

Mailer#url_for not called in views with Rails 3.1.

History

#1 Updated by Etienne Massip almost 6 years ago

It should be either not displayed or absolute.

#2 Updated by Sridhar P almost 6 years ago

Apparently the default_url_options of app/models/mailer.rb is not being picked up or fired when link_to is called in the app/helpers/issues_helper.rb.
As a temporary hack we can provide the host and protocol options to link_to. May be there is an elegant solution that I am not aware of.

#3 Updated by Sridhar P over 5 years ago

When I set the ":only_path => false" in the app/models/mailer.rb > def self.default_url_options, it remains so up until line 76 of app/models/mailer.rb. By the time the flow reaches to app/helpers/issues_helper.rb for the diff link, the "only_path" option is "true". Not sure where or how it is being changed.

I am submitting the patch with :only_path => false for the diff_link in the issues_helper.rb. This works for me.

#4 Updated by Sridhar P over 5 years ago

I think I got hold of the reason. The link_to method called from the issues_helper is the one defined in actionpack-2.3.14/lib/action_view/helpers/url_helper.rb
The link_to invokes the url_for and code in question that is causing the trouble is here:
ActionView::Helpers::UrlHelper - lines 83-85

83 options = { :only_path => options[:host].nil? }.update(options.symbolize_keys)
84 escape = options.key?(:escape) ? options.delete(:escape) : true
85 @controller.send(:url_for, options)

We are not passing the :host parameter in the options from the show_detail function of issues_helper, so the line 83 effective sets the
options[:only_path] to true.
On line 85, the @controller.send(:url_for, options) is called which will call the url_for defined in the ActionController::UrlWriter with options[:only_path] = true and hence the host and protocol options(being set in the app/models/mailer.rb) are being ignored.

#5 Updated by Etienne Massip over 5 years ago

Yes, I saw that.

I think that this is an issue of RoR #link_to not taking default url_options into account and wanted to have a look to Rails code to see if this has been fixed.

A simple fix would be to override #url_for in Mailer:

def url_for(options={})
 options[:only_path] = false
 super options
end

#6 Updated by Etienne Massip over 5 years ago

  • Status changed from Confirmed to Resolved
  • Target version changed from Candidate for next minor release to 1.3.2
  • Resolution set to Fixed

So: fixed in a RoR3 incompatible way with r9022, fixed another way by JPL with r9023.

#7 Updated by Jean-Philippe Lang over 5 years ago

  • Status changed from Resolved to Closed
  • Target version changed from 1.3.2 to 1.4.0
  • Affected version (unused) changed from 1.3.1 to devel
  • Affected version deleted (1.3.1)

1.3-stable is not affected (links are not displayed).

#8 Updated by Etienne Massip over 5 years ago

Yes they are?

#9 Updated by Jean-Philippe Lang over 5 years ago

HTML is disabled when displaying change details: source:/tags/1.3.1/app/views/mailer/issue_edit.html.erb

#10 Updated by Etienne Massip over 5 years ago

Indeed, it was changed in devel with r8721, I thought it was a 1.3 issue.

So maybe it would have been simpler and even more consistent to set back the no_html to true?

Also available in: Atom PDF