Project

General

Profile

Actions

Defect #10251

closed

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

Added by Etienne Massip almost 13 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Email notifications
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
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.


Files

description_diff_in_email_notification.PNG (14.8 KB) description_diff_in_email_notification.PNG Etienne Massip, 2012-02-16 09:25
10251_1.diff (670 Bytes) 10251_1.diff Sridhar P, 2012-02-21 14:36

Related issues

Related to Redmine - Feature #6386: Issue mail should render the HTML version of the issue detailsClosed2010-09-14

Actions
Actions #1

Updated by Etienne Massip almost 13 years ago

It should be either not displayed or absolute.

Actions #2

Updated by Sridhar P almost 13 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.

Actions #3

Updated by Sridhar P almost 13 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.

Actions #4

Updated by Sridhar P almost 13 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.

Actions #5

Updated by Etienne Massip almost 13 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
Actions #6

Updated by Etienne Massip almost 13 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.

Actions #7

Updated by Jean-Philippe Lang almost 13 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).

Actions #8

Updated by Etienne Massip almost 13 years ago

Yes they are?

Actions #9

Updated by Jean-Philippe Lang almost 13 years ago

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

Actions #10

Updated by Etienne Massip almost 13 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?

Actions

Also available in: Atom PDF