diff --git a/app/channels/unread_rooms_channel.rb b/app/channels/unread_rooms_channel.rb index f7fcc08..a3f6cc0 100644 --- a/app/channels/unread_rooms_channel.rb +++ b/app/channels/unread_rooms_channel.rb @@ -1,5 +1,12 @@ class UnreadRoomsChannel < ApplicationCable::Channel + # Scoped per user, like ReadRoomsChannel. A single global stream would tell every + # authenticated connection when any room on the account is active, including closed + # rooms and direct conversations they're not part of. + def self.stream_name_for(user_id) + "user_#{user_id}_unreads" + end + def subscribed - stream_from "unread_rooms" + stream_from self.class.stream_name_for(current_user.id) end end diff --git a/app/models/message/broadcasts.rb b/app/models/message/broadcasts.rb index 1f909dc..4623f2a 100644 --- a/app/models/message/broadcasts.rb +++ b/app/models/message/broadcasts.rb @@ -1,10 +1,19 @@ module Message::Broadcasts def broadcast_create broadcast_append_to room, :messages, target: [ room, :messages ] - ActionCable.server.broadcast("unread_rooms", { roomId: room.id }) + broadcast_unread_room end def broadcast_remove broadcast_remove_to room, :messages end + + private + # Fanned out to the room's members rather than published on one global stream, so + # that the timing of activity in a room only reaches people who are in it. + def broadcast_unread_room + room.memberships.pluck(:user_id).each do |user_id| + ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), { roomId: room.id } + end + end end diff --git a/test/channels/unread_rooms_channel_test.rb b/test/channels/unread_rooms_channel_test.rb new file mode 100644 index 0000000..4444eb1 --- /dev/null +++ b/test/channels/unread_rooms_channel_test.rb @@ -0,0 +1,47 @@ +require "test_helper" + +class UnreadRoomsChannelTest < ActionCable::Channel::TestCase + test "streams only the subscriber's own unread stream" do + stub_connection(current_user: users(:jz)) + + subscribe + + assert subscription.confirmed? + assert_has_stream "user_#{users(:jz).id}_unreads" + assert_not_includes subscription.streams, "unread_rooms" + end + + test "an outsider is not told about activity in a room they can't see" do + direct = rooms(:bender_and_kevin) + assert_not direct.users.include?(users(:jz)), "jz must be an outsider for this test to mean anything" + + broadcasts = capture_unread_broadcasts_for(users(:jz)) do + direct.messages.create!(body: "Private", creator: users(:kevin), client_message_id: "outsider").broadcast_create + end + + assert_empty broadcasts + end + + test "a member is told about activity in their own room" do + direct = rooms(:bender_and_kevin) + + broadcasts = capture_unread_broadcasts_for(users(:kevin)) do + direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member").broadcast_create + end + + assert_equal [ direct.id ], broadcasts.collect { |broadcast| broadcast["roomId"] } + end + + private + def capture_unread_broadcasts_for(user) + stub_connection(current_user: user) + subscribe + + stream = subscription.streams.sole + before = ActionCable.server.pubsub.broadcasts(stream).size + + yield + + ActionCable.server.pubsub.broadcasts(stream).drop(before).collect { |broadcast| JSON.parse(broadcast) } + end +end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index 1f556bf..8088694 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -57,9 +57,22 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest end end - test "creating a message broadcasts unread room" do - assert_broadcasts "unread_rooms", 1 do - post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + test "creating a message broadcasts unread room to each member" do + @room.users.each do |member| + assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one #{member.id}", client_message_id: member.id } } + end + end + end + + test "creating a message doesn't broadcast unread room to non-members" do + outsiders = User.where.not(id: @room.users.map(&:id)) + assert outsiders.any?, "need someone outside the room for this test to mean anything" + + outsiders.each do |outsider| + assert_no_broadcasts UnreadRoomsChannel.stream_name_for(outsider.id) do + post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } } + end end end