Reset former members' connections and grant open rooms in one statement

Deleting the memberships with delete_all skips Membership's after_destroy_commit, which resets a member's
connections when one membership is revoked. Until the job ran, a member who had the room open kept its
streams, and a message that still landed in the room (a request already past the membership check, a bot's
reply) reached them. The request now reads the members' ids in the transaction that deletes their
memberships, and the job resets their connections before it destroys the messages. It costs a Redis round
trip per member, about 1.5 s for 10,000, so it's done in the job rather than in the request.

User#grant_membership_to_open_rooms read the open rooms and inserted in a separate statement, so a user
created while a room was being closed could read it as open and be granted it after the close. It's now a
single insert ... select, which SQLite runs under the write lock, skipping duplicates as insert_all did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa

app/jobs/room/destroy_job.rb

@@ -1,7 +1,8 @@
 class Room::DestroyJob < ApplicationJob
   discard_on ActiveJob::DeserializationError
 
-  def perform(room)
+  def perform(room, former_member_ids)
+    room.reset_remote_connections_of(former_member_ids)
     room.destroy_one_message_at_a_time
   end
 end

app/models/room.rb

@@ -51,12 +51,19 @@ class Room < ApplicationRecord
   def destroy_later
     room = open? ? becomes!(Rooms::Closed) : self
 
-    transaction do
+    former_member_ids = transaction do
       room.save!
-      room.memberships.delete_all
+      room.memberships.pluck(:user_id).tap { room.memberships.delete_all }
     end
 
-    Room::DestroyJob.perform_later(room)
+    Room::DestroyJob.perform_later(room, former_member_ids)
+  end
+
+  # Deleting the memberships skips the reset that revoking one does, so the former members get it here, all at once,
+  # before the messages go: their connections stop receiving the room's streams. It's done in the job, not the
+  # request, because it costs a Redis round trip per member.
+  def reset_remote_connections_of(user_ids)
+    User.where(id: user_ids).find_each(&:reset_remote_connections)
   end
 
   # Each message is destroyed in its own transaction, so other writes get through in between. Any message posted

app/models/user.rb

@@ -50,8 +50,18 @@ class User < ApplicationRecord
   end
 
   private
+    # One statement, which SQLite runs under the write lock, so no room is closed between reading the open ones and
+    # inserting. A room that Room#destroy_later closes is either closed before, and skipped, or after, and takes this
+    # membership away with the others. A room that granted itself to every user when it was created is skipped too,
+    # as insert_all did.
     def grant_membership_to_open_rooms
-      Membership.insert_all(Rooms::Open.pluck(:id).collect { |room_id| { room_id: room_id, user_id: id } })
+      now = Time.current
+
+      Membership.connection.execute Membership.sanitize_sql([ <<~SQL, id, now, now ])
+        insert into memberships (room_id, user_id, created_at, updated_at)
+        select id, ?, ?, ? from rooms where type = 'Rooms::Open'
+        on conflict do nothing
+      SQL
     end
 
     def deactived_email_address

test/controllers/rooms_controller_test.rb

@@ -90,8 +90,9 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest
 
   test "destroy takes the room away from its members at once and leaves its messages to a job" do
     room = rooms(:designers)
+    member_ids = room.memberships.pluck(:user_id)
 
-    assert_enqueued_with(job: Room::DestroyJob, args: [ room ]) do
+    assert_enqueued_with(job: Room::DestroyJob, args: [ room, member_ids ]) do
       assert_no_difference -> { Message.count } do
         delete room_url(room)
         assert_redirected_to root_url

test/models/room_test.rb

@@ -1,6 +1,8 @@
 require "test_helper"
 
 class RoomTest < ActiveSupport::TestCase
+  include ActionCable::TestHelper
+
   test "grant membership to user" do
     rooms(:watercooler).memberships.grant_to(users(:kevin))
     assert rooms(:watercooler).users.include?(users(:kevin))
@@ -41,6 +43,19 @@ class RoomTest < ActiveSupport::TestCase
     assert_not Room.exists?(room.id)
   end
 
+  test "a room destroyed later resets the connections of its former members, as revoking their memberships would" do
+    room = rooms(:designers)
+    former_members = room.users.to_a
+    assert former_members.many?
+
+    room.destroy_later
+    perform_enqueued_jobs only: Room::DestroyJob
+
+    former_members.each do |member|
+      assert_broadcast_on "action_cable/#{member.to_gid_param}", { type: "disconnect", reconnect: true }
+    end
+  end
+
   test "destroying one message at a time leaves nothing of the room behind" do
     room = rooms(:designers)
     searchable = room.messages.create!(body: "Kept in the search index", creator: users(:david))

test/models/user_test.rb

@@ -12,6 +12,18 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  test "memberships granted to the open rooms are like any other" do
+    freeze_time
+    user = create_new_user
+
+    assert_equal Rooms::Open.ids.sort, user.memberships.pluck(:room_id).sort
+    user.memberships.each do |membership|
+      assert membership.involved_in_mentions?
+      assert_equal Time.current, membership.created_at
+      assert_equal Time.current, membership.updated_at
+    end
+  end
+
   test "deactivating a user deletes push subscriptions, searches, memberships for non-direct rooms, and changes their email address" do
     assert_difference -> { Membership.count }, -users(:david).memberships.without_direct_rooms.count do
     assert_difference -> { Push::Subscription.count }, -users(:david).push_subscriptions.count do