From 83068fab94e26f17fb6538360c1d38175e026aa1 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 21 Oct 2014 19:36:55 +0530 Subject: [PATCH 1/2] FEATURE: show full name in emails --- app/mailers/user_notifications.rb | 15 +++++++++++---- spec/mailers/user_notifications_spec.rb | 24 ++++++++++++++++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 3f4d8015d86..f016d4d1b91 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -126,7 +126,8 @@ class UserNotifications < ActionMailer::Base send_notification_email( title: post.topic.title, post: post, - from_alias: post.user.username, + username: post.user.username, + from_alias: SiteSetting.enable_names ? post.user.name : post.user.username, allow_reply_by_email: true, use_site_subject: true, add_re_to_subject: true, @@ -170,7 +171,11 @@ class UserNotifications < ActionMailer::Base return unless @notification = opts[:notification] return unless @post = opts[:post] - username = @notification.data_hash[:original_username] + user_name = @notification.data_hash[:original_username] + if @post && SiteSetting.enable_names + user_name = User.find_by(id: @post.user_id).name + end + notification_type = opts[:notification_type] || Notification.types[@notification.notification_type].to_s return if user.mailing_list_mode && !@post.topic.private_message? && @@ -185,7 +190,8 @@ class UserNotifications < ActionMailer::Base send_notification_email( title: title, post: @post, - from_alias: username, + username: @notification.data_hash[: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, @@ -202,6 +208,7 @@ class UserNotifications < ActionMailer::Base allow_reply_by_email = opts[:allow_reply_by_email] use_site_subject = opts[:use_site_subject] add_re_to_subject = opts[:add_re_to_subject] && post.post_number > 1 + username = opts[:username] from_alias = opts[:from_alias] notification_type = opts[:notification_type] user = opts[:user] @@ -254,7 +261,7 @@ class UserNotifications < ActionMailer::Base post_id: post.id, topic_id: post.topic_id, context: context, - username: from_alias, + username: username, add_unsubscribe_link: true, allow_reply_by_email: allow_reply_by_email, use_site_subject: use_site_subject, diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index baca811de91..706bc9f698f 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -71,16 +71,21 @@ describe UserNotifications do end describe '.user_replied' do + let(:response_by_user) { Fabricate(:user, name: "John Doe") } let(:category) { Fabricate(:category, name: 'India') } let(:topic) { Fabricate(:topic, category: category) } let(:post) { Fabricate(:post, topic: topic) } - let(:response) { Fabricate(:post, topic: post.topic)} + let(:response) { Fabricate(:post, topic: post.topic, user: response_by_user)} let(:user) { Fabricate(:user) } let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do + SiteSetting.stubs(:enable_names).returns(true) mail = UserNotifications.user_replied(response.user, post: response, notification: notification) + # from should include full user name + expect(mail[:from].display_names).to eql(['John Doe']) + # subject should include category name expect(mail.subject).to match(/India/) @@ -109,14 +114,19 @@ describe UserNotifications do end describe '.user_posted' do + let(:response_by_user) { Fabricate(:user, name: "John Doe") } let(:post) { Fabricate(:post) } - let(:response) { Fabricate(:post, topic: post.topic)} + let(:response) { Fabricate(:post, topic: post.topic, user: response_by_user)} let(:user) { Fabricate(:user) } let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do + SiteSetting.stubs(:enable_names).returns(false) mail = UserNotifications.user_posted(response.user, post: response, notification: notification) + # from should not include full user name if "show user full names" is disabled + expect(mail[:from].display_names).to_not eql(['John Doe']) + # subject should not include category name expect(mail.subject).not_to match(/Uncategorized/) @@ -133,14 +143,19 @@ describe UserNotifications do end describe '.user_private_message' do + let(:response_by_user) { Fabricate(:user, name: "John Doe") } let(:topic) { Fabricate(:private_message_topic) } - let(:response) { Fabricate(:post, topic: topic)} + let(:response) { Fabricate(:post, topic: topic, user: response_by_user)} let(:user) { Fabricate(:user) } let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do + SiteSetting.stubs(:enable_names).returns(true) mail = UserNotifications.user_private_message(response.user, post: response, notification: notification) + # from should include full user name + expect(mail[:from].display_names).to eql(['John Doe']) + # subject should include "[PM]" expect(mail.subject).to match("[PM]") @@ -224,7 +239,8 @@ describe UserNotifications do end it "has a from alias" do - expects_build_with(has_entry(:from_alias, "#{username}")) + SiteSetting.stubs(:enable_names).returns(true) + expects_build_with(has_entry(:from_alias, "#{user.name}")) end it "should explain how to respond" do From 8700716fcd705d7ab9baaad0beec99b4387453ca Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 22 Oct 2014 00:44:56 +0530 Subject: [PATCH 2/2] separate site setting for showing full name in emails --- app/mailers/user_notifications.rb | 4 ++-- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 1 + spec/mailers/user_notifications_spec.rb | 8 ++++---- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index f016d4d1b91..c76dbf2db1b 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -127,7 +127,7 @@ class UserNotifications < ActionMailer::Base title: post.topic.title, post: post, username: post.user.username, - from_alias: SiteSetting.enable_names ? post.user.name : post.user.username, + from_alias: SiteSetting.enable_email_names ? post.user.name : post.user.username, allow_reply_by_email: true, use_site_subject: true, add_re_to_subject: true, @@ -172,7 +172,7 @@ class UserNotifications < ActionMailer::Base return unless @post = opts[:post] user_name = @notification.data_hash[:original_username] - if @post && SiteSetting.enable_names + if @post && SiteSetting.enable_email_names user_name = User.find_by(id: @post.user_id).name end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4706a307191..ea169c9e4a1 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -941,6 +941,8 @@ en: strip_images_from_short_emails: "Strip images from emails having size less than 2800 Bytes" short_email_length: "Short email length in Bytes" + enable_email_names: "Allow showing user full name in emails. Disable to hide full name in emails." + pop3_polling_enabled: "Poll via POP3 for email replies." pop3_polling_ssl: "Use SSL while connecting to the POP3 server. (Recommended)" pop3_polling_period_mins: "The period in minutes between checking the POP3 account for email. NOTE: requires restart." diff --git a/config/site_settings.yml b/config/site_settings.yml index 6e736b48b0e..525a68b3693 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -416,6 +416,7 @@ email: disable_emails: false strip_images_from_short_emails: true short_email_length: 2800 + enable_email_names: true files: max_image_size_kb: diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 706bc9f698f..c166c758404 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -80,7 +80,7 @@ describe UserNotifications do let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do - SiteSetting.stubs(:enable_names).returns(true) + SiteSetting.stubs(:enable_email_names).returns(true) mail = UserNotifications.user_replied(response.user, post: response, notification: notification) # from should include full user name @@ -121,7 +121,7 @@ describe UserNotifications do let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do - SiteSetting.stubs(:enable_names).returns(false) + SiteSetting.stubs(:enable_email_names).returns(false) mail = UserNotifications.user_posted(response.user, post: response, notification: notification) # from should not include full user name if "show user full names" is disabled @@ -150,7 +150,7 @@ describe UserNotifications do let(:notification) { Fabricate(:notification, user: user) } it 'generates a correct email' do - SiteSetting.stubs(:enable_names).returns(true) + SiteSetting.stubs(:enable_email_names).returns(true) mail = UserNotifications.user_private_message(response.user, post: response, notification: notification) # from should include full user name @@ -239,7 +239,7 @@ describe UserNotifications do end it "has a from alias" do - SiteSetting.stubs(:enable_names).returns(true) + SiteSetting.stubs(:enable_email_names).returns(true) expects_build_with(has_entry(:from_alias, "#{user.name}")) end