From f1d76a7cb7412429a00cbaed38f1b4399bf2a7be Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 17 Sep 2025 13:15:07 +0200 Subject: [PATCH] Exclude inactive users The system user should never be a subsriber but we should not serve it if it is --- app/models/card/watchable.rb | 11 +++++++---- app/models/user/accessor.rb | 2 +- test/models/card/watchable_test.rb | 4 ++++ test/models/user/accessor_test.rb | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 test/models/user/accessor_test.rb diff --git a/app/models/card/watchable.rb b/app/models/card/watchable.rb index 883975398..4d198ae48 100644 --- a/app/models/card/watchable.rb +++ b/app/models/card/watchable.rb @@ -21,10 +21,13 @@ module Card::Watchable end def watchers_and_subscribers(include_only_watching: false) - involvements = include_only_watching ? [ :watching ] : [] - subscribers = collection.users.where(accesses: { involvement: involvements }) - - User.where(id: subscribers.pluck(:id) + + User.active.where(id: subscribers(include_only_watching:).pluck(:id) + watches.watching.pluck(:user_id) - watches.not_watching.pluck(:user_id)) end + + private + def subscribers(include_only_watching: false) + involvements = include_only_watching ? [ :watching ] : [] + collection.users.where(accesses: { involvement: involvements }) + end end diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index f2c366d04..c538c87a4 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -7,7 +7,7 @@ module User::Accessor has_many :accessible_cards, through: :collections, source: :cards has_many :accessible_comments, through: :accessible_cards, source: :comments - after_create_commit :grant_access_to_collections + after_create_commit :grant_access_to_collections, unless: :system? end private diff --git a/test/models/card/watchable_test.rb b/test/models/card/watchable_test.rb index 2c0328026..b8622fd9e 100644 --- a/test/models/card/watchable_test.rb +++ b/test/models/card/watchable_test.rb @@ -39,5 +39,9 @@ class Card::WatchableTest < ActiveSupport::TestCase cards(:logo).watch_by users(:david) assert_equal [ users(:kevin), users(:david) ].sort, cards(:logo).watchers_and_subscribers.sort + + # Only active users + users(:david).system! + assert_equal [ users(:kevin) ].sort, cards(:logo).watchers_and_subscribers.sort end end diff --git a/test/models/user/accessor_test.rb b/test/models/user/accessor_test.rb new file mode 100644 index 000000000..668a97dda --- /dev/null +++ b/test/models/user/accessor_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class User::AccessorTest < ActiveSupport::TestCase + test "new users get added to all_access collections on creation" do + regular_user = User.create!(name: "Jorge", email_address: "testregular@example.com", password: "secret123456") + + assert_includes regular_user.collections, collections(:writebook) + assert_equal Collection.all_access.count, regular_user.collections.count + end + + test "system user does not get added to collections on creation" do + system_user = User.create!(role: "system", name: "Test System User") + assert_empty system_user.collections + end +end