From af4f94c4bd85ca1faaad9fcf16c07c9ebaad13ee Mon Sep 17 00:00:00 2001 From: Marcello Costagliola Date: Wed, 7 Oct 2026 16:41:20 +0200 Subject: [PATCH] Leave members who are unread already out of a new message's update Every message rewrote the row of every disconnected member of the room, including the ones who were unread already and stay unread. Open rooms take in the whole account, so in steady state that is every member on every message: 325 WAL pages per post at 10,000 members, against 16 when only the members who had read the room are written. Directs keep touching all their members: a direct's sidebar row is cached by its membership and carries the room's recency, so it has to be refreshed on every message. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017dFSDHrgrLoBELwjV3Qunq --- app/models/room.rb | 7 +++- .../users/sidebars_controller_test.rb | 36 +++++++++++++++++++ test/models/room_test.rb | 14 ++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/app/models/room.rb b/app/models/room.rb index ed7280d..aaca7d5 100644 --- a/app/models/room.rb +++ b/app/models/room.rb @@ -104,8 +104,13 @@ class Room < ApplicationRecord end end + # Rewriting every member on every message is most of what posting to a large room writes, + # so members who are unread already stay as they are. Directs keep touching them all: a + # direct's sidebar row is cached by membership and shows the room's recency. def unread_memberships(message) - memberships.visible.disconnected.where.not(user: message.creator).update_all(unread_at: message.created_at, updated_at: Time.current) + recipients = memberships.visible.disconnected.where.not(user: message.creator) + recipients = recipients.where(unread_at: nil) unless direct? + recipients.update_all(unread_at: message.created_at, updated_at: Time.current) end def push_later(message) diff --git a/test/controllers/users/sidebars_controller_test.rb b/test/controllers/users/sidebars_controller_test.rb index c563541..dd8ad03 100644 --- a/test/controllers/users/sidebars_controller_test.rb +++ b/test/controllers/users/sidebars_controller_test.rb @@ -28,6 +28,23 @@ class Users::SidebarsControllerTest < ActionDispatch::IntegrationTest assert_select ".unread", count: users(:david).memberships.reject { |m| m.room.direct? || !m.unread? }.count end + test "a cached direct that is already unread still sorts by its latest message" do + room = rooms(:david_and_jason) + + with_memory_cache do + travel_to 1.hour.ago do + room.messages.create! client_message_id: 998, body: "First", creator: users(:jason) + end + get user_sidebar_url + assert memberships(:david_david_and_jason).reload.unread? + + room.messages.create! client_message_id: 999, body: "Second", creator: users(:jason) + get user_sidebar_url + + assert_select "##{dom_id(room, :list)}[data-sorted-list-number=?]", room.reload.updated_at.to_fs(:epoch) + end + end + test "directs are ordered by room recency, not name" do older = rooms(:david_and_jason) newer = rooms(:david_and_kevin) @@ -48,4 +65,23 @@ class Users::SidebarsControllerTest < ActionDispatch::IntegrationTest assert positions.all? assert_equal positions.sort, positions end + + private + def with_memory_cache + old_cache = Rails.cache + old_collection_cache = ActionView::PartialRenderer.collection_cache + old_controller_cache = Users::SidebarsController.cache_store + old_caching = Users::SidebarsController.perform_caching + + Rails.cache = ActiveSupport::Cache::MemoryStore.new + ActionView::PartialRenderer.collection_cache = Rails.cache + Users::SidebarsController.cache_store = Rails.cache + Users::SidebarsController.perform_caching = true + yield + ensure + Rails.cache = old_cache + ActionView::PartialRenderer.collection_cache = old_collection_cache + Users::SidebarsController.cache_store = old_controller_cache + Users::SidebarsController.perform_caching = old_caching + end end diff --git a/test/models/room_test.rb b/test/models/room_test.rb index 09771f3..306a38f 100644 --- a/test/models/room_test.rb +++ b/test/models/room_test.rb @@ -25,6 +25,20 @@ class RoomTest < ActiveSupport::TestCase assert room.users.include?(users(:david)) end + test "a new message marks unread the members who had read the room and leaves the unread ones as they were" do + room = rooms(:watercooler) + reader = memberships(:jason_watercooler) + behind = memberships(:bender_watercooler) + behind.update_columns unread_at: 1.hour.ago, updated_at: 1.hour.ago + behind_before = behind.reload.attributes.slice("unread_at", "updated_at") + + message = room.messages.create! creator: users(:david), body: "Hello", client_message_id: "unread-once" + + assert_equal message.created_at, reader.reload.unread_at + assert_equal behind_before, behind.reload.attributes.slice("unread_at", "updated_at") + assert_not memberships(:david_watercooler).reload.unread? + end + test "type" do assert Rooms::Open.new.open? assert_not Rooms::Open.new.direct?