From eb0614c44f0e447d7a4ef0e68658d467783f3a26 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 6 Apr 2025 19:17:24 +0200 Subject: [PATCH] Use a more ergonomic API for watching Passing a boolean vs using explicit on/off methods feel too low level and doesnt read right in most of these cases. --- app/controllers/bubbles/watches_controller.rb | 12 ++++-------- app/models/bubble/assignable.rb | 2 +- app/models/bubble/commentable.rb | 2 +- app/models/bubble/watchable.rb | 15 +++++++-------- .../bubbles/watches_controller_test.rb | 4 ++-- test/models/bubble/assignable_test.rb | 2 +- test/models/bubble/watchable_test.rb | 12 ++++++------ 7 files changed, 22 insertions(+), 27 deletions(-) diff --git a/app/controllers/bubbles/watches_controller.rb b/app/controllers/bubbles/watches_controller.rb index 221f51405..641564889 100644 --- a/app/controllers/bubbles/watches_controller.rb +++ b/app/controllers/bubbles/watches_controller.rb @@ -2,16 +2,12 @@ class Bubbles::WatchesController < ApplicationController include BubbleScoped def create - set_watching_and_redirect(true) + @bubble.watch_by Current.user + redirect_to bucket_bubble_watch_path(@bucket, @bubble) end def destroy - set_watching_and_redirect(false) + @bubble.unwatch_by Current.user + redirect_to bucket_bubble_watch_path(@bucket, @bubble) end - - private - def set_watching_and_redirect(watching) - @bubble.set_watching(Current.user, watching) - redirect_to bucket_bubble_watch_path(@bucket, @bubble) - end end diff --git a/app/models/bubble/assignable.rb b/app/models/bubble/assignable.rb index 98ea3d8ea..c0dd36ff4 100644 --- a/app/models/bubble/assignable.rb +++ b/app/models/bubble/assignable.rb @@ -21,7 +21,7 @@ module Bubble::Assignable private def assign(user) assignments.create! assignee: user, assigner: Current.user - set_watching(user, true) + watch_by user track_event :assigned, assignee_ids: [ user.id ] rescue ActiveRecord::RecordNotUnique diff --git a/app/models/bubble/commentable.rb b/app/models/bubble/commentable.rb index 5b8f630f5..ab1c72a74 100644 --- a/app/models/bubble/commentable.rb +++ b/app/models/bubble/commentable.rb @@ -7,7 +7,7 @@ module Bubble::Commentable def comment_created(comment) increment! :comments_count - set_watching comment.creator, true + watch_by comment.creator track_event :commented, comment_id: comment.id rescore diff --git a/app/models/bubble/watchable.rb b/app/models/bubble/watchable.rb index 8ad47554d..bc4a82ccf 100644 --- a/app/models/bubble/watchable.rb +++ b/app/models/bubble/watchable.rb @@ -5,15 +5,19 @@ module Bubble::Watchable has_many :watches, dependent: :destroy has_many :watchers, -> { merge(Watch.watching) }, through: :watches, source: :user - after_create :set_watching_for_creator + after_create -> { watch_by creator } end def watched_by?(user) watchers_and_subscribers(include_only_watching: true).include?(user) end - def set_watching(user, watching) - watches.where(user: user).first_or_create.update!(watching: watching) + def watch_by(user) + watches.where(user: user).first_or_create.update!(watching: true) + end + + def unwatch_by(user) + watches.where(user: user).first_or_create.update!(watching: false) end def watchers_and_subscribers(include_only_watching: false) @@ -23,9 +27,4 @@ module Bubble::Watchable User.where(id: subscribers.pluck(:id) + watches.watching.pluck(:user_id) - watches.not_watching.pluck(:user_id)) end - - private - def set_watching_for_creator - set_watching(creator, true) - end end diff --git a/test/controllers/bubbles/watches_controller_test.rb b/test/controllers/bubbles/watches_controller_test.rb index 4e1d02958..739149948 100644 --- a/test/controllers/bubbles/watches_controller_test.rb +++ b/test/controllers/bubbles/watches_controller_test.rb @@ -6,7 +6,7 @@ class Bubbles::WatchesControllerTest < ActionDispatch::IntegrationTest end test "create" do - bubbles(:logo).set_watching(users(:kevin), false) + bubbles(:logo).unwatch_by users(:kevin) assert_changes -> { bubbles(:logo).watched_by?(users(:kevin)) }, from: false, to: true do post bucket_bubble_watch_url(buckets(:writebook), bubbles(:logo)) @@ -16,7 +16,7 @@ class Bubbles::WatchesControllerTest < ActionDispatch::IntegrationTest end test "destroy" do - bubbles(:logo).set_watching(users(:kevin), true) + bubbles(:logo).watch_by users(:kevin) assert_changes -> { bubbles(:logo).watched_by?(users(:kevin)) }, from: true, to: false do delete bucket_bubble_watch_url(buckets(:writebook), bubbles(:logo)) diff --git a/test/models/bubble/assignable_test.rb b/test/models/bubble/assignable_test.rb index ae61906f8..56542544c 100644 --- a/test/models/bubble/assignable_test.rb +++ b/test/models/bubble/assignable_test.rb @@ -3,7 +3,7 @@ require "test_helper" class Bubble::AssignableTest < ActiveSupport::TestCase test "assigning a user makes them watch the bubble" do assert_not bubbles(:layout).assigned_to?(users(:kevin)) - bubbles(:layout).set_watching(users(:kevin), false) + bubbles(:layout).unwatch_by users(:kevin) with_current_user(:jz) do bubbles(:layout).toggle_assignment(users(:kevin)) diff --git a/test/models/bubble/watchable_test.rb b/test/models/bubble/watchable_test.rb index 267bbf2d8..aa33b75d2 100644 --- a/test/models/bubble/watchable_test.rb +++ b/test/models/bubble/watchable_test.rb @@ -9,10 +9,10 @@ class Bubble::WatchableTest < ActiveSupport::TestCase test "watched_by?" do assert_not bubbles(:logo).watched_by?(users(:kevin)) - bubbles(:logo).set_watching(users(:kevin), true) + bubbles(:logo).watch_by users(:kevin) assert bubbles(:logo).watched_by?(users(:kevin)) - bubbles(:logo).set_watching(users(:kevin), false) + bubbles(:logo).unwatch_by users(:kevin) assert_not bubbles(:logo).watched_by?(users(:kevin)) end @@ -20,7 +20,7 @@ class Bubble::WatchableTest < ActiveSupport::TestCase buckets(:writebook).access_for(users(:kevin)).watching! assert bubbles(:text).watched_by?(users(:kevin)) - bubbles(:logo).set_watching(users(:kevin), false) + bubbles(:logo).unwatch_by users(:kevin) assert_not bubbles(:logo).watched_by?(users(:kevin)) end @@ -34,9 +34,9 @@ class Bubble::WatchableTest < ActiveSupport::TestCase buckets(:writebook).access_for(users(:kevin)).watching! buckets(:writebook).access_for(users(:jz)).everything! - bubbles(:logo).set_watching(users(:kevin), true) - bubbles(:logo).set_watching(users(:jz), false) - bubbles(:logo).set_watching(users(:david), true) + bubbles(:logo).watch_by users(:kevin) + bubbles(:logo).unwatch_by users(:jz) + bubbles(:logo).watch_by users(:david) assert_equal [ users(:kevin), users(:david) ].sort, bubbles(:logo).watchers_and_subscribers.sort end