diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 9faa3d367e..985f5b5803 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -718,16 +718,8 @@ class UpdateMessageEvent extends Event { // final String? streamName; // ignore - @JsonKey(name: 'stream_id') - final int? origStreamId; - final int? newStreamId; - - final PropagateMode? propagateMode; - - @JsonKey(name: 'orig_subject') - final TopicName? origTopic; - @JsonKey(name: 'subject') - final TopicName? newTopic; + @JsonKey(readValue: _readMoveData, fromJson: UpdateMessageMoveData.tryParseFromJson, includeToJson: false) + final UpdateMessageMoveData? moveData; // final List topicLinks; // TODO handle @@ -747,11 +739,7 @@ class UpdateMessageEvent extends Event { required this.messageIds, required this.flags, required this.editTimestamp, - required this.origStreamId, - required this.newStreamId, - required this.propagateMode, - required this.origTopic, - required this.newTopic, + required this.moveData, required this.origContent, required this.origRenderedContent, required this.content, @@ -759,6 +747,12 @@ class UpdateMessageEvent extends Event { required this.isMeMessage, }); + static Map _readMoveData(Map json, String key) { + // Parsing [UpdateMessageMoveData] requires `json`, not the default `json[key]`. + assert(json is Map); // value came through `fromJson` with this type + return json as Map; + } + factory UpdateMessageEvent.fromJson(Map json) => _$UpdateMessageEventFromJson(json); @@ -766,6 +760,72 @@ class UpdateMessageEvent extends Event { Map toJson() => _$UpdateMessageEventToJson(this); } +/// Data structure representing a message move. +class UpdateMessageMoveData { + final int origStreamId; + final int newStreamId; + final TopicName origTopic; + final TopicName newTopic; + final PropagateMode propagateMode; + + UpdateMessageMoveData({ + required this.origStreamId, + required this.newStreamId, + required this.origTopic, + required this.newTopic, + required this.propagateMode, + }) : assert(newStreamId != origStreamId || newTopic != origTopic); + + /// Try to extract [UpdateMessageMoveData] from the JSON object for an + /// [UpdateMessageEvent]. + /// + /// Returns `null` if there was no message move. + /// + /// Throws an error if the data is malformed. + // When parsing this, 'stream_id', which is also present when there was only + // a content edit, cannot be recovered if this ends up returning `null`. + // This may matter if we ever need 'stream_id' when no message move occurred. + static UpdateMessageMoveData? tryParseFromJson(Map json) { + final origStreamId = (json['stream_id'] as num?)?.toInt(); + final newStreamIdRaw = (json['new_stream_id'] as num?)?.toInt(); + final newStreamId = newStreamIdRaw ?? origStreamId; + + final origTopic = json['orig_subject'] == null ? null + : TopicName.fromJson(json['orig_subject'] as String); + final newTopicRaw = json['subject'] == null ? null + : TopicName.fromJson(json['subject'] as String); + final newTopic = newTopicRaw ?? origTopic; + + final propagateModeString = json['propagate_mode'] as String?; + final propagateMode = propagateModeString == null ? null + : PropagateMode.fromRawString(propagateModeString); + + if (newStreamId == origStreamId && newTopic == origTopic) { + if (propagateMode != null) { + throw FormatException( + 'Malformed UpdateMessageEvent: incoherent message-move fields; ' + 'propagate_mode present but no new channel or topic'); + } + return null; + } + + return UpdateMessageMoveData( + // The `stream_id` field (aka origStreamId) is documented to be present on moves; + // newStreamId should not be null either because it falls back to origStreamId. + origStreamId: origStreamId!, + newStreamId: newStreamId!, + + // The `orig_subject` field (aka origTopic) is documented to be present on moves; + // newTopic should not be null either because it falls back to origTopic. + origTopic: origTopic!, + newTopic: newTopic!, + + // The `propagate_mode` field (aka propagateMode) is documented to be present on moves. + propagateMode: propagateMode!, + ); + } +} + /// A Zulip event of type `delete_message`: https://zulip.com/api/get-events#delete_message @JsonSerializable(fieldRename: FieldRename.snake) class DeleteMessageEvent extends Event { diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index d0b0cc7b1b..c5b56d013a 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -443,20 +443,10 @@ UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => .map((e) => $enumDecode(_$MessageFlagEnumMap, e)) .toList(), editTimestamp: (json['edit_timestamp'] as num?)?.toInt(), - origStreamId: (json['stream_id'] as num?)?.toInt(), - newStreamId: (json['new_stream_id'] as num?)?.toInt(), - propagateMode: $enumDecodeNullable( - _$PropagateModeEnumMap, - json['propagate_mode'], + moveData: UpdateMessageMoveData.tryParseFromJson( + UpdateMessageEvent._readMoveData(json, 'move_data') + as Map, ), - origTopic: - json['orig_subject'] == null - ? null - : TopicName.fromJson(json['orig_subject'] as String), - newTopic: - json['subject'] == null - ? null - : TopicName.fromJson(json['subject'] as String), origContent: json['orig_content'] as String?, origRenderedContent: json['orig_rendered_content'] as String?, content: json['content'] as String?, @@ -474,11 +464,6 @@ Map _$UpdateMessageEventToJson(UpdateMessageEvent instance) => 'message_ids': instance.messageIds, 'flags': instance.flags, 'edit_timestamp': instance.editTimestamp, - 'stream_id': instance.origStreamId, - 'new_stream_id': instance.newStreamId, - 'propagate_mode': instance.propagateMode, - 'orig_subject': instance.origTopic, - 'subject': instance.newTopic, 'orig_content': instance.origContent, 'orig_rendered_content': instance.origRenderedContent, 'content': instance.content, @@ -497,12 +482,6 @@ const _$MessageFlagEnumMap = { MessageFlag.unknown: 'unknown', }; -const _$PropagateModeEnumMap = { - PropagateMode.changeOne: 'change_one', - PropagateMode.changeLater: 'change_later', - PropagateMode.changeAll: 'change_all', -}; - DeleteMessageEvent _$DeleteMessageEventFromJson(Map json) => DeleteMessageEvent( id: (json['id'] as num).toInt(), diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index fad8ddc5bc..5c94626584 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -954,4 +954,15 @@ enum PropagateMode { changeAll; String toJson() => _$PropagateModeEnumMap[this]!; + + /// Get a [PropagateMode] from a raw string. Throws if the string is + /// unrecognized. + /// + /// Example: + /// 'change_one' -> PropagateMode.changeOne + static PropagateMode fromRawString(String raw) => _byRawString[raw]!; + + // _$…EnumMap is thanks to `alwaysCreate: true` and `fieldRename: FieldRename.snake` + static final _byRawString = _$PropagateModeEnumMap + .map((key, value) => MapEntry(value, key)); } diff --git a/lib/model/message.dart b/lib/model/message.dart index 84f3bbc3e5..d1886b810f 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -169,48 +169,17 @@ class MessageStoreImpl with MessageStore { } void _handleUpdateMessageEventMove(UpdateMessageEvent event) { - // The interaction between the fields of these events are a bit tricky. - // For reference, see: https://zulip.com/api/get-events#update_message - - final origStreamId = event.origStreamId; - final newStreamId = event.newStreamId; // null if topic-only move - final origTopic = event.origTopic; - final newTopic = event.newTopic; - final propagateMode = event.propagateMode; - - if (origTopic == null) { + final messageMove = event.moveData; + if (messageMove == null) { // There was no move. - assert(() { - if (newStreamId != null && origStreamId != null - && newStreamId != origStreamId) { - // This should be impossible; `orig_subject` (aka origTopic) is - // documented to be present when either the stream or topic changed. - debugLog('Malformed UpdateMessageEvent: stream move but no origTopic'); // TODO(log) - } - return true; - }()); return; } - if (newStreamId == null && newTopic == null) { - // If neither the channel nor topic name changed, nothing moved. - // In that case `orig_subject` (aka origTopic) should have been null. - assert(debugLog('Malformed UpdateMessageEvent: move but no newStreamId or newTopic')); // TODO(log) - return; - } - if (origStreamId == null) { - // The `stream_id` field (aka origStreamId) is documented to be present on moves. - assert(debugLog('Malformed UpdateMessageEvent: move but no origStreamId')); // TODO(log) - return; - } - if (propagateMode == null) { - // The `propagate_mode` field (aka propagateMode) is documented to be present on moves. - assert(debugLog('Malformed UpdateMessageEvent: move but no propagateMode')); // TODO(log) - return; - } + final UpdateMessageMoveData( + :origStreamId, :newStreamId, :origTopic, :newTopic) = messageMove; - final wasResolveOrUnresolve = (newStreamId == null - && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic!)); + final wasResolveOrUnresolve = newStreamId == origStreamId + && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic); for (final messageId in event.messageIds) { final message = messages[messageId]; @@ -221,14 +190,14 @@ class MessageStoreImpl with MessageStore { continue; } - if (newStreamId != null) { + if (newStreamId != origStreamId) { message.streamId = newStreamId; // See [StreamMessage.displayRecipient] on why the invalidation is // needed. message.displayRecipient = null; } - if (newTopic != null) { + if (newTopic != origTopic) { message.topic = newTopic; } @@ -239,14 +208,7 @@ class MessageStoreImpl with MessageStore { } for (final view in _messageListViews) { - view.messagesMoved( - origStreamId: origStreamId, - newStreamId: newStreamId ?? origStreamId, - origTopic: origTopic, - newTopic: newTopic ?? origTopic, - messageIds: event.messageIds, - propagateMode: propagateMode, - ); + view.messagesMoved(messageMove: messageMove, messageIds: event.messageIds); } } diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index ea120cd72e..a1df255cb2 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -720,13 +720,12 @@ class MessageListView with ChangeNotifier, _MessageSequence { } void messagesMoved({ - required int origStreamId, - required int newStreamId, - required TopicName origTopic, - required TopicName newTopic, + required UpdateMessageMoveData messageMove, required List messageIds, - required PropagateMode propagateMode, }) { + final UpdateMessageMoveData( + :origStreamId, :newStreamId, :origTopic, :newTopic, :propagateMode, + ) = messageMove; switch (narrow) { case DmNarrow(): // DMs can't be moved (nor created by moves), diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 1fcea3f83c..34b4492013 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -259,10 +259,8 @@ class Unreads extends ChangeNotifier { (f) => f == MessageFlag.mentioned || f == MessageFlag.wildcardMentioned, ); - // 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 + // We expect the event's 'read' flag to be boring, + // matching the message's local unread state. final bool isRead = event.flags.contains(MessageFlag.read); assert(() { final isUnreadLocally = isUnread(messageId); @@ -272,6 +270,17 @@ class Unreads extends ChangeNotifier { // We were going to check something but can't; shrug. if (isUnreadLocally == null) return true; + final newChannelId = event.moveData?.newStreamId; + if (newChannelId != null && !channelStore.subscriptions.containsKey(newChannelId)) { + // When unread messages are moved to an unsubscribed channel, the server + // marks them as read without sending a mark-as-read event. Clients are + // asked to special-case this by marking them as read, which we do in + // _handleMessageMove. That contract is clear enough and doesn't involve + // this event's 'read' flag, so don't bother logging about the flag; + // its behavior seems like an implementation detail that could change. + return true; + } + if (isUnreadLocally != isUnreadInEvent) { // If this happens, then either: // - the server and client have been out of sync about the message's @@ -296,13 +305,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 = _popAllInStreamTopic( + event.messageIds.toSet(), origStreamId, origTopic)?..sort(); + + if (messageToMoveIds == null || messageToMoveIds.isEmpty) return false; + assert(event.messageIds.toSet().containsAll(messageToMoveIds)); + + 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, newStreamId, newTopic); + + return true; + } + void handleDeleteMessageEvent(DeleteMessageEvent event) { mentions.removeAll(event.messageIds); final messageIdsSet = Set.of(event.messageIds); @@ -455,6 +490,8 @@ class Unreads extends ChangeNotifier { // [messageIds] must be sorted ascending and without duplicates. void _addAllInStreamTopic(QueueList messageIds, int streamId, TopicName topic) { + assert(messageIds.isNotEmpty); + assert(isSortedWithoutDuplicates(messageIds)); final topics = streams[streamId] ??= {}; topics.update(topic, ifAbsent: () => messageIds, @@ -504,6 +541,49 @@ class Unreads extends ChangeNotifier { } } + /// Remove unread stream messages contained in `incomingMessageIds`, with + /// the matching `streamId` and `topic`. + /// + /// Returns the removed message IDs, or `null` if no messages are affected. + /// + /// Use [_removeAllInStreamTopic] if the removed message IDs are not needed. + // Part of this is adapted from [ListBase.removeWhere]. + QueueList? _popAllInStreamTopic(Set incomingMessageIds, int streamId, TopicName topic) { + final topics = streams[streamId]; + if (topics == null) return null; + final messageIds = topics[topic]; + if (messageIds == null) return null; + + final retainedMessageIds = messageIds.whereNot( + (id) => incomingMessageIds.contains(id)).toList(); + + if (retainedMessageIds.isEmpty) { + // This is an optimization for the case when all messages in the + // conversation are removed, which avoids making a copy of `messageIds` + // unnecessarily. + topics.remove(topic); + if (topics.isEmpty) { + streams.remove(streamId); + } + return messageIds; + } + + QueueList? poppedMessageIds; + if (retainedMessageIds.length != messageIds.length) { + poppedMessageIds = QueueList.from( + messageIds.where((id) => incomingMessageIds.contains(id))); + messageIds.setRange(0, retainedMessageIds.length, retainedMessageIds); + messageIds.length = retainedMessageIds.length; + } + if (messageIds.isEmpty) { + topics.remove(topic); + if (topics.isEmpty) { + streams.remove(streamId); + } + } + return poppedMessageIds; + } + // TODO use efficient model lookups bool _slowIsPresentInDms(int messageId) { return dms.values.any((ids) => ids.contains(messageId)); diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index c1fa0a117f..12d74479de 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -48,11 +48,7 @@ extension UpdateMessageEventChecks on Subject { Subject> get messageIds => has((e) => e.messageIds, 'messageIds'); Subject> get flags => has((e) => e.flags, 'flags'); Subject get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp'); - Subject get origStreamId => has((e) => e.origStreamId, 'origStreamId'); - Subject get newStreamId => has((e) => e.newStreamId, 'newStreamId'); - Subject get propagateMode => has((e) => e.propagateMode, 'propagateMode'); - Subject get origTopic => has((e) => e.origTopic, 'origTopic'); - Subject get newTopic => has((e) => e.newTopic, 'newTopic'); + Subject get moveData => has((e) => e.moveData, 'moveData'); Subject get origContent => has((e) => e.origContent, 'origContent'); Subject get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent'); Subject get content => has((e) => e.content, 'content'); @@ -60,6 +56,14 @@ extension UpdateMessageEventChecks on Subject { Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); } +extension UpdateMessageMoveDataChecks on Subject { + Subject get origStreamId => has((e) => e.origStreamId, 'origStreamId'); + Subject get newStreamId => has((e) => e.newStreamId, 'newStreamId'); + Subject get origTopic => has((e) => e.origTopic, 'origTopic'); + Subject get newTopic => has((e) => e.newTopic, 'newTopic'); + Subject get propagateMode => has((e) => e.propagateMode, 'propagateMode'); +} + extension DeleteMessageEventChecks on Subject { Subject get messageType => has((e) => e.messageType, 'messageType'); } diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart index 51b36350cc..7cdfa94eff 100644 --- a/test/api/model/events_test.dart +++ b/test/api/model/events_test.dart @@ -101,24 +101,106 @@ void main() { 'edit_timestamp': 1718741351, 'stream_id': eg.stream().streamId, }; + final baseMoveJson = { ...baseJson, + 'orig_subject': 'foo', + 'propagate_mode': 'change_all', + }; + + test('smoke moveData', () { + check(Event.fromJson({ ...baseMoveJson, + 'stream_id': 1, + 'new_stream_id': 2, + 'orig_subject': 'foo', + 'subject': 'bar', + 'propagate_mode': 'change_all', + })).isA().moveData.isNotNull() + ..origStreamId.equals(1) + ..newStreamId.equals(2) + ..origTopic.equals(const TopicName('foo')) + ..newTopic.equals(const TopicName('bar')) + ..propagateMode.equals(PropagateMode.changeAll); + }); test('stream_id -> origStreamId', () { - check(Event.fromJson({ ...baseJson, + check(Event.fromJson({ ...baseMoveJson, 'stream_id': 1, 'new_stream_id': 2, - }) as UpdateMessageEvent) + })).isA().moveData.isNotNull() ..origStreamId.equals(1) ..newStreamId.equals(2); }); test('orig_subject -> origTopic, subject -> newTopic', () { - check(Event.fromJson({ ...baseJson, + check(Event.fromJson({ ...baseMoveJson, 'orig_subject': 'foo', 'subject': 'bar', - }) as UpdateMessageEvent) + })).isA().moveData.isNotNull() ..origTopic.equals(const TopicName('foo')) ..newTopic.equals(const TopicName('bar')); }); + + test('new channel, same topic: fill in newTopic', () { + // The server omits 'subject' in this situation. + check(Event.fromJson({ ...baseMoveJson, + 'stream_id': 1, + 'new_stream_id': 2, + 'orig_subject': 'foo', + 'subject': null, + })).isA().moveData.isNotNull() + ..origTopic.equals(const TopicName('foo')) + ..newTopic.equals(const TopicName('foo')); + }); + + test('same channel, new topic; fill in newStreamId', () { + // The server omits 'new_stream_id' in this situation. + check(Event.fromJson({ ...baseMoveJson, + 'stream_id': 1, + 'new_stream_id': null, + 'orig_subject': 'foo', + 'subject': 'bar', + })).isA().moveData.isNotNull() + ..origStreamId.equals(1) + ..newStreamId.equals(1); + }); + + test('no message move', () { + check(Event.fromJson({ ...baseJson, + 'orig_content': 'foo', + 'orig_rendered_content': 'foo', + 'content': 'bar', + 'rendered_content': 'bar', + })).isA().moveData.isNull(); + }); + + test('stream move but no orig_subject', () { + check(() => Event.fromJson({ ...baseMoveJson, + 'stream_id': 1, + 'new_stream_id': 2, + 'orig_subject': null, + })).throws(); + }); + + test('move but no subject or new_stream_id', () { + check(() => Event.fromJson({ ...baseMoveJson, + 'new_stream_id': null, + 'subject': null, + })).throws(); + }); + + test('move but no orig_stream_id', () { + check(() => Event.fromJson({ ...baseMoveJson, + 'stream_id': null, + 'new_stream_id': 2, + })).throws(); + }); + + test('move but no propagate_mode', () { + check(() => Event.fromJson({ ...baseMoveJson, + 'orig_subject': 'foo', + 'subject': 'bar', + 'propagate_mode': null, + })).throws(); + }); }); test('delete_message: require streamId and topic for stream messages', () { diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 8b39b1ad57..77995466ce 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -53,6 +53,7 @@ extension TopicNameChecks on Subject { extension StreamMessageChecks on Subject { Subject get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient'); + Subject get streamId => has((e) => e.streamId, 'streamId'); Subject get topic => has((e) => e.topic, 'topic'); } diff --git a/test/example_data.dart b/test/example_data.dart index 955f5dacfb..cfb333baec 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -655,11 +655,7 @@ UpdateMessageEvent updateMessageEditEvent( messageIds: [messageId], flags: flags ?? origMessage.flags, editTimestamp: editTimestamp ?? 1234567890, // TODO generate timestamp - origStreamId: origMessage is StreamMessage ? origMessage.streamId : null, - newStreamId: null, - propagateMode: null, - origTopic: null, - newTopic: null, + moveData: null, origContent: 'some probably-mismatched old Markdown', origRenderedContent: origMessage.content, content: 'some probably-mismatched new Markdown', @@ -681,8 +677,6 @@ UpdateMessageEvent _updateMessageMoveEvent( }) { _checkPositive(origStreamId, 'stream ID'); _checkPositive(newStreamId, 'stream ID'); - assert(newTopic != origTopic - || (newStreamId != null && newStreamId != origStreamId)); assert(messageIds.isNotEmpty); return UpdateMessageEvent( id: 0, @@ -692,11 +686,13 @@ UpdateMessageEvent _updateMessageMoveEvent( messageIds: messageIds, flags: flags, editTimestamp: 1234567890, // TODO generate timestamp - origStreamId: origStreamId, - newStreamId: newStreamId, - propagateMode: propagateMode, - origTopic: origTopic, - newTopic: newTopic, + moveData: UpdateMessageMoveData( + origStreamId: origStreamId, + newStreamId: newStreamId ?? origStreamId, + origTopic: origTopic, + newTopic: newTopic ?? origTopic, + propagateMode: propagateMode, + ), origContent: origContent, origRenderedContent: origContent, content: newContent, diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index da286f3ef5..92c08ddd6b 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -825,11 +825,14 @@ void main() { }); test('unrelated channel -> new channel: unaffected', () async { + final thirdStream = eg.stream(); await prepareNarrow(narrow, initialMessages); + await store.addStream(thirdStream); + await store.addSubscription(eg.subscription(thirdStream)); await store.handleEvent(eg.updateMessageEventMoveFrom( origMessages: otherChannelMovedMessages, - newStreamId: otherStream.streamId, + newStreamId: thirdStream.streamId, )); checkHasMessages(initialMessages); checkNotNotified(); diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 40e074dcaa..9a2e899ba7 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -469,6 +469,175 @@ void main() { } } }); + + group('moves', () { + final origChannel = eg.stream(); + const origTopic = 'origTopic'; + const newTopic = 'newTopic'; + + late List readMessages; + late List unreadMessages; + + Future prepareStore() async { + prepare(); + await channelStore.addStream(origChannel); + await channelStore.addSubscription(eg.subscription(origChannel)); + readMessages = List.generate(10, + (_) => eg.streamMessage(stream: origChannel, topic: origTopic, + flags: [MessageFlag.read])); + unreadMessages = List.generate(10, + (_) => eg.streamMessage(stream: origChannel, topic: origTopic)); + } + + List copyMessagesWith(Iterable messages, { + ZulipStream? newChannel, + String? newTopic, + }) { + assert(newChannel != null || newTopic != null); + return messages.map((message) => StreamMessage.fromJson( + message.toJson() + ..['stream_id'] = newChannel?.streamId ?? message.streamId + ..['subject'] = newTopic ?? message.topic + )).toList(); + } + + test('moved messages = unread messages', () async { + await prepareStore(); + final newChannel = eg.stream(); + await channelStore.addStream(newChannel); + await channelStore.addSubscription(eg.subscription(newChannel)); + fillWithMessages(unreadMessages); + final originalMessageIds = + model.streams[origChannel.streamId]![TopicName(origTopic)]!; + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newStreamId: newChannel.streamId, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages(copyMessagesWith(unreadMessages, + newChannel: newChannel, newTopic: newTopic)); + final newMessageIds = + model.streams[newChannel.streamId]![TopicName(newTopic)]!; + // Check we successfully avoided making a copy of the list. + check(originalMessageIds).identicalTo(newMessageIds); + }); + + test('moved messages ⊂ read messages', () async { + await prepareStore(); + final messagesToMove = readMessages.take(2).toList(); + fillWithMessages(unreadMessages + readMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: newTopic)); + checkNotNotified(); + checkMatchesMessages(unreadMessages); + }); + + test('moved messages ⊂ unread messages', () async { + await prepareStore(); + final messagesToMove = unreadMessages.take(2).toList(); + fillWithMessages(unreadMessages + readMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + ...copyMessagesWith(messagesToMove, newTopic: newTopic), + ...unreadMessages.skip(2), + ]); + }); + + test('moved messages ∩ unread messages ≠ Ø, moved messages ∩ read messages ≠ Ø, moved messages ⊅ unread messages', () async { + await prepareStore(); + final messagesToMove = [unreadMessages.first, readMessages.first]; + fillWithMessages(unreadMessages + readMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages([ + ...copyMessagesWith(unreadMessages.take(1), newTopic: newTopic), + ...unreadMessages.skip(1), + ]); + }); + + test('moved messages ⊃ unread messages', () async { + await prepareStore(); + final messagesToMove = unreadMessages + readMessages.take(2).toList(); + fillWithMessages(unreadMessages + readMessages); + final originalMessageIds = + model.streams[origChannel.streamId]![TopicName(origTopic)]!; + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: messagesToMove, + newTopicStr: newTopic)); + checkNotifiedOnce(); + checkMatchesMessages(copyMessagesWith(unreadMessages, newTopic: newTopic)); + final newMessageIds = + model.streams[origChannel.streamId]![TopicName(newTopic)]!; + // Check we successfully avoided making a copy of the list. + check(originalMessageIds).identicalTo(newMessageIds); + }); + + test('moving to unsubscribed channels drops the unreads', () async { + await prepareStore(); + final unsubscribedChannel = eg.stream(); + await channelStore.addStream(unsubscribedChannel); + assert(!channelStore.subscriptions.containsKey( + unsubscribedChannel.streamId)); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: unreadMessages, + newStreamId: unsubscribedChannel.streamId)); + checkNotifiedOnce(); + checkMatchesMessages([]); + }); + + test('tolerates 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(copyMessagesWith(unreadMessages, newTopic: newTopic)); + }); + + test('tolerates unreads unknown to the model', () async { + await prepareStore(); + fillWithMessages(unreadMessages); + + final unknownChannel = eg.stream(); + assert(!channelStore.streams.containsKey(unknownChannel.streamId)); + final unknownUnreadMessage = eg.streamMessage( + stream: unknownChannel, topic: origTopic); + + model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom( + origMessages: [unknownUnreadMessage], + newTopicStr: newTopic)); + checkNotNotified(); + checkMatchesMessages(unreadMessages); + }); + + test('message edit but no move', () async { + await prepareStore(); + fillWithMessages(unreadMessages); + + model.handleUpdateMessageEvent(eg.updateMessageEditEvent( + unreadMessages.first)); + checkNotNotified(); + checkMatchesMessages(unreadMessages); + }); + }); });