From e5daf25618e017e50293a77ea33ddc51cf1603b3 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 17 Feb 2008 14:17:20 +0000 Subject: [PATCH] Fixes: * email notifications: host name is missing in generated links (#639, #201) * email notifications: referenced changesets, wiki pages, attachments... are not turned into links (only ticket ids are) * attachment links and inline images don't work in issue notes git-svn-id: http://redmine.rubyforge.org/svn/trunk@1161 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/helpers/application_helper.rb | 37 ++++++++++++------- app/models/journal.rb | 9 ++++- app/views/mailer/_issue_text_html.rhtml | 2 +- .../mailer/document_added.text.html.rhtml | 2 +- app/views/mailer/issue_edit.text.html.rhtml | 2 +- .../mailer/message_posted.text.html.rhtml | 2 +- app/views/mailer/news_added.text.html.rhtml | 2 +- test/fixtures/journals.yml | 2 +- test/unit/mailer_test.rb | 21 ++++++++++- 9 files changed, 58 insertions(+), 21 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b66851eef..e39ea0906 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -178,16 +178,20 @@ module ApplicationHelper case args.size when 1 obj = nil - text = args.shift || '' + text = args.shift when 2 obj = args.shift - text = obj.send(args.shift) + text = obj.send(args.shift).to_s else raise ArgumentError, 'invalid arguments to textilizable' end + return '' if text.blank? + + only_path = options.delete(:only_path) == false ? false : true # when using an image link, try to use an attachment, if possible - attachments = options[:attachments] + attachments = options[:attachments] || (obj && obj.respond_to?(:attachments) ? obj.attachments : nil) + if attachments text = text.gsub(/!((\<|\=|\>)?(\([^\)]+\))?(\[[^\]]+\])?(\{[^\}]+\})?)(\S+\.(gif|jpg|jpeg|png))!/) do |m| style = $1 @@ -195,7 +199,7 @@ module ApplicationHelper rf = Regexp.new(filename, Regexp::IGNORECASE) # search for the picture in attachments if found = attachments.detect { |att| att.filename =~ rf } - image_url = url_for :controller => 'attachments', :action => 'download', :id => found.id + image_url = url_for :only_path => only_path, :controller => 'attachments', :action => 'download', :id => found.id "!#{style}#{image_url}!" else "!#{style}#{filename}!" @@ -216,10 +220,10 @@ module ApplicationHelper # used for single-file wiki export format_wiki_link = Proc.new {|project, title| "##{title}" } else - format_wiki_link = Proc.new {|project, title| url_for :controller => 'wiki', :action => 'index', :id => project, :page => title } + format_wiki_link = Proc.new {|project, title| url_for(:only_path => only_path, :controller => 'wiki', :action => 'index', :id => project, :page => title) } end - project = options[:project] || @project + project = options[:project] || @project || (obj && obj.respond_to?(:project) ? obj.project : nil) # Wiki links # @@ -278,7 +282,8 @@ module ApplicationHelper if esc.nil? if prefix.nil? && sep == 'r' if project && (changeset = project.changesets.find_by_revision(oid)) - link = link_to("r#{oid}", {:controller => 'repositories', :action => 'revision', :id => project.id, :rev => oid}, :class => 'changeset', + link = link_to("r#{oid}", {:only_path => only_path, :controller => 'repositories', :action => 'revision', :id => project.id, :rev => oid}, + :class => 'changeset', :title => truncate(changeset.comments, 100)) end elsif sep == '#' @@ -286,17 +291,20 @@ module ApplicationHelper case prefix when nil if issue = Issue.find_by_id(oid, :include => [:project, :status], :conditions => Project.visible_by(User.current)) - link = link_to("##{oid}", {:controller => 'issues', :action => 'show', :id => oid}, :class => 'issue', + link = link_to("##{oid}", {:only_path => only_path, :controller => 'issues', :action => 'show', :id => oid}, + :class => 'issue', :title => "#{truncate(issue.subject, 100)} (#{issue.status.name})") link = content_tag('del', link) if issue.closed? end when 'document' if document = Document.find_by_id(oid, :include => [:project], :conditions => Project.visible_by(User.current)) - link = link_to h(document.title), {:controller => 'documents', :action => 'show', :id => document}, :class => 'document' + link = link_to h(document.title), {:only_path => only_path, :controller => 'documents', :action => 'show', :id => document}, + :class => 'document' end when 'version' if version = Version.find_by_id(oid, :include => [:project], :conditions => Project.visible_by(User.current)) - link = link_to h(version.name), {:controller => 'versions', :action => 'show', :id => version}, :class => 'version' + link = link_to h(version.name), {:only_path => only_path, :controller => 'versions', :action => 'show', :id => version}, + :class => 'version' end end elsif sep == ':' @@ -305,15 +313,18 @@ module ApplicationHelper case prefix when 'document' if project && document = project.documents.find_by_title(name) - link = link_to h(document.title), {:controller => 'documents', :action => 'show', :id => document}, :class => 'document' + link = link_to h(document.title), {:only_path => only_path, :controller => 'documents', :action => 'show', :id => document}, + :class => 'document' end when 'version' if project && version = project.versions.find_by_name(name) - link = link_to h(version.name), {:controller => 'versions', :action => 'show', :id => version}, :class => 'version' + link = link_to h(version.name), {:only_path => only_path, :controller => 'versions', :action => 'show', :id => version}, + :class => 'version' end when 'attachment' if attachments && attachment = attachments.detect {|a| a.filename == name } - link = link_to h(attachment.filename), {:controller => 'attachments', :action => 'download', :id => attachment}, :class => 'attachment' + link = link_to h(attachment.filename), {:only_path => only_path, :controller => 'attachments', :action => 'download', :id => attachment}, + :class => 'attachment' end end end diff --git a/app/models/journal.rb b/app/models/journal.rb index 5aebbb7ce..d757ef90d 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -51,7 +51,14 @@ class Journal < ActiveRecord::Base end def editable_by?(usr) - project = journalized.project usr && usr.logged? && (usr.allowed_to?(:edit_issue_notes, project) || (self.user == usr && usr.allowed_to?(:edit_own_issue_notes, project))) end + + def project + journalized.respond_to?(:project) ? journalized.project : nil + end + + def attachments + journalized.respond_to?(:attachments) ? journalized.attachments : nil + end end diff --git a/app/views/mailer/_issue_text_html.rhtml b/app/views/mailer/_issue_text_html.rhtml index a3eb05b01..d0f247812 100644 --- a/app/views/mailer/_issue_text_html.rhtml +++ b/app/views/mailer/_issue_text_html.rhtml @@ -12,4 +12,4 @@ <% end %> -<%= textilizable(issue.description) %> +<%= textilizable(issue, :description, :only_path => false) %> diff --git a/app/views/mailer/document_added.text.html.rhtml b/app/views/mailer/document_added.text.html.rhtml index 2ef63012b..dc1f659a0 100644 --- a/app/views/mailer/document_added.text.html.rhtml +++ b/app/views/mailer/document_added.text.html.rhtml @@ -1,3 +1,3 @@ <%= link_to @document.title, @document_url %> (<%= @document.category.name %>)

-<%= textilizable(@document.description) %> +<%= textilizable(@document, :description, :only_path => false) %> diff --git a/app/views/mailer/issue_edit.text.html.rhtml b/app/views/mailer/issue_edit.text.html.rhtml index 7ce1f47c4..48affaf77 100644 --- a/app/views/mailer/issue_edit.text.html.rhtml +++ b/app/views/mailer/issue_edit.text.html.rhtml @@ -6,6 +6,6 @@ <% end %> -<%= textilizable(@journal.notes) %> +<%= textilizable(@journal, :notes, :only_path => false) %>
<%= render :partial => "issue_text_html", :locals => { :issue => @issue, :issue_url => @issue_url } %> diff --git a/app/views/mailer/message_posted.text.html.rhtml b/app/views/mailer/message_posted.text.html.rhtml index 837272c0a..d91ce5a04 100644 --- a/app/views/mailer/message_posted.text.html.rhtml +++ b/app/views/mailer/message_posted.text.html.rhtml @@ -1,4 +1,4 @@

<%=h @message.board.project.name %> - <%=h @message.board.name %>: <%= link_to @message.subject, @message_url %>

<%= @message.author %> -<%= textilizable @message.content %> +<%= textilizable(@message, :content, :only_path => false) %> diff --git a/app/views/mailer/news_added.text.html.rhtml b/app/views/mailer/news_added.text.html.rhtml index 010ef8ee1..15bc89fac 100644 --- a/app/views/mailer/news_added.text.html.rhtml +++ b/app/views/mailer/news_added.text.html.rhtml @@ -1,4 +1,4 @@

<%= link_to @news.title, @news_url %>

<%= @news.author.name %> -<%= textilizable(@news.description) %> +<%= textilizable(@news, :description, :only_path => false) %> diff --git a/test/fixtures/journals.yml b/test/fixtures/journals.yml index 169713fc5..70aa5da73 100644 --- a/test/fixtures/journals.yml +++ b/test/fixtures/journals.yml @@ -8,7 +8,7 @@ journals_001: journalized_id: 1 journals_002: created_on: <%= 1.days.ago.to_date.to_s(:db) %> - notes: "Some notes" + notes: "Some notes with Redmine links: #2, r2." id: 2 journalized_type: Issue user_id: 2 diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index 096551ee5..dc3fde414 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -18,7 +18,26 @@ require File.dirname(__FILE__) + '/../test_helper' class MailerTest < Test::Unit::TestCase - fixtures :projects, :issues, :users, :members, :documents, :attachments, :news, :tokens, :journals, :journal_details, :trackers, :issue_statuses, :enumerations + fixtures :projects, :issues, :users, :members, :documents, :attachments, :news, :tokens, :journals, :journal_details, :changesets, :trackers, :issue_statuses, :enumerations + + def test_generated_links_in_emails + ActionMailer::Base.deliveries.clear + Setting.host_name = 'mydomain.foo' + Setting.protocol = 'https' + + journal = Journal.find(2) + assert Mailer.deliver_issue_edit(journal) + + mail = ActionMailer::Base.deliveries.last + assert_kind_of TMail::Mail, mail + # link to the main ticket + assert mail.body.include?('Bug #1: Can\'t print recipes') + + # link to a referenced ticket + assert mail.body.include?('#2') + # link to a changeset + assert mail.body.include?('r2') + end # test mailer methods for each language def test_issue_add -- 2.39.5