FEATURE: limit daily emails per user to 100 per day via site setting
- controlled via max_emails_per_day_per_user, 0 to disable - when limit is reached user is warned
This commit is contained in:
parent
389801f244
commit
c095304d6d
|
@ -35,8 +35,19 @@ module Jobs
|
||||||
users.each do |user|
|
users.each do |user|
|
||||||
if Guardian.new(user).can_see?(post)
|
if Guardian.new(user).can_see?(post)
|
||||||
begin
|
begin
|
||||||
message = UserNotifications.mailing_list_notify(user, post)
|
if EmailLog.reached_max_emails?(user)
|
||||||
Email::Sender.new(message, :mailing_list, user).send if message
|
EmailLog.create!(
|
||||||
|
email_type: 'mailing_list',
|
||||||
|
to_address: user.email,
|
||||||
|
user_id: user.id,
|
||||||
|
post_id: post.id,
|
||||||
|
skipped: true,
|
||||||
|
skipped_reason: "[MailingList] #{I18n.t('email_log.exceeded_limit')}"
|
||||||
|
)
|
||||||
|
else
|
||||||
|
message = UserNotifications.mailing_list_notify(user, post)
|
||||||
|
Email::Sender.new(message, :mailing_list, user).send if message
|
||||||
|
end
|
||||||
rescue => e
|
rescue => e
|
||||||
Discourse.handle_job_exception(e, error_context(args, "Sending post to mailing list subscribers", { user_id: user.id, user_email: user.email }))
|
Discourse.handle_job_exception(e, error_context(args, "Sending post to mailing list subscribers", { user_id: user.id, user_email: user.email }))
|
||||||
end
|
end
|
||||||
|
|
|
@ -113,6 +113,10 @@ module Jobs
|
||||||
email_args[:new_email] = user.email
|
email_args[:new_email] = user.email
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if EmailLog.reached_max_emails?(user)
|
||||||
|
return skip_message(I18n.t('email_log.exceeded_limit'))
|
||||||
|
end
|
||||||
|
|
||||||
message = UserNotifications.send(type, user, email_args)
|
message = UserNotifications.send(type, user, email_args)
|
||||||
|
|
||||||
# Update the to address if we have a custom one
|
# Update the to address if we have a custom one
|
||||||
|
|
|
@ -317,6 +317,11 @@ class UserNotifications < ActionMailer::Base
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
reached_limit = SiteSetting.max_emails_per_day_per_user > 0
|
||||||
|
reached_limit &&= (EmailLog.where(user_id: user.id, skipped: false)
|
||||||
|
.where('created_at > ?', 1.day.ago)
|
||||||
|
.count) >= (SiteSetting.max_emails_per_day_per_user-1)
|
||||||
|
|
||||||
topic_excerpt = ""
|
topic_excerpt = ""
|
||||||
if opts[:use_template_html]
|
if opts[:use_template_html]
|
||||||
topic_excerpt = post.excerpt.gsub("\n", " ") if post.is_first_post? && post.excerpt
|
topic_excerpt = post.excerpt.gsub("\n", " ") if post.is_first_post? && post.excerpt
|
||||||
|
@ -326,6 +331,7 @@ class UserNotifications < ActionMailer::Base
|
||||||
template: 'email/notification',
|
template: 'email/notification',
|
||||||
format: :html,
|
format: :html,
|
||||||
locals: { context_posts: context_posts,
|
locals: { context_posts: context_posts,
|
||||||
|
reached_limit: reached_limit,
|
||||||
post: post,
|
post: post,
|
||||||
in_reply_to_post: in_reply_to_post,
|
in_reply_to_post: in_reply_to_post,
|
||||||
classes: RTL.new(user).css_class
|
classes: RTL.new(user).css_class
|
||||||
|
@ -339,10 +345,11 @@ class UserNotifications < ActionMailer::Base
|
||||||
template << "_staged" if user.staged?
|
template << "_staged" if user.staged?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
email_opts = {
|
email_opts = {
|
||||||
topic_title: title,
|
topic_title: title,
|
||||||
topic_excerpt: topic_excerpt,
|
topic_excerpt: topic_excerpt,
|
||||||
message: email_post_markdown(post),
|
message: email_post_markdown(post) + (reached_limit ? "\n\n#{I18n.t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user}" : ""),
|
||||||
url: post.url,
|
url: post.url,
|
||||||
post_id: post.id,
|
post_id: post.id,
|
||||||
topic_id: post.topic_id,
|
topic_id: post.topic_id,
|
||||||
|
|
|
@ -13,6 +13,16 @@ class EmailLog < ActiveRecord::Base
|
||||||
User.where(id: user_id).update_all("last_emailed_at = CURRENT_TIMESTAMP") if user_id.present? && !skipped
|
User.where(id: user_id).update_all("last_emailed_at = CURRENT_TIMESTAMP") if user_id.present? && !skipped
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.reached_max_emails?(user)
|
||||||
|
return false if SiteSetting.max_emails_per_day_per_user == 0
|
||||||
|
|
||||||
|
count = sent.where('created_at > ?', 1.day.ago)
|
||||||
|
.where(user_id: user.id)
|
||||||
|
.count
|
||||||
|
|
||||||
|
count >= SiteSetting.max_emails_per_day_per_user
|
||||||
|
end
|
||||||
|
|
||||||
def self.count_per_day(start_date, end_date)
|
def self.count_per_day(start_date, end_date)
|
||||||
sent.where("created_at BETWEEN ? AND ?", start_date, end_date)
|
sent.where("created_at BETWEEN ? AND ?", start_date, end_date)
|
||||||
.group("DATE(created_at)")
|
.group("DATE(created_at)")
|
||||||
|
|
|
@ -10,12 +10,12 @@
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<% if in_reply_to_post.present? %>
|
<% if in_reply_to_post.present? %>
|
||||||
<h4 class='.previous-discussion'><%= t "user_notifications.in_reply_to" %></h4>
|
<h4 class='previous-discussion'><%= t "user_notifications.in_reply_to" %></h4>
|
||||||
<%= render partial: 'email/post', locals: { post: in_reply_to_post, use_excerpt: true} %>
|
<%= render partial: 'email/post', locals: { post: in_reply_to_post, use_excerpt: true} %>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|
||||||
<% if context_posts.present? %>
|
<% if context_posts.present? %>
|
||||||
<h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
|
<h4 class='previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
|
||||||
<% context_posts.each do |p| %>
|
<% context_posts.each do |p| %>
|
||||||
<%= render partial: 'email/post', locals: { post: p, use_excerpt: false } %>
|
<%= render partial: 'email/post', locals: { post: p, use_excerpt: false } %>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
@ -23,6 +23,9 @@
|
||||||
|
|
||||||
<hr>
|
<hr>
|
||||||
|
|
||||||
|
<% if reached_limit %>
|
||||||
|
<div class='footer'><%= t "user_notifications.reached_limit", count: SiteSetting.max_emails_per_day_per_user %></div>
|
||||||
|
<% end %>
|
||||||
<div class='footer'>%{respond_instructions}</div>
|
<div class='footer'>%{respond_instructions}</div>
|
||||||
<div class='footer'>%{unsubscribe_link}%{unsubscribe_via_email_link}</div>
|
<div class='footer'>%{unsubscribe_link}%{unsubscribe_via_email_link}</div>
|
||||||
|
|
||||||
|
|
|
@ -1161,6 +1161,7 @@ en:
|
||||||
unsubscribe_via_email_footer: "Attach an unsubscribe link to the footer of sent emails"
|
unsubscribe_via_email_footer: "Attach an unsubscribe link to the footer of sent emails"
|
||||||
|
|
||||||
delete_email_logs_after_days: "Delete email logs after (N) days. 0 to keep indefinitely"
|
delete_email_logs_after_days: "Delete email logs after (N) days. 0 to keep indefinitely"
|
||||||
|
max_emails_per_day_per_user: "Maximum number of emails to send users per day. 0 to disable the limit"
|
||||||
|
|
||||||
manual_polling_enabled: "Push emails using the API for email replies."
|
manual_polling_enabled: "Push emails using the API for email replies."
|
||||||
pop3_polling_enabled: "Poll via POP3 for email replies."
|
pop3_polling_enabled: "Poll via POP3 for email replies."
|
||||||
|
@ -2037,6 +2038,9 @@ en:
|
||||||
|
|
||||||
user_notifications:
|
user_notifications:
|
||||||
previous_discussion: "Previous Replies"
|
previous_discussion: "Previous Replies"
|
||||||
|
reached_limit:
|
||||||
|
one: "WARNING: you reached the limit of daily emails. Further email notifications will be suppressed."
|
||||||
|
other: "WARNING: you reached the limit of %{count} daily emails. Further email notifications will be suppressed."
|
||||||
in_reply_to: "In Reply To"
|
in_reply_to: "In Reply To"
|
||||||
unsubscribe:
|
unsubscribe:
|
||||||
title: "Unsubscribe"
|
title: "Unsubscribe"
|
||||||
|
@ -2363,6 +2367,7 @@ en:
|
||||||
post_deleted: "post was deleted by the author"
|
post_deleted: "post was deleted by the author"
|
||||||
user_suspended: "user was suspended"
|
user_suspended: "user was suspended"
|
||||||
already_read: "user has already read this post"
|
already_read: "user has already read this post"
|
||||||
|
exceeded_limit: "Exceeded max_emails_per_day_per_user"
|
||||||
message_blank: "message is blank"
|
message_blank: "message is blank"
|
||||||
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"
|
||||||
|
|
|
@ -559,6 +559,7 @@ email:
|
||||||
delete_email_logs_after_days:
|
delete_email_logs_after_days:
|
||||||
default: 365
|
default: 365
|
||||||
min: 0
|
min: 0
|
||||||
|
max_emails_per_day_per_user: 100
|
||||||
|
|
||||||
files:
|
files:
|
||||||
max_image_size_kb: 3072
|
max_image_size_kb: 3072
|
||||||
|
|
|
@ -3,9 +3,34 @@ require "rails_helper"
|
||||||
describe Jobs::NotifyMailingListSubscribers do
|
describe Jobs::NotifyMailingListSubscribers do
|
||||||
|
|
||||||
context "with mailing list on" do
|
context "with mailing list on" do
|
||||||
before { SiteSetting.stubs(:default_email_mailing_list_mode).returns(true) }
|
before { SiteSetting.default_email_mailing_list_mode = true }
|
||||||
let(:user) { Fabricate(:user) }
|
let(:user) { Fabricate(:user) }
|
||||||
|
|
||||||
|
context "SiteSetting.max_emails_per_day_per_user" do
|
||||||
|
|
||||||
|
it 'stops sending mail once limit is reached' do
|
||||||
|
SiteSetting.max_emails_per_day_per_user = 2
|
||||||
|
post = Fabricate(:post)
|
||||||
|
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
|
||||||
|
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
|
||||||
|
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context "totally skipped if mailing list mode disabled" do
|
||||||
|
|
||||||
|
it "sends no email to the user" do
|
||||||
|
SiteSetting.disable_mailing_list_mode = true
|
||||||
|
|
||||||
|
post = Fabricate(:post)
|
||||||
|
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
|
||||||
|
expect(EmailLog.count).to eq(0)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context "with a valid post" do
|
context "with a valid post" do
|
||||||
let!(:post) { Fabricate(:post, user: user) }
|
let!(:post) { Fabricate(:post, user: user) }
|
||||||
|
|
||||||
|
|
|
@ -193,6 +193,17 @@ describe Jobs::UserEmail do
|
||||||
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
|
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "does not send notification if limit is reached" do
|
||||||
|
SiteSetting.max_emails_per_day_per_user = 2
|
||||||
|
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
|
||||||
|
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id, post_id: post.id)
|
||||||
|
|
||||||
|
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
|
||||||
|
end
|
||||||
|
|
||||||
it "doesn't send the mail if the user is using mailing list mode" do
|
it "doesn't send the mail if the user is using mailing list mode" do
|
||||||
Email::Sender.any_instance.expects(:send).never
|
Email::Sender.any_instance.expects(:send).never
|
||||||
user.user_option.update_column(:mailing_list_mode, true)
|
user.user_option.update_column(:mailing_list_mode, true)
|
||||||
|
@ -205,7 +216,7 @@ describe Jobs::UserEmail do
|
||||||
# When post does not have a topic
|
# When post does not have a topic
|
||||||
post = Fabricate(:post)
|
post = Fabricate(:post)
|
||||||
post.topic.destroy
|
post.topic.destroy
|
||||||
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_type: "posted", post_id: post)
|
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 email if the post has been user deleted" do
|
it "doesn't send the email if the post has been user deleted" do
|
||||||
|
|
|
@ -243,6 +243,31 @@ describe UserNotifications do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
|
it 'adds a warning when mail limit is reached' do
|
||||||
|
SiteSetting.max_emails_per_day_per_user = 2
|
||||||
|
user = Fabricate(:user)
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id, skipped: false)
|
||||||
|
|
||||||
|
post = Fabricate(:post)
|
||||||
|
reply = Fabricate(:post, topic_id: post.topic_id)
|
||||||
|
|
||||||
|
notification = Fabricate(:notification, topic_id: post.topic_id, post_number: reply.post_number,
|
||||||
|
user: post.user, data: {original_username: 'bob'}.to_json)
|
||||||
|
|
||||||
|
mail = UserNotifications.user_replied(
|
||||||
|
user,
|
||||||
|
post: reply,
|
||||||
|
notification_type: notification.notification_type,
|
||||||
|
notification_data_hash: notification.data_hash
|
||||||
|
)
|
||||||
|
|
||||||
|
# WARNING: you reached the limit of 100 email notifications per day. Further emails will be suppressed.
|
||||||
|
# Consider watching less topics or disabling mailing list mode.
|
||||||
|
expect(mail.html_part.to_s).to match("WARNING: ")
|
||||||
|
expect(mail.body.to_s).to match("WARNING: ")
|
||||||
|
end
|
||||||
|
|
||||||
def expects_build_with(condition)
|
def expects_build_with(condition)
|
||||||
UserNotifications.any_instance.expects(:build_email).with(user.email, condition)
|
UserNotifications.any_instance.expects(:build_email).with(user.email, condition)
|
||||||
mailer = UserNotifications.send(mail_type, user,
|
mailer = UserNotifications.send(mail_type, user,
|
||||||
|
|
|
@ -26,6 +26,21 @@ describe EmailLog do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#reached_max_emails?' do
|
||||||
|
it "tracks when max emails are reached" do
|
||||||
|
SiteSetting.max_emails_per_day_per_user = 2
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id, skipped: true)
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id, created_at: 3.days.ago)
|
||||||
|
|
||||||
|
expect(EmailLog.reached_max_emails?(user)).to eq(false)
|
||||||
|
|
||||||
|
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
|
||||||
|
|
||||||
|
expect(EmailLog.reached_max_emails?(user)).to eq(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#count_per_day' do
|
describe '#count_per_day' do
|
||||||
it "counts sent emails" do
|
it "counts sent emails" do
|
||||||
user.email_logs.create(email_type: 'blah', to_address: user.email)
|
user.email_logs.create(email_type: 'blah', to_address: user.email)
|
||||||
|
|
Loading…
Reference in New Issue