From cdce060a382c844cf9492ca32ba4b9ad294102a9 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 3 May 2017 13:33:43 +0530 Subject: [PATCH] FIX: don't apply max emails per day per user to forgot password --- app/jobs/regular/user_email.rb | 2 +- app/models/email_log.rb | 8 ++++---- spec/jobs/user_email_spec.rb | 21 ++++++++++++++------- spec/models/email_log_spec.rb | 10 ++++++++-- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 6f316d530e2..c0841b79bb3 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -121,7 +121,7 @@ module Jobs email_args[:email_token] = email_token if email_token.present? email_args[:new_email] = user.email if type.to_s == "notify_old_email" - if EmailLog.reached_max_emails?(user) + if EmailLog.reached_max_emails?(user, type.to_s) return skip_message(I18n.t('email_log.exceeded_emails_limit')) end diff --git a/app/models/email_log.rb b/app/models/email_log.rb index ac93e08ccea..5098fe99be4 100644 --- a/app/models/email_log.rb +++ b/app/models/email_log.rb @@ -28,12 +28,12 @@ class EmailLog < ActiveRecord::Base end end - def self.reached_max_emails?(user) - return false if SiteSetting.max_emails_per_day_per_user == 0 + def self.reached_max_emails?(user, email_type=nil) + return false if SiteSetting.max_emails_per_day_per_user == 0 || email_type == 'forgot_password' count = sent.where('created_at > ?', 1.day.ago) - .where(user_id: user.id) - .count + .where(user_id: user.id) + .count count >= SiteSetting.max_emails_per_day_per_user end diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index 3d84c790d3b..0fae04a156c 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -222,15 +222,22 @@ describe Jobs::UserEmail do Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) end - it "does not send notification if limit is reached" do - SiteSetting.max_emails_per_day_per_user = 2 + context 'max_emails_per_day_per_user limit is reached' do + before 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) + end - 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) + it "does not send notification if limit is reached" do + 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 - 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) + it "sends forgot password email" do + Jobs::UserEmail.new.execute(type: :forgot_password, user_id: user.id, notification_id: notification.id, post_id: post.id) + expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(0) + end end it "does not send notification if bounce threshold is reached" do diff --git a/spec/models/email_log_spec.rb b/spec/models/email_log_spec.rb index 79608a8d8e1..06cc4fe2dd0 100644 --- a/spec/models/email_log_spec.rb +++ b/spec/models/email_log_spec.rb @@ -53,18 +53,24 @@ describe EmailLog do end describe '#reached_max_emails?' do - it "tracks when max emails are reached" do + before 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) + end + it "tracks when max emails are reached" do 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 + + it "returns false for forgot_password email" do + user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id) + expect(EmailLog.reached_max_emails?(user, 'forgot_password')).to eq(false) + end end describe '#count_per_day' do