Merge pull request #306 from basecamp/quieter-notifs

Quieter notifs

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

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

app/models/notifier/published.rb

@@ -1,2 +1,6 @@
 class Notifier::Published < Notifier
+  private
+    def recipients
+      bubble.watchers_and_subscribers.without(creator)
+    end
 end

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

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 %>

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
 

test/fixtures/subscriptions.yml

@@ -1,3 +1,7 @@
 writebook_kevin:
   subscribable: writebook (Bucket)
   user: kevin
+
+writebook_jz:
+  subscribable: writebook (Bucket)
+  user: jz

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

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