From 5369c365927690fdcb9e7757bd8167b3e8840f91 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 27 Jan 2025 19:56:16 -0500 Subject: [PATCH] unreads: Handle updates when there are moved messages Signed-off-by: Zixuan James Li --- lib/model/unreads.dart | 31 +++++- test/model/unreads_test.dart | 183 +++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 4 deletions(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 36da3622b1..78b027fdba 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -260,9 +260,6 @@ class Unreads extends ChangeNotifier { ); // We assume this event can't signal a change in a message's 'read' flag. - // TODO can it actually though, when it's about messages being moved into an - // unsubscribed stream? - // https://chat.zulip.org/#narrow/stream/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/1639957 final bool isRead = event.flags.contains(MessageFlag.read); assert(() { final isUnreadLocally = isUnread(messageId); @@ -296,13 +293,39 @@ class Unreads extends ChangeNotifier { madeAnyUpdate |= mentions.add(messageId); } - // TODO(#901) handle moved messages + madeAnyUpdate |= _handleMessageMove(event); if (madeAnyUpdate) { notifyListeners(); } } + bool _handleMessageMove(UpdateMessageEvent event) { + if (event.moveData == null) { + // No moved messages. + return false; + } + final UpdateMessageMoveData( + :origStreamId, :newStreamId, :origTopic, :newTopic) = event.moveData!; + + final messageToMoveIds = _removeAllInStreamTopic( + event.messageIds.toSet(), origStreamId, origTopic); + if (messageToMoveIds == null || messageToMoveIds.isEmpty) { + // No known unreads affected by move; nothing to do. + return false; + } + + if (!channelStore.subscriptions.containsKey(newStreamId)) { + // Unreads moved to an unsubscribed channel; just drop them. + // See also: + // https://chat.zulip.org/#narrow/channel/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/2101926 + return true; + } + + _addAllInStreamTopic(messageToMoveIds..sort(), newStreamId, newTopic); + return true; + } + void handleDeleteMessageEvent(DeleteMessageEvent event) { mentions.removeAll(event.messageIds); final messageIdsSet = Set.of(event.messageIds); diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 40e074dcaa..df781fec36 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -469,6 +469,189 @@ void main() { } } }); + + group('moves', () { + final origChannel = eg.stream(); + const origTopic = 'origTopic'; + const newTopic = 'newTopic'; + + Future prepareStore() async { + prepare(); + await channelStore.addStream(origChannel); + await channelStore.addSubscription(eg.subscription(origChannel)); + } + + group('move read messages', () { + final readMessages = List.generate(10, + (_) => eg.streamMessage( + stream: origChannel, topic: origTopic, flags: [MessageFlag.read])); + + test('to new channel', () async { + await prepareStore(); + final newChannel = eg.stream(); + await channelStore.addStream(newChannel); + await channelStore.addSubscription(eg.subscription(newChannel)); + fillWithMessages(readMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: readMessages, + newStreamId: newChannel.streamId)); + checkNotNotified(); + checkMatchesMessages([]); + }); + + test('to new topic', () async { + await prepareStore(); + fillWithMessages(readMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: readMessages, + newTopicStr: newTopic)); + checkNotNotified(); + checkMatchesMessages([]); + }); + + test('from topic with unreads', () async { + await prepareStore(); + final unreadMessage = eg.streamMessage( + stream: origChannel, topic: origTopic); + fillWithMessages([...readMessages, unreadMessage]); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: readMessages, + newTopicStr: newTopic)); + checkNotNotified(); + checkMatchesMessages([unreadMessage]); + }); + + test('to topic with unreads', () async { + await prepareStore(); + final unreadMessage = eg.streamMessage( + stream: origChannel, topic: newTopic); + fillWithMessages([...readMessages, unreadMessage]); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: readMessages, + newTopicStr: newTopic, + )); + checkNotNotified(); + checkMatchesMessages([unreadMessage]); + }); + }); + + group('move unread messages', () { + final unreadMessages = List.generate(10, + (_) => eg.streamMessage(stream: origChannel, topic: origTopic)); + + test('to another subscribed channel', () async { + await prepareStore(); + final newChannel = eg.stream(); + await channelStore.addStream(newChannel); + await channelStore.addSubscription(eg.subscription(newChannel)); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newStreamId: newChannel.streamId)); + checkNotifiedOnce(); + checkMatchesMessages([ + for (final message in unreadMessages) + Message.fromJson( + message.toJson()..['stream_id'] = newChannel.streamId), + ]); + }); + + test('to unsubscribed channel', () async { + await prepareStore(); + final newChannel = eg.stream(); + await channelStore.addStream(newChannel); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newStreamId: newChannel.streamId)); + checkNotifiedOnce(); + checkMatchesMessages([]); + }); + + test('to new topic', () async { + await prepareStore(); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + for (final message in unreadMessages) + Message.fromJson(message.toJson()..['subject'] = newTopic), + ]); + }); + + test('from topic containing other unreads', () async { + await prepareStore(); + final unreadMessage = eg.streamMessage( + stream: origChannel, topic: origTopic); + fillWithMessages([...unreadMessages, unreadMessage]); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + for (final message in unreadMessages) + Message.fromJson(message.toJson()..['subject'] = newTopic), + unreadMessage, + ]); + }); + + test('to topic containing other unreads', () async { + await prepareStore(); + final unreadMessage = eg.streamMessage( + stream: origChannel, topic: newTopic); + fillWithMessages([...unreadMessages, unreadMessage]); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + for (final message in unreadMessages) + Message.fromJson(message.toJson()..['subject'] = newTopic), + unreadMessage, + ]); + }); + + test('tolerate unsorted messages', () async { + await prepareStore(); + final unreadMessages = List.generate(10, + (i) => eg.streamMessage(id: 1000-i, stream: origChannel, topic: origTopic)); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + for (final message in unreadMessages) + Message.fromJson(message.toJson()..['subject'] = newTopic) + ]); + }); + + test('tolerate unreads unknown to the model', () async { + await prepareStore(); + final unknownUnreadMessage = eg.streamMessage( + stream: eg.stream(), topic: origTopic); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: [unknownUnreadMessage], + newTopicStr: newTopic)); + checkNotNotified(); + checkMatchesMessages(unreadMessages); + }); + }); + }); });