From bc5e32e737506f88cad82728fa7f8a769452e49f Mon Sep 17 00:00:00 2001
From: Kevin McConnell <kevin@37signals.com>
Date: Tue, 18 Mar 2025 17:32:06 +0000
Subject: [PATCH 1/3] Only "published" notifies collection subscribers

The other notifications are only sent to the watchers of the bubble.
---
 app/models/bubble/watchable.rb       | 1 +
 app/models/notifier.rb               | 2 +-
 app/models/notifier/published.rb     | 4 ++++
 app/models/user.rb                   | 2 +-
 app/views/bubbles/_messages.html.erb | 2 +-
 5 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/models/bubble/watchable.rb b/app/models/bubble/watchable.rb
index 05e5f9dbd..eff70c96f 100644
--- a/app/models/bubble/watchable.rb
+++ b/app/models/bubble/watchable.rb
@@ -3,6 +3,7 @@ module Bubble::Watchable
 
   included do
     has_many :watches, dependent: :destroy
+    has_many :watchers, ->{ merge(Watch.watching) }, through: :watches, source: :user
 
     after_create :set_watching_for_creator
   end
diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index 9c89d6bdf..10b1ba3c2 100644
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -27,7 +27,7 @@ class Notifier
     end
 
     def recipients
-      bubble.watchers_and_subscribers.without(creator)
+      bubble.watchers.without(creator)
     end
 
     def resource
diff --git a/app/models/notifier/published.rb b/app/models/notifier/published.rb
index d5b50b87b..d43b8c8e3 100644
--- a/app/models/notifier/published.rb
+++ b/app/models/notifier/published.rb
@@ -1,2 +1,6 @@
 class Notifier::Published < Notifier
+  private
+    def recipients
+      bubble.watchers_and_subscribers.without(creator)
+    end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index a79e4729e..57a8e636c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -31,7 +31,7 @@ class User < ApplicationRecord
   after_create_commit :grant_access_to_buckets
 
   scope :alphabetically, -> { order("lower(name)") }
-  scope :sorted_with_user_first, ->(user) { order(Arel.sql("id != ?, lower(name)", user.id)) }
+  scope :sorted_with_user_first, ->(user) { order(Arel.sql("users.id != ?, lower(name)", user.id)) }
 
   def initials
     name.to_s.scan(/\b\p{L}/).join.upcase
diff --git a/app/views/bubbles/_messages.html.erb b/app/views/bubbles/_messages.html.erb
index 5ddce595a..5a8eeea0e 100644
--- a/app/views/bubbles/_messages.html.erb
+++ b/app/views/bubbles/_messages.html.erb
@@ -7,7 +7,7 @@
   <div class="flex flex-column gap-half justify-start full-width txt-align-start">
     <span>Watching this…</span>
     <div class="flex align-center flex-wrap gap-half max-width">
-      <%= render partial: "bubbles/watches/watcher", collection: bubble.watchers_and_subscribers.sorted_with_user_first(Current.user) %>
+      <%= render partial: "bubbles/watches/watcher", collection: bubble.watchers.sorted_with_user_first(Current.user) %>
     </div>
   </div>
 <% end %>

From e9ae2772e41d919a983ec9487ab64cc94bf7b535 Mon Sep 17 00:00:00 2001
From: Kevin McConnell <kevin@37signals.com>
Date: Tue, 18 Mar 2025 17:45:47 +0000
Subject: [PATCH 2/3] Add tests to validate notification recipients

---
 .../buckets/subscriptions_controller_test.rb           |  8 ++++----
 test/fixtures/subscriptions.yml                        |  4 ++++
 test/fixtures/watches.yml                              | 10 ----------
 test/models/notifier_test.rb                           |  6 ++++++
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/test/controllers/buckets/subscriptions_controller_test.rb b/test/controllers/buckets/subscriptions_controller_test.rb
index a884db57c..ccd019145 100644
--- a/test/controllers/buckets/subscriptions_controller_test.rb
+++ b/test/controllers/buckets/subscriptions_controller_test.rb
@@ -2,11 +2,11 @@ require "test_helper"
 
 class Buckets::SubscriptionsControllerTest < ActionDispatch::IntegrationTest
   setup do
-    sign_in_as :jz
+    sign_in_as :david
   end
 
   test "subscribing to a bucket" do
-    assert_changes -> { buckets(:writebook).subscribed_by?(users(:jz)) }, from: false, to: true do
+    assert_changes -> { buckets(:writebook).subscribed_by?(users(:david)) }, from: false, to: true do
       put bucket_subscriptions_url(buckets(:writebook)), params: { subscribed: "1" }
     end
 
@@ -14,9 +14,9 @@ class Buckets::SubscriptionsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "unsubscribing from a bucket" do
-    buckets(:writebook).subscribe(users(:jz))
+    buckets(:writebook).subscribe(users(:david))
 
-    assert_changes -> { buckets(:writebook).subscribed_by?(users(:jz)) }, from: true, to: false do
+    assert_changes -> { buckets(:writebook).subscribed_by?(users(:david)) }, from: true, to: false do
       put bucket_subscriptions_url(buckets(:writebook))
     end
 
diff --git a/test/fixtures/subscriptions.yml b/test/fixtures/subscriptions.yml
index 36ab19a7e..6ca23ecf3 100644
--- a/test/fixtures/subscriptions.yml
+++ b/test/fixtures/subscriptions.yml
@@ -1,3 +1,7 @@
 writebook_kevin:
   subscribable: writebook (Bucket)
   user: kevin
+
+writebook_jz:
+  subscribable: writebook (Bucket)
+  user: jz
diff --git a/test/fixtures/watches.yml b/test/fixtures/watches.yml
index 470fbc362..517505c8d 100644
--- a/test/fixtures/watches.yml
+++ b/test/fixtures/watches.yml
@@ -3,11 +3,6 @@ logo_david:
   user: david
   watching: true
 
-logo_jz:
-  bubble: logo
-  user: jz
-  watching: true
-
 logo_kevin:
   bubble: logo
   user: kevin
@@ -18,11 +13,6 @@ layout_david:
   user: david
   watching: true
 
-layout_jz:
-  bubble: layout
-  user: jz
-  watching: true
-
 layout_kevin:
   bubble: layout
   user: kevin
diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb
index 9e41ab3f4..df6faa127 100644
--- a/test/models/notifier_test.rb
+++ b/test/models/notifier_test.rb
@@ -23,6 +23,12 @@ class NotifierTest < ActiveSupport::TestCase
   end
 
   test "creates a notification for each watcher, other than the event creator" do
+    notifications = Notifier.for(events(:layout_commented)).generate
+
+    assert_equal [ users(:kevin) ], notifications.map(&:user)
+  end
+
+  test "the published event creates notifications for subscribers as well as watchers" do
     notifications = Notifier.for(events(:logo_published)).generate
 
     assert_equal users(:kevin, :jz).sort, notifications.map(&:user).sort

From 4216b1123e0d299a799efd83c7065b7691566fc7 Mon Sep 17 00:00:00 2001
From: Kevin McConnell <kevin@37signals.com>
Date: Tue, 18 Mar 2025 17:53:32 +0000
Subject: [PATCH 3/3] Formatting

---
 app/models/bubble/watchable.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/bubble/watchable.rb b/app/models/bubble/watchable.rb
index eff70c96f..efd2c5cb5 100644
--- a/app/models/bubble/watchable.rb
+++ b/app/models/bubble/watchable.rb
@@ -3,7 +3,7 @@ module Bubble::Watchable
 
   included do
     has_many :watches, dependent: :destroy
-    has_many :watchers, ->{ merge(Watch.watching) }, through: :watches, source: :user
+    has_many :watchers, -> { merge(Watch.watching) }, through: :watches, source: :user
 
     after_create :set_watching_for_creator
   end