Merge pull request #4508 from kstaikov/mailing_list_exclude_own_replies

FEATURE:'No Echo' option for mailing list mode.
This commit is contained in:
Régis Hanol 2016-10-22 10:45:14 +02:00 committed by GitHub
commit 3c8e0a8348
9 changed files with 62 additions and 5 deletions

View File

@ -79,7 +79,8 @@ export default Ember.Controller.extend(CanCheckEmails, {
mailingListModeOptions() { mailingListModeOptions() {
return [ return [
{name: I18n.t('user.mailing_list_mode.daily'), value: 0}, {name: I18n.t('user.mailing_list_mode.daily'), value: 0},
{name: this.get('frequencyEstimate'), value: 1} {name: this.get('frequencyEstimate'), value: 1},
{name: I18n.t('user.mailing_list_mode.individual_no_echo'), value: 2}
]; ];
}, },

View File

@ -16,7 +16,7 @@ module Jobs
users = users =
User.activated.not_blocked.not_suspended.real User.activated.not_blocked.not_suspended.real
.joins(:user_option) .joins(:user_option)
.where(user_options: {mailing_list_mode: true, mailing_list_mode_frequency: 1}) .where('user_options.mailing_list_mode AND user_options.mailing_list_mode_frequency > 0')
.where('NOT EXISTS( .where('NOT EXISTS(
SELECT 1 SELECT 1
FROM topic_users tu FROM topic_users tu
@ -46,6 +46,11 @@ module Jobs
next next
end end
if (user.id == post.user_id) && (user.user_option.mailing_list_mode_frequency == 2)
skip(user.email, user.id, post.id, I18n.t('email_log.no_echo_mailing_list_mode'))
next
end
begin begin
if message = UserNotifications.mailing_list_notify(user, post) if message = UserNotifications.mailing_list_notify(user, post)
EmailLog.unique_email_per_post(post, user) do EmailLog.unique_email_per_post(post, user) do

View File

@ -103,7 +103,7 @@ module Jobs
end end
if user.user_option.mailing_list_mode? && if user.user_option.mailing_list_mode? &&
user.user_option.mailing_list_mode_frequency == 1 && # don't catch notifications for users on daily mailing list mode user.user_option.mailing_list_mode_frequency > 0 && # don't catch notifications for users on daily mailing list mode
(!post.try(:topic).try(:private_message?)) && (!post.try(:topic).try(:private_message?)) &&
NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type]) NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type])
# no need to log a reason when the mail was already sent via the mailing list job # no need to log a reason when the mail was already sent via the mailing list job

View File

@ -9,7 +9,8 @@ class MailingListModeSiteSetting < EnumSiteSetting
def self.values def self.values
@values ||= [ @values ||= [
{ name: 'user.mailing_list_mode.daily', value: 0 }, { name: 'user.mailing_list_mode.daily', value: 0 },
{ name: 'user.mailing_list_mode.individual', value: 1 } { name: 'user.mailing_list_mode.individual', value: 1 },
{ name: 'user.mailing_list_mode.individual_no_echo', value: 2 }
] ]
end end

View File

@ -535,6 +535,7 @@ en:
Muted topics and categories are not included in these emails. Muted topics and categories are not included in these emails.
daily: "Send daily updates" daily: "Send daily updates"
individual: "Send an email for every new post" individual: "Send an email for every new post"
individual_no_echo: "Send an email for every new post except my own"
many_per_day: "Send me an email for every new post (about {{dailyEmailEstimate}} per day)" many_per_day: "Send me an email for every new post (about {{dailyEmailEstimate}} per day)"
few_per_day: "Send me an email for every new post (about 2 per day)" few_per_day: "Send me an email for every new post (about 2 per day)"
tag_settings: "Tags" tag_settings: "Tags"

View File

@ -2614,6 +2614,7 @@ en:
message_to_blank: "message.to is blank" message_to_blank: "message.to is blank"
text_part_body_blank: "text_part.body is blank" text_part_body_blank: "text_part.body is blank"
body_blank: "body is blank" body_blank: "body is blank"
no_echo_mailing_list_mode: "Mailing list notifications disabled for user's own posts"
color_schemes: color_schemes:
base_theme_name: "Base" base_theme_name: "Base"

View File

@ -82,6 +82,11 @@ describe Jobs::EnqueueMailingListEmails do
expect(subject).to_not include user.id expect(subject).to_not include user.id
end end
it "doesn't return users with mailing list mode set to 'individual_excluding_own'" do
user_option.update(mailing_list_mode_frequency: 2)
expect(subject).to_not include user.id
end
it "doesn't return a user who has received the mailing list summary earlier" do it "doesn't return a user who has received the mailing list summary earlier" do
user.update(first_seen_at: 5.hours.ago) user.update(first_seen_at: 5.hours.ago)
expect(subject).to_not include user.id expect(subject).to_not include user.id

View File

@ -47,9 +47,15 @@ describe Jobs::NotifyMailingListSubscribers do
end end
end end
context "with a valid post" do context "with a valid post authored by same user" do
let!(:post) { Fabricate(:post, user: user) } let!(:post) { Fabricate(:post, user: user) }
it "doesn't send the email to the user if the frequency is set to 'always with no echo'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2)
UserNotifications.expects(:mailing_list_notify).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "sends the email to the user if the frequency is set to 'always'" do it "sends the email to the user if the frequency is set to 'always'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1) user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1)
UserNotifications.expects(:mailing_list_notify).with(user, post).once UserNotifications.expects(:mailing_list_notify).with(user, post).once
@ -63,6 +69,28 @@ describe Jobs::NotifyMailingListSubscribers do
end end
end end
context "with a valid post created by someone other than the user" do
let!(:post) { Fabricate(:post, user: user, user_id: 'different') }
it "sends the email to the user if the frequency is set to 'always with no echo'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2)
UserNotifications.expects(:mailing_list_notify).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "sends the email to the user if the frequency is set to 'always'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1)
UserNotifications.expects(:mailing_list_notify).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "does not send the email to the user if the frequency is set to 'daily'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
UserNotifications.expects(:mailing_list_notify).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "with a deleted post" do context "with a deleted post" do
let!(:post) { Fabricate(:post, user: user, deleted_at: Time.now) } let!(:post) { Fabricate(:post, user: user, deleted_at: Time.now) }

View File

@ -240,6 +240,21 @@ describe Jobs::UserEmail do
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id)
end end
it "doesn't send the mail if the user is using individual mailing list mode with no echo" do
Email::Sender.any_instance.expects(:send).never
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2)
# sometimes, we pass the notification_id
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id)
# other times, we only pass the type of notification
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id)
# When post is nil
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted")
# When post does not have a topic
post = Fabricate(:post)
post.topic.destroy
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post.id)
end
it "doesn't send the email if the post has been user deleted" do it "doesn't send the email if the post has been user deleted" do
Email::Sender.any_instance.expects(:send).never Email::Sender.any_instance.expects(:send).never
post.update_column(:user_deleted, true) post.update_column(:user_deleted, true)