FIX: Don't send notification email when user isn't allowed to see topic

FIX: Don’t send notification email when user isn’t allowed to see topic

diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb
index 3c36f32..447872f 100644
--- a/app/jobs/regular/user_email.rb
+++ b/app/jobs/regular/user_email.rb
@@ -41,6 +41,10 @@ module Jobs
         unless post.present?
           return skip(SkippedEmailLog.reason_types[:user_email_post_not_found])
         end
+
+        if !Guardian.new(user).can_see?(post)
+          return skip(SkippedEmailLog.reason_types[:user_email_access_denied])
+        end
       end
 
       if args[:notification_id].present?
diff --git a/app/models/skipped_email_log.rb b/app/models/skipped_email_log.rb
index 90c5e15..374e8cf 100644
--- a/app/models/skipped_email_log.rb
+++ b/app/models/skipped_email_log.rb
@@ -35,7 +35,8 @@ class SkippedEmailLog < ActiveRecord::Base
       sender_text_part_body_blank: 18,
       sender_body_blank: 19,
       sender_post_deleted: 20,
-      sender_message_to_invalid: 21
+      sender_message_to_invalid: 21,
+      user_email_access_denied: 22
       # you need to add the reason in server.en.yml below the "skipped_email_log" key
       # when you add a new enum value
     )
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index c38eb10..9c231b9 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -939,7 +939,7 @@ en:
     description: '"%{application_name}" is requesting the following access to your account:'
     instructions: 'We just generated a new user API key for you to use with "%{application_name}", please paste the following key into your application:'
     otp_description: 'Would you like to allow "%{application_name}" to access this site?'
-    otp_confirmation: 
+    otp_confirmation:
       confirm_title: Continue to %{site_name}
       logging_in_as: Logging in as %{username}
       confirm_button: Finish Login
@@ -3560,6 +3560,7 @@ en:
     user_email_post_deleted: "post was deleted by the author"
     user_email_user_suspended: "user was suspended"
     user_email_already_read: "user has already read this post"
+    user_email_access_denied: "user is not allowed to see this post"
     sender_message_blank: "message is blank"
     sender_message_to_blank: "message.to is blank"
     sender_text_part_body_blank: "text_part.body is blank"
diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb
index a111720..33c32e0 100644
--- a/spec/jobs/user_email_spec.rb
+++ b/spec/jobs/user_email_spec.rb
@@ -235,6 +235,24 @@ describe Jobs::UserEmail do
       expect(user.last_emailed_at).to eq(last_emailed_at)
     end
 
+    it "creates a skipped email log when the usere isn't allowed to see the post" do
+      user.user_option.update(email_level: UserOption.email_level_types[:always])
+      post.topic.convert_to_private_message(Discourse.system_user)
+
+      expect do
+        Jobs::UserEmail.new.execute(type: :user_posted, user_id: user.id, post_id: post.id)
+      end.to change { SkippedEmailLog.count }.by(1)
+
+      expect(SkippedEmailLog.exists?(
+        email_type: "user_posted",
+        user: user,
+        post: post,
+        to_address: user.email,
+        reason_type: SkippedEmailLog.reason_types[:user_email_access_denied]
+      )).to eq(true)
+
+      expect(ActionMailer::Base.deliveries).to eq([])
+    end
   end
 
   context 'args' do

GitHub sha: d513c28e

This commit has been mentioned on Discourse Meta. There might be relevant details there:

Small typo “usere” => “user

1 Like

I think we can approve this knowing @gschlager is going to fix the typo in a follow up. Also it’s in a rspec description which is the lowest priority I can imagine!

3 Likes