From 0119a2f9800de5a17d4141a029a1bac21c491fd0 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 15 Apr 2016 15:59:01 +1000
Subject: [PATCH] FIX: only ever send users 1 email per post

in the past ninja editing a post to add a mention could trigger duplicate
emails to a user (and a few other edge cases)
---
 .../notify_mailing_list_subscribers.rb        |  6 ++++-
 app/jobs/regular/user_email.rb                |  4 ++-
 app/models/email_log.rb                       | 14 ++++++++++
 spec/models/email_log_spec.rb                 | 26 +++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/app/jobs/regular/notify_mailing_list_subscribers.rb b/app/jobs/regular/notify_mailing_list_subscribers.rb
index e24aa6ae6ef..38cb079a3dc 100644
--- a/app/jobs/regular/notify_mailing_list_subscribers.rb
+++ b/app/jobs/regular/notify_mailing_list_subscribers.rb
@@ -48,7 +48,11 @@ module Jobs
               )
             else
               message = UserNotifications.mailing_list_notify(user, post)
-              Email::Sender.new(message, :mailing_list, user).send if message
+              if message
+                EmailLog.unique_email_per_post(post, user) do
+                  Email::Sender.new(message, :mailing_list, user).send
+                end
+              end
             end
           rescue => e
             Discourse.handle_job_exception(e, error_context(args, "Sending post to mailing list subscribers", { user_id: user.id, user_email: user.email }))
diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb
index 160629dfefe..bfdc6fa9f9c 100644
--- a/app/jobs/regular/user_email.rb
+++ b/app/jobs/regular/user_email.rb
@@ -117,7 +117,9 @@ module Jobs
         return skip_message(I18n.t('email_log.exceeded_limit'))
       end
 
-      message = UserNotifications.send(type, user, email_args)
+      message = EmailLog.unique_email_per_post(post, user) do
+        UserNotifications.send(type, user, email_args)
+      end
 
       # Update the to address if we have a custom one
       if message && to_address.present?
diff --git a/app/models/email_log.rb b/app/models/email_log.rb
index eb6c1a31738..7d2eba5fd2b 100644
--- a/app/models/email_log.rb
+++ b/app/models/email_log.rb
@@ -1,3 +1,5 @@
+require_dependency 'distributed_mutex'
+
 class EmailLog < ActiveRecord::Base
   belongs_to :user
   belongs_to :post
@@ -13,6 +15,18 @@ class EmailLog < ActiveRecord::Base
     User.where(id: user_id).update_all("last_emailed_at = CURRENT_TIMESTAMP") if user_id.present? && !skipped
   end
 
+  def self.unique_email_per_post(post, user)
+    return yield unless post && user
+
+    DistributedMutex.synchronize("email_log_#{post.id}_#{user.id}") do
+      if where(post_id: post.id, user_id: user.id, skipped: false).exists?
+        nil
+      else
+        yield
+      end
+    end
+  end
+
   def self.reached_max_emails?(user)
     return false if SiteSetting.max_emails_per_day_per_user == 0
 
diff --git a/spec/models/email_log_spec.rb b/spec/models/email_log_spec.rb
index 3b5ca7df465..79608a8d8e1 100644
--- a/spec/models/email_log_spec.rb
+++ b/spec/models/email_log_spec.rb
@@ -8,6 +8,32 @@ describe EmailLog do
 
   let(:user) { Fabricate(:user) }
 
+  context 'unique email per post' do
+    it 'only allows through one email per post' do
+      post = Fabricate(:post)
+      user = post.user
+
+      # skipped emails do not matter
+      user.email_logs.create(email_type: 'blah', post_id: post.id, to_address: user.email, user_id: user.id, skipped: true)
+
+
+      ran = EmailLog.unique_email_per_post(post, user) do
+        true
+      end
+
+      expect(ran).to eq(true)
+
+      user.email_logs.create(email_type: 'blah', post_id: post.id, to_address: user.email, user_id: user.id)
+
+      ran = EmailLog.unique_email_per_post(post, user) do
+        true
+      end
+
+      expect(ran).to be_falsy
+
+    end
+  end
+
   context 'after_create' do
     context 'with user' do
       it 'updates the last_emailed_at value for the user' do