From 4221f8cee900317eb175b29a35f46f3d75371475 Mon Sep 17 00:00:00 2001
From: Karina Kwiatek <kjk@kjk.dog>
Date: Wed, 29 Mar 2023 15:35:16 +0200
Subject: [PATCH] Fix incorrect user being notified and mutes not being
 respected

---
 app/models/subscription.rb       |  9 ++++++++-
 spec/models/subscription_spec.rb | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 spec/models/subscription_spec.rb

diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index b7971a98..66520203 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -28,7 +28,14 @@ class Subscription < ApplicationRecord
     def notify(source, target)
       return nil if source.nil? || target.nil?
 
-      notifications = Subscription.where(answer: target).where.not(user: target.user).map do |s|
+      muted_by = Relationships::Mute.where(target: source.user).pluck(&:source_id)
+
+      # As we will need to notify for each person subscribed,
+      # it's much faster to bulk insert than to use +Notification.notify+
+      notifications = Subscription.where(answer: target)
+                                  .where.not(user: source.user)
+                                  .where.not(user_id: muted_by)
+                                  .map do |s|
         { target_id: source.id, target_type: Comment, recipient_id: s.user_id, new: true, type: Notification::Commented }
       end
 
diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb
new file mode 100644
index 00000000..eac76a29
--- /dev/null
+++ b/spec/models/subscription_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe Subscription do
+  describe "singleton object" do
+    describe "#notify" do
+      subject { Subscription.notify(source, target) }
+
+      context "answer with one comment" do
+        let(:answer_author) { FactoryBot.create(:user) }
+        let(:answer) { FactoryBot.create(:answer, user: answer_author) }
+        let(:commenter) { FactoryBot.create(:user) }
+        let!(:comment) { FactoryBot.create(:comment, answer:, user: commenter) }
+        let(:source) { comment }
+        let(:target) { answer }
+
+        it "notifies the target about source" do
+          # The method we're testing here is already called the +after_create+ of +Comment+ so there already is a notification
+          expect { subject }.to change { Notification.count }.from(1).to(2)
+          created = Notification.order(:created_at).first!
+          expect(created.target).to eq(comment)
+          expect(created.recipient).to eq(answer_author)
+        end
+      end
+    end
+  end
+end