From 8d1da9ceddab88e8be6553329b535dd5398f7b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 23 Feb 2016 01:34:16 +0100 Subject: [PATCH] FEATURE: keep original title when sending email notifications about a PM --- app/mailers/user_notifications.rb | 15 +++++---------- app/models/notification.rb | 2 +- app/services/post_alerter.rb | 10 +++++++++- spec/services/post_alerter_spec.rb | 22 ++++++++++++++++++++++ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 542744d3983..018fba10251 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -236,25 +236,20 @@ class UserNotifications < ActionMailer::Base user_name = name unless name.blank? end - title = notification_data[:topic_title] allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended? - use_site_subject = opts[:use_site_subject] - add_re_to_subject = opts[:add_re_to_subject] - show_category_in_subject = opts[:show_category_in_subject] - use_template_html = opts[:use_template_html] original_username = notification_data[:original_username] || notification_data[:display_username] send_notification_email( - title: title, + title: notification_data[:topic_title], post: post, username: original_username, from_alias: user_name, allow_reply_by_email: allow_reply_by_email, - use_site_subject: use_site_subject, - add_re_to_subject: add_re_to_subject, - show_category_in_subject: show_category_in_subject, + use_site_subject: opts[:use_site_subject], + add_re_to_subject: opts[:add_re_to_subject], + show_category_in_subject: opts[:show_category_in_subject], notification_type: notification_type, - use_template_html: use_template_html, + use_template_html: opts[:use_template_html], user: user ) end diff --git a/app/models/notification.rb b/app/models/notification.rb index d21875c8b2c..2f8090abc02 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -98,8 +98,8 @@ class Notification < ActiveRecord::Base # Be wary of calling this frequently. O(n) JSON parsing can suck. def data_hash @data_hash ||= begin - return nil if data.blank? + parsed = JSON.parse(data) return nil if parsed.blank? diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 5ee0ad8e19c..29b9ec1aa15 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -259,8 +259,16 @@ class PostAlerter UserActionObserver.log_notification(original_post, user, type, opts[:acting_user_id]) + topic_title = post.topic.title + # when sending a private message email, keep the original title + if post.topic.private_message? && modifications = post.revisions.map(&:modifications).to_a + if first_title_modification = modifications.first { |m| m.has_key?("title") } + topic_title = first_title_modification["title"][0] + end + end + notification_data = { - topic_title: post.topic.title, + topic_title: topic_title, original_post_id: original_post.id, original_post_type: original_post.post_type, original_username: original_username, diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 882e680dc8a..10f4c10b085 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -150,4 +150,26 @@ describe PostAlerter do end end + + + describe ".create_notification" do + + it "keeps the original title for PMs" do + user = Fabricate(:user) + topic = Fabricate(:private_message_topic, user: user, created_at: 1.hour.ago) + post = Fabricate(:post, topic: topic, created_at: 1.hour.ago) + + original_title = topic.title + + post.revise(user, { title: "This is the revised title" }, revised_at: Time.now) + + expect { + PostAlerter.new.create_notification(user, Notification.types[:private_message], post) + }.to change { user.notifications.count } + + expect(user.notifications.last.data_hash["topic_title"]).to eq(original_title) + end + + end + end