From: Tom Hughes Date: Sun, 5 Feb 2017 13:50:07 +0000 (+0000) Subject: Merge remote-tracking branch 'openstreetmap/pull/1401' X-Git-Tag: live~3697 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/c57d2b29fc1f4647b460b6b74d072c409932e3f2?hp=30f9f95ef6f6fd0fa91e343094cdd5309213ff7a Merge remote-tracking branch 'openstreetmap/pull/1401' --- diff --git a/app/assets/images/osm_logo_30.png b/app/assets/images/osm_logo_30.png new file mode 100644 index 000000000..c963f4f4a Binary files /dev/null and b/app/assets/images/osm_logo_30.png differ diff --git a/app/helpers/notifier_helper.rb b/app/helpers/notifier_helper.rb index 4b2cd2a06..3e53e2543 100644 --- a/app/helpers/notifier_helper.rb +++ b/app/helpers/notifier_helper.rb @@ -2,4 +2,24 @@ module NotifierHelper def fp(text) format_paragraph(text, 72, 0) end + + def link_to_user(display_name) + link_to( + display_name, + user_url(display_name, :host => SERVER_URL), + :target => "_blank", + :style => "text-decoration: none; color: #222; font-weight: bold" + ) + end + + def message_body(&block) + render( + :partial => "message_body", + :locals => { :body => capture(&block) } + ) + end + + def style_message(html) + html.gsub /

/, '

' + end end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 0539bdeb7..0ed7071ac 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -3,6 +3,8 @@ class Notifier < ActionMailer::Base :return_path => EMAIL_RETURN_PATH, :auto_submitted => "auto-generated" helper :application + before_action :set_shared_template_vars + before_action :attach_project_logo def signup_confirm(user, token) with_recipient_locale user do @@ -76,6 +78,9 @@ class Notifier < ActionMailer::Base @replyurl = url_for(:host => SERVER_URL, :controller => "message", :action => "reply", :message_id => message.id) + @author = @from_user + + attach_user_avatar(message.sender) mail :from => from_address(message.sender.display_name, "m", message.id, message.digest), :to => message.recipient.email, @@ -106,6 +111,9 @@ class Notifier < ActionMailer::Base :action => "new", :display_name => comment.user.display_name, :title => "Re: #{comment.diary_entry.title}") + @author = @from_user + + attach_user_avatar(comment.user) mail :from => from_address(comment.user.display_name, "c", comment.id, comment.digest, recipient.id), :to => recipient.email, @@ -122,7 +130,9 @@ class Notifier < ActionMailer::Base @friendurl = url_for(:host => SERVER_URL, :controller => "user", :action => "make_friend", :display_name => @friend.befriender.display_name) + @author = @friend.befriender.display_name + attach_user_avatar(@friend.befriender) mail :to => friend.befriendee.email, :subject => I18n.t("notifier.friend_notification.subject", :user => friend.befriender.display_name) end @@ -142,6 +152,9 @@ class Notifier < ActionMailer::Base I18n.t("notifier.note_comment_notification.anonymous") end + @author = @commenter + attach_user_avatar(comment.author) + subject = if @owner I18n.t("notifier.note_comment_notification.#{@event}.subject_own", :commenter => @commenter) else @@ -161,6 +174,7 @@ class Notifier < ActionMailer::Base @changeset_comment = comment.changeset.tags["comment"].presence @time = comment.created_at @changeset_author = comment.changeset.user.display_name + @author = @commenter subject = if @owner I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter) @@ -168,12 +182,35 @@ class Notifier < ActionMailer::Base I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter) end + attach_user_avatar(comment.author) + mail :to => recipient.email, :subject => subject end end private + def set_shared_template_vars + @root_url = root_url(:host => SERVER_URL) + end + + def attach_project_logo + attachments.inline["logo.png"] = File.read("#{Rails.root}/app/assets/images/osm_logo_30.png") + end + + def attach_user_avatar(user) + attachments.inline["avatar.png"] = File.read(user_avatar_file_path(user)) + end + + def user_avatar_file_path(user) + image = user && user.image + if image && image.file? + return image.path(:small) + else + return "#{Rails.root}/app/assets/images/users/images/small.png" + end + end + def with_recipient_locale(recipient) I18n.with_locale Locale.available.preferred(recipient.preferred_languages) do yield diff --git a/app/views/layouts/notifier.html.erb b/app/views/layouts/notifier.html.erb new file mode 100644 index 000000000..dd041cca5 --- /dev/null +++ b/app/views/layouts/notifier.html.erb @@ -0,0 +1,46 @@ + + + + + + + + + + + + +
+ + + + + + + + +
+ + <%= image_tag attachments["logo.png"].url, alt: "OpenStreetMap logo", title: "OpenStreetMap", height: "30", width: "30", border: "0" %> + + + +
OpenStreetMap
+ +
+ + + + +
+ <%= raw style_message(yield) %> +
+
+
+ <%= yield :footer %> +
+ OpenStreetMap +
+
+ + diff --git a/app/views/notifier/_gpx_description.html.erb b/app/views/notifier/_gpx_description.html.erb index 282579919..8d44336c9 100644 --- a/app/views/notifier/_gpx_description.html.erb +++ b/app/views/notifier/_gpx_description.html.erb @@ -1,14 +1,12 @@ -<%= t'notifier.gpx_notification.greeting' %> - <%= t'notifier.gpx_notification.your_gpx_file' %> - - <%= @trace_name %> - +<%= @trace_name %> <%= t'notifier.gpx_notification.with_description' %> - - <%= @trace_description %> +<%= @trace_description %> <% if @trace_tags.length>0 %> -<%= t'notifier.gpx_notification.and_the_tags' %> -<% @trace_tags.each do |tag| %> - <%= tag.tag.rstrip %><% end %><% else %> -<%= t'notifier.gpx_notification.and_no_tags' %><% end %> + <%= t'notifier.gpx_notification.and_the_tags' %> + <% @trace_tags.each do |tag| %> + <%= tag.tag.rstrip %> + <% end %> +<% else %> + <%= t'notifier.gpx_notification.and_no_tags' %> +<% end %> diff --git a/app/views/notifier/_message_body.html.erb b/app/views/notifier/_message_body.html.erb new file mode 100644 index 000000000..d86d44133 --- /dev/null +++ b/app/views/notifier/_message_body.html.erb @@ -0,0 +1,21 @@ + + + + + +
+ <%= link_to( + image_tag( + attachments["avatar.png"].url, + alt: @author, + title: @author, + width: 50, + height: 50, + border: 0 + ), + user_url(@author, :host => SERVER_URL), + :target => "_blank" + ) %> + + <%= body %> +
diff --git a/app/views/notifier/changeset_comment_notification.html.erb b/app/views/notifier/changeset_comment_notification.html.erb index b7646a886..c2b552ed5 100644 --- a/app/views/notifier/changeset_comment_notification.html.erb +++ b/app/views/notifier/changeset_comment_notification.html.erb @@ -1,20 +1,26 @@ -

<%= t 'notifier.changeset_comment_notification.greeting' %>

<% if @owner %> - <%= t "notifier.changeset_comment_notification.commented.your_changeset", :commenter => @commenter, :time => @time %> + <%= raw t "notifier.changeset_comment_notification.commented.your_changeset", :commenter => link_to_user(@commenter), :time => @time %> <% else %> - <%= t "notifier.changeset_comment_notification.commented.commented_changeset", :commenter => @commenter, :time => @time, :changeset_author => @changeset_author %> + <%= raw t "notifier.changeset_comment_notification.commented.commented_changeset", :commenter => link_to_user(@commenter), :time => @time, :changeset_author => @changeset_author %> <% end %> <% if @changeset_comment %> - <%= t "notifier.changeset_comment_notification.commented.partial_changeset_with_comment", :changeset_comment => @changeset_comment %> + <%= raw t "notifier.changeset_comment_notification.commented.partial_changeset_with_comment", :changeset_comment => content_tag("em", @changeset_comment) %> <% else %> <%= t "notifier.changeset_comment_notification.commented.partial_changeset_without_comment" %> <% end %>

-== -<%= @comment.to_html %> -== +<%= message_body do %> + <%= @comment.to_html %> +<% end %> + +

+ <%= raw t 'notifier.changeset_comment_notification.details', :url => link_to(@changeset_url, @changeset_url, :style => "white-space: nowrap") %> +

<%= raw t 'notifier.changeset_comment_notification.details', :url => link_to(@changeset_url, @changeset_url) %>

+<% content_for :footer do %> +

+ <%= raw t 'notifier.changeset_comment_notification.unsubscribe', :url => link_to(@changeset_url, @changeset_url, :style => "color: #222; white-space: nowrap") %> +

+<% end %> diff --git a/app/views/notifier/changeset_comment_notification.text.erb b/app/views/notifier/changeset_comment_notification.text.erb index 44a3c1216..5da6feddc 100644 --- a/app/views/notifier/changeset_comment_notification.text.erb +++ b/app/views/notifier/changeset_comment_notification.text.erb @@ -16,3 +16,5 @@ == <%= t 'notifier.changeset_comment_notification.details', :url => @changeset_url %> + +<%= t 'notifier.changeset_comment_notification.unsubscribe', :url => @changeset_url %> diff --git a/app/views/notifier/diary_comment_notification.html.erb b/app/views/notifier/diary_comment_notification.html.erb index b47900a63..73bfe9a33 100644 --- a/app/views/notifier/diary_comment_notification.html.erb +++ b/app/views/notifier/diary_comment_notification.html.erb @@ -1,9 +1,18 @@ -

<%= t'notifier.diary_comment_notification.hi', :to_user => @to_user %>

+ <%= t'notifier.diary_comment_notification.hi', :to_user => @to_user %> +

+ <%= raw t'notifier.diary_comment_notification.header', :from_user => link_to_user(@from_user), :subject => content_tag("em", @title) %> +

<%= raw t'notifier.diary_comment_notification.header', :from_user => link_to(@from_user, :host => SERVER_URL, :controller => :user, :action => :view, :display_name => @from_user), :subject => @title %>

+<%= message_body do %> + <%= @text.to_html %> +<% end %> -== -<%= @text.to_html %> -== - -

<%= raw t'notifier.diary_comment_notification.footer', :readurl => link_to(@readurl, @readurl), :commenturl => link_to(@commenturl, @commenturl), :replyurl => link_to(@replyurl, @replyurl) %>

+<% content_for :footer do %> +

<%= raw t'notifier.diary_comment_notification.footer', + :readurl => link_to(@readurl, @readurl) + tag(:br), + :commenturl => link_to(@commenturl, @commenturl) + tag(:br), + :replyurl => link_to(@replyurl, @replyurl) + %>

+<% end %> diff --git a/app/views/notifier/email_confirm.html.erb b/app/views/notifier/email_confirm.html.erb index 5b7c74d7f..5d8f49d3d 100644 --- a/app/views/notifier/email_confirm.html.erb +++ b/app/views/notifier/email_confirm.html.erb @@ -4,4 +4,4 @@

<%= t 'notifier.email_confirm_html.click_the_link' %>

<%= @url %>

diff --git a/app/views/notifier/friend_notification.html.erb b/app/views/notifier/friend_notification.html.erb index 181b2b825..cfea97195 100644 --- a/app/views/notifier/friend_notification.html.erb +++ b/app/views/notifier/friend_notification.html.erb @@ -1,7 +1,9 @@

<%= t 'notifier.friend_notification.had_added_you', :user => @friend.befriender.display_name %>

<%= raw t 'notifier.friend_notification.see_their_profile', :userurl => link_to(@viewurl, @viewurl) %>

+<%= message_body do %> +

<%= raw t 'notifier.friend_notification.see_their_profile', :userurl => link_to(@viewurl, @viewurl) %>

-<% unless @friend.befriendee.is_friends_with?(@friend.befriender) -%> -

<%= raw t 'notifier.friend_notification.befriend_them', :befriendurl => link_to(@friendurl, @friendurl) %>

-<% end -%> + <% unless @friend.befriendee.is_friends_with?(@friend.befriender) -%> +

<%= raw t 'notifier.friend_notification.befriend_them', :befriendurl => link_to(@friendurl, @friendurl) %>

+ <% end -%> +<% end %> diff --git a/app/views/notifier/gpx_failure.html.erb b/app/views/notifier/gpx_failure.html.erb index f59aa3dbb..dace18522 100644 --- a/app/views/notifier/gpx_failure.html.erb +++ b/app/views/notifier/gpx_failure.html.erb @@ -1,9 +1,16 @@ -<%= render :partial => "gpx_description" %> -<%= t'notifier.gpx_notification.failure.failed_to_import' %> +

<%= t'notifier.gpx_notification.greeting' %>

- <%= @error %> +

+ <%= render :partial => "gpx_description" %> + <%= t'notifier.gpx_notification.failure.failed_to_import' %> +

-<%= t'notifier.gpx_notification.failure.more_info_1' %> -<%= t'notifier.gpx_notification.failure.more_info_2' %> +

+ <%= @error %> +

+ <%= t'notifier.gpx_notification.failure.more_info_1' %> + <%= t'notifier.gpx_notification.failure.more_info_2' %> <%= t'notifier.gpx_notification.failure.import_failures_url' %> +

diff --git a/app/views/notifier/gpx_success.html.erb b/app/views/notifier/gpx_success.html.erb index 1983fe71c..d298bd70a 100644 --- a/app/views/notifier/gpx_success.html.erb +++ b/app/views/notifier/gpx_success.html.erb @@ -1,2 +1,6 @@ -<%= render :partial => "gpx_description" %> -<%= t'notifier.gpx_notification.success.loaded_successfully', :trace_points => @trace_points, :possible_points => @possible_points %> +

<%= t'notifier.gpx_notification.greeting' %>

+ +

+ <%= render :partial => "gpx_description" %> + <%= t'notifier.gpx_notification.success.loaded_successfully', :trace_points => @trace_points, :possible_points => @possible_points %> +

diff --git a/app/views/notifier/message_notification.html.erb b/app/views/notifier/message_notification.html.erb index 19704251d..97a352a49 100644 --- a/app/views/notifier/message_notification.html.erb +++ b/app/views/notifier/message_notification.html.erb @@ -1,9 +1,22 @@ -

<%= t'notifier.message_notification.hi', :to_user => @to_user %>

+ <%= t'notifier.message_notification.hi', :to_user => @to_user %> +

+ <%= raw t'notifier.message_notification.header', + :from_user => link_to_user(@from_user), + :subject => content_tag("em", @title) + %> +

<%= raw t'notifier.message_notification.header', :from_user => link_to(@from_user, :host => SERVER_URL, :controller => :user, :action => :view, :display_name => @from_user), :subject => @title %>

+<%= message_body do %> + <%= @text.to_html %> +<% end %> -== -<%= @text.to_html %> -== - -

<%= t'notifier.message_notification.footer_html', :readurl => link_to(@readurl, @readurl), :replyurl => link_to(@replyurl, @replyurl) %>

+<% content_for :footer do %> +

+ <%= t'notifier.message_notification.footer_html', + :readurl => link_to(@readurl, @readurl) + tag(:br), + :replyurl => link_to(@replyurl, @replyurl) + %> +

+<% end %> diff --git a/app/views/notifier/note_comment_notification.html.erb b/app/views/notifier/note_comment_notification.html.erb index d82723bbb..ecaff81dc 100644 --- a/app/views/notifier/note_comment_notification.html.erb +++ b/app/views/notifier/note_comment_notification.html.erb @@ -1,13 +1,13 @@

<%= t 'notifier.note_comment_notification.greeting' %>

<% if @owner %> -

<%= t "notifier.note_comment_notification.#{@event}.your_note", :commenter => @commenter, :place => @place %>

<%= raw t "notifier.note_comment_notification.#{@event}.your_note", :commenter => link_to_user(@commenter), :place => @place %>

<% else %> -

<%= t "notifier.note_comment_notification.#{@event}.commented_note", :commenter => @commenter, :place => @place %>

<%= raw t "notifier.note_comment_notification.#{@event}.commented_note", :commenter => link_to_user(@commenter), :place => @place %>

<% end %> -== -<%= @comment.to_html %> -== +<%= message_body do %> + <%= @comment.to_html %> +<% end %>

<%= raw t 'notifier.note_comment_notification.details', :url => link_to(@noteurl, @noteurl) %>

diff --git a/app/views/notifier/signup_confirm.html.erb b/app/views/notifier/signup_confirm.html.erb index 814deee91..41b2ceb49 100644 --- a/app/views/notifier/signup_confirm.html.erb +++ b/app/views/notifier/signup_confirm.html.erb @@ -4,6 +4,6 @@

<%= t("notifier.signup_confirm.confirm") %>

<%= link_to @url, @url %>

<%= link_to @url, @url, :style => "white-space: nowrap" %>

<%= t("notifier.signup_confirm.welcome") %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index cf80c0862..9389c0e2f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1310,6 +1310,7 @@ en: commented_note: "%{commenter} has reactivated a map note you have commented on. The note is near %{place}." details: "More details about the note can be found at %{url}." changeset_comment_notification: + hi: "Hi %{to_user}," greeting: "Hi," commented: subject_own: "[OpenStreetMap] %{commenter} has commented on one of your changesets" @@ -1319,6 +1320,7 @@ en: partial_changeset_with_comment: "with comment '%{changeset_comment}'" partial_changeset_without_comment: "without comment" details: "More details about the changeset can be found at %{url}." + unsubscribe: 'To unsubscribe from updates to this changeset, visit %{url} and click "Unsubscribe".' message: inbox: title: "Inbox" diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index db35be5f3..09d2cc1fd 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -135,10 +135,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -248,10 +248,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_openid/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -365,10 +365,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_google/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -478,10 +478,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_facebook/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -591,10 +591,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_windowslive/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -704,10 +704,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_github/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success @@ -817,10 +817,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_equal register_email.to[0], new_email # Check that the confirm account url is correct confirm_regex = Regexp.new("/user/redirect_tester_wikipedia/confirm\\?confirm_string=([a-zA-Z0-9]*)") - register_email.parts.each do |part| + email_text_parts(register_email).each do |part| assert_match confirm_regex, part.body.to_s end - confirm_string = register_email.parts[0].body.match(confirm_regex)[1] + confirm_string = email_text_parts(register_email)[0].body.match(confirm_regex)[1] # Check the page assert_response :success diff --git a/test/test_helper.rb b/test/test_helper.rb index e535d1645..10a4eb397 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -183,5 +183,15 @@ module ActiveSupport stub_request(:get, "http://api.hostip.info/country.php?ip=0.0.0.0") stub_request(:get, "http://api.hostip.info/country.php?ip=127.0.0.1") end + + def email_text_parts(message) + message.parts.each_with_object([]) do |part, text_parts| + if part.content_type.start_with?("text/") + text_parts.push(part) + elsif part.multipart? + text_parts.concat(email_text_parts(part)) + end + end + end end end