diff --git a/integration_test/unreadmarker_test.dart b/integration_test/unreadmarker_test.dart index babc6cd3e8..548a446841 100644 --- a/integration_test/unreadmarker_test.dart +++ b/integration_test/unreadmarker_test.dart @@ -32,7 +32,7 @@ void main() { newestResult(foundOldest: true, messages: messages).toJson()); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: const MessageListPage(narrow: CombinedFeedNarrow()))); + child: const MessageListPage(initNarrow: CombinedFeedNarrow()))); await tester.pumpAndSettle(); return messages; } diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 1a4e35253e..614a784511 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -480,7 +480,7 @@ sealed class Message { final int senderId; final String senderRealmStr; @JsonKey(name: 'subject') - final String topic; + String topic; // final List submessages; // TODO handle final int timestamp; String get type; @@ -576,8 +576,12 @@ class StreamMessage extends Message { @JsonKey(includeToJson: true) String get type => 'stream'; - final String displayRecipient; - final int streamId; + // This is not nullable API-wise, but if the message moves across channels, + // [displayRecipient] still refers to the original channel and it has to be + // invalidated. + @JsonKey(required: true, disallowNullValue: true) + String? displayRecipient; + int streamId; StreamMessage({ required super.client, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index a824abba9b..10c1c6b00c 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -260,55 +260,70 @@ Map _$SubscriptionToJson(Subscription instance) => 'color': instance.color, }; -StreamMessage _$StreamMessageFromJson(Map json) => - StreamMessage( - client: json['client'] as String, - content: json['content'] as String, - contentType: json['content_type'] as String, - editState: Message._messageEditStateFromJson( - MessageEditState._readFromMessage(json, 'edit_state')), - id: (json['id'] as num).toInt(), - isMeMessage: json['is_me_message'] as bool, - lastEditTimestamp: (json['last_edit_timestamp'] as num?)?.toInt(), - reactions: Message._reactionsFromJson(json['reactions']), - recipientId: (json['recipient_id'] as num).toInt(), - senderEmail: json['sender_email'] as String, - senderFullName: json['sender_full_name'] as String, - senderId: (json['sender_id'] as num).toInt(), - senderRealmStr: json['sender_realm_str'] as String, - topic: json['subject'] as String, - timestamp: (json['timestamp'] as num).toInt(), - flags: Message._flagsFromJson(json['flags']), - matchContent: json['match_content'] as String?, - matchTopic: json['match_subject'] as String?, - displayRecipient: json['display_recipient'] as String, - streamId: (json['stream_id'] as num).toInt(), - ); - -Map _$StreamMessageToJson(StreamMessage instance) => - { - 'client': instance.client, - 'content': instance.content, - 'content_type': instance.contentType, - 'edit_state': _$MessageEditStateEnumMap[instance.editState]!, - 'id': instance.id, - 'is_me_message': instance.isMeMessage, - 'last_edit_timestamp': instance.lastEditTimestamp, - 'reactions': Message._reactionsToJson(instance.reactions), - 'recipient_id': instance.recipientId, - 'sender_email': instance.senderEmail, - 'sender_full_name': instance.senderFullName, - 'sender_id': instance.senderId, - 'sender_realm_str': instance.senderRealmStr, - 'subject': instance.topic, - 'timestamp': instance.timestamp, - 'flags': instance.flags, - 'match_content': instance.matchContent, - 'match_subject': instance.matchTopic, - 'type': instance.type, - 'display_recipient': instance.displayRecipient, - 'stream_id': instance.streamId, - }; +StreamMessage _$StreamMessageFromJson(Map json) { + $checkKeys( + json, + requiredKeys: const ['display_recipient'], + disallowNullValues: const ['display_recipient'], + ); + return StreamMessage( + client: json['client'] as String, + content: json['content'] as String, + contentType: json['content_type'] as String, + editState: Message._messageEditStateFromJson( + MessageEditState._readFromMessage(json, 'edit_state')), + id: (json['id'] as num).toInt(), + isMeMessage: json['is_me_message'] as bool, + lastEditTimestamp: (json['last_edit_timestamp'] as num?)?.toInt(), + reactions: Message._reactionsFromJson(json['reactions']), + recipientId: (json['recipient_id'] as num).toInt(), + senderEmail: json['sender_email'] as String, + senderFullName: json['sender_full_name'] as String, + senderId: (json['sender_id'] as num).toInt(), + senderRealmStr: json['sender_realm_str'] as String, + topic: json['subject'] as String, + timestamp: (json['timestamp'] as num).toInt(), + flags: Message._flagsFromJson(json['flags']), + matchContent: json['match_content'] as String?, + matchTopic: json['match_subject'] as String?, + displayRecipient: json['display_recipient'] as String?, + streamId: (json['stream_id'] as num).toInt(), + ); +} + +Map _$StreamMessageToJson(StreamMessage instance) { + final val = { + 'client': instance.client, + 'content': instance.content, + 'content_type': instance.contentType, + 'edit_state': _$MessageEditStateEnumMap[instance.editState]!, + 'id': instance.id, + 'is_me_message': instance.isMeMessage, + 'last_edit_timestamp': instance.lastEditTimestamp, + 'reactions': Message._reactionsToJson(instance.reactions), + 'recipient_id': instance.recipientId, + 'sender_email': instance.senderEmail, + 'sender_full_name': instance.senderFullName, + 'sender_id': instance.senderId, + 'sender_realm_str': instance.senderRealmStr, + 'subject': instance.topic, + 'timestamp': instance.timestamp, + 'flags': instance.flags, + 'match_content': instance.matchContent, + 'match_subject': instance.matchTopic, + 'type': instance.type, + }; + + void writeNotNull(String key, dynamic value) { + if (value != null) { + val[key] = value; + } + } + + writeNotNull('display_recipient', instance.displayRecipient); + val['stream_id'] = instance.streamId; + return val; +} const _$MessageEditStateEnumMap = { MessageEditState.none: 'none', diff --git a/lib/model/message.dart b/lib/model/message.dart index e58049710e..1e3abbbcdb 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -152,6 +152,7 @@ class MessageStoreImpl with MessageStore { final newStreamId = event.newStreamId; // null if topic-only move final origTopic = event.origTopic; final newTopic = event.newTopic; + final propagateMode = event.propagateMode; if (origTopic == null) { // There was no move. @@ -167,9 +168,10 @@ class MessageStoreImpl with MessageStore { return; } - if (newTopic == null) { - // The `subject` field (aka newTopic) is documented to be present on moves. - assert(debugLog('Malformed UpdateMessageEvent: move but no newTopic')); // TODO(log) + 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) { @@ -177,23 +179,50 @@ class MessageStoreImpl with MessageStore { assert(debugLog('Malformed UpdateMessageEvent: move but no origStreamId')); // TODO(log) return; } - - if (newStreamId == null - && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic)) { - // The topic was only resolved/unresolved. - // No change to the messages' editState. + 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; } - // TODO(#150): Handle message moves. The views' recipient headers - // may need updating, and consequently showSender too. - // Currently only editState gets updated. + final wasResolveOrUnresolve = (newStreamId == null + && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic!)); + for (final messageId in event.messageIds) { final message = messages[messageId]; if (message == null) continue; - // Do not override the edited marker if the message has also been moved. - if (message.editState == MessageEditState.edited) continue; - message.editState = MessageEditState.moved; + + if (message is! StreamMessage) { + assert(debugLog('Bad UpdateMessageEvent: stream/topic move on a DM')); // TODO(log) + continue; + } + + if (newStreamId != null) { + message.streamId = newStreamId; + // See [StreamMessage.displayRecipient] on why the invalidation is + // needed. + message.displayRecipient = null; + } + + if (newTopic != null) { + message.topic = newTopic; + } + + if (!wasResolveOrUnresolve + && message.editState == MessageEditState.none) { + message.editState = MessageEditState.moved; + } + } + + for (final view in _messageListViews) { + view.messagesMoved( + origStreamId: origStreamId, + newStreamId: newStreamId ?? origStreamId, + origTopic: origTopic, + newTopic: newTopic ?? origTopic, + messageIds: event.messageIds, + propagateMode: propagateMode, + ); } } diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index fa9dc026c1..f20960e221 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -65,6 +65,9 @@ class MessageListHistoryStartItem extends MessageListItem { /// /// This comprises much of the guts of [MessageListView]. mixin _MessageSequence { + /// A sequence number for invalidating stale fetches. + int generation = 0; + /// The messages. /// /// See also [contents] and [items]. @@ -192,6 +195,17 @@ mixin _MessageSequence { _reprocessAll(); } + /// Reset all [_MessageSequence] data, and cancel any active fetches. + void _reset() { + generation += 1; + messages.clear(); + _fetched = false; + _haveOldest = false; + _fetchingOlder = false; + contents.clear(); + items.clear(); + } + /// Redo all computations from scratch, based on [messages]. void _recompute() { assert(contents.length == messages.length); @@ -345,7 +359,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { } final PerAccountStore store; - final Narrow narrow; + Narrow narrow; /// Whether [message] should actually appear in this message list, /// given that it does belong to the narrow. @@ -398,12 +412,14 @@ class MessageListView with ChangeNotifier, _MessageSequence { assert(!fetched && !haveOldest && !fetchingOlder); assert(messages.isEmpty && contents.isEmpty); // TODO schedule all this in another isolate + final generation = this.generation; final result = await getMessages(store.connection, narrow: narrow.apiEncode(), anchor: AnchorCode.newest, numBefore: kMessageListFetchBatchSize, numAfter: 0, ); + if (this.generation > generation) return; store.reconcileMessages(result.messages); store.recentSenders.handleMessages(result.messages); // TODO(#824) for (final message in result.messages) { @@ -426,6 +442,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { _fetchingOlder = true; _updateEndMarkers(); notifyListeners(); + final generation = this.generation; try { final result = await getMessages(store.connection, narrow: narrow.apiEncode(), @@ -434,6 +451,7 @@ class MessageListView with ChangeNotifier, _MessageSequence { numBefore: kMessageListFetchBatchSize, numAfter: 0, ); + if (this.generation > generation) return; if (result.messages.isNotEmpty && result.messages.last.id == messages[0].id) { @@ -451,9 +469,11 @@ class MessageListView with ChangeNotifier, _MessageSequence { _insertAllMessages(0, fetchedMessages); _haveOldest = result.foundOldest; } finally { - _fetchingOlder = false; - _updateEndMarkers(); - notifyListeners(); + if (this.generation == generation) { + _fetchingOlder = false; + _updateEndMarkers(); + notifyListeners(); + } } } @@ -489,6 +509,87 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } + void _messagesMovedInternally(List messageIds) { + for (final messageId in messageIds) { + if (_findMessageWithId(messageId) != -1) { + _reprocessAll(); + notifyListeners(); + return; + } + } + } + + void _messagesMovedIntoNarrow() { + // If there are some messages we don't have in [MessageStore], and they + // occur later than the messages we have here, then we just have to + // re-fetch from scratch. That's always valid, so just do that always. + // TODO in cases where we do have data to do better, do better. + _reset(); + notifyListeners(); + fetchInitial(); + } + + void _messagesMovedFromNarrow(List messageIds) { + if (_removeMessagesById(messageIds)) { + notifyListeners(); + } + } + + void _handlePropagateMode(PropagateMode propagateMode, Narrow newNarrow) { + switch (propagateMode) { + case PropagateMode.changeAll: + case PropagateMode.changeLater: + narrow = newNarrow; + _reset(); + fetchInitial(); + case PropagateMode.changeOne: + } + } + + void messagesMoved({ + required int origStreamId, + required int newStreamId, + required String origTopic, + required String newTopic, + required List messageIds, + required PropagateMode propagateMode, + }) { + switch (narrow) { + case DmNarrow(): + // DMs can't be moved (nor created by moves), + // so the messages weren't in this narrow and still aren't. + return; + + case CombinedFeedNarrow(): + case MentionsNarrow(): + // The messages were and remain in this narrow. + // TODO(#421): … except they may have become muted or not. + // We'll handle that at the same time as we handle muting itself changing. + // Recipient headers, and downstream of those, may change, though. + _messagesMovedInternally(messageIds); + + case ChannelNarrow(:final streamId): + switch ((origStreamId == streamId, newStreamId == streamId)) { + case (false, false): return; + case (true, true ): _messagesMovedInternally(messageIds); + case (false, true ): _messagesMovedIntoNarrow(); + case (true, false): _messagesMovedFromNarrow(messageIds); + } + + case TopicNarrow(:final streamId, :final topic): + final oldMatch = (origStreamId == streamId && origTopic == topic); + final newMatch = (newStreamId == streamId && newTopic == topic); + switch ((oldMatch, newMatch)) { + case (false, false): return; + case (true, true ): return; // TODO(log) no-op move + case (false, true ): _messagesMovedIntoNarrow(); + case (true, false): + _messagesMovedFromNarrow(messageIds); + _handlePropagateMode(propagateMode, TopicNarrow(newStreamId, newTopic)); + } + } + } + // Repeal the `@protected` annotation that applies on the base implementation, // so we can call this method from [MessageStoreImpl]. @override diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 0787a61bee..c0aa0ca831 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -272,7 +272,7 @@ class NotificationDisplayManager { // TODO(nav): Better interact with existing nav stack on notif open navigator.push(MaterialAccountWidgetRoute(accountId: account.id, // TODO(#82): Open at specific message, not just conversation - page: MessageListPage(narrow: narrow))); + page: MessageListPage(initNarrow: narrow))); return; } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index fca5db5664..4df50ccd37 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -191,12 +191,12 @@ abstract class MessageListPageState { } class MessageListPage extends StatefulWidget { - const MessageListPage({super.key, required this.narrow}); + const MessageListPage({super.key, required this.initNarrow}); static Route buildRoute({int? accountId, BuildContext? context, required Narrow narrow}) { return MaterialAccountWidgetRoute(accountId: accountId, context: context, - page: MessageListPage(narrow: narrow)); + page: MessageListPage(initNarrow: narrow)); } /// The [MessageListPageState] above this context in the tree. @@ -211,7 +211,7 @@ class MessageListPage extends StatefulWidget { return state!; } - final Narrow narrow; + final Narrow initNarrow; @override State createState() => _MessageListPageState(); @@ -219,13 +219,25 @@ class MessageListPage extends StatefulWidget { class _MessageListPageState extends State implements MessageListPageState { @override - Narrow get narrow => widget.narrow; + late Narrow narrow; @override ComposeBoxController? get composeBoxController => _composeBoxKey.currentState; final GlobalKey _composeBoxKey = GlobalKey(); + @override + void initState() { + super.initState(); + narrow = widget.initNarrow; + } + + void _narrowChanged(Narrow newNarrow) { + setState(() { + narrow = newNarrow; + }); + } + @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); @@ -233,7 +245,7 @@ class _MessageListPageState extends State implements MessageLis final Color? appBarBackgroundColor; bool removeAppBarBottomBorder = false; - switch(widget.narrow) { + switch(narrow) { case CombinedFeedNarrow(): case MentionsNarrow(): appBarBackgroundColor = null; // i.e., inherit @@ -256,7 +268,7 @@ class _MessageListPageState extends State implements MessageLis } return Scaffold( - appBar: AppBar(title: MessageListAppBarTitle(narrow: widget.narrow), + appBar: AppBar(title: MessageListAppBarTitle(narrow: narrow), backgroundColor: appBarBackgroundColor, shape: removeAppBarBottomBorder ? const Border() @@ -280,11 +292,11 @@ class _MessageListPageState extends State implements MessageLis // The compose box, when present, pads the bottom inset. // TODO(#311) If we have a bottom nav, it will pad the bottom // inset, and this should always be true. - removeBottom: ComposeBox.hasComposeBox(widget.narrow), + removeBottom: ComposeBox.hasComposeBox(narrow), child: Expanded( - child: MessageList(narrow: widget.narrow))), - ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow), + child: MessageList(narrow: narrow, onNarrowChanged: _narrowChanged))), + ComposeBox(controllerKey: _composeBoxKey, narrow: narrow), ])))); } } @@ -362,9 +374,10 @@ const _kShortMessageHeight = 80; const kFetchMessagesBufferPixels = (kMessageListFetchBatchSize / 2) * _kShortMessageHeight; class MessageList extends StatefulWidget { - const MessageList({super.key, required this.narrow}); + const MessageList({super.key, required this.narrow, required this.onNarrowChanged}); final Narrow narrow; + final void Function(Narrow newNarrow) onNarrowChanged; @override State createState() => _MessageListState(); @@ -401,6 +414,11 @@ class _MessageListState extends State with PerAccountStoreAwareStat } void _modelChanged() { + if (model!.narrow != widget.narrow) { + // A message move event occurred, where propagate mode is + // [PropagateMode.changeAll] or [PropagateMode.changeLater]. + widget.onNarrowChanged(model!.narrow); + } setState(() { // The actual state lives in the [MessageListView] model. // This method was called because that just changed. @@ -978,7 +996,9 @@ class StreamMessageRecipientHeader extends StatelessWidget { streamWidget = const SizedBox(width: 16); } else { final stream = store.streams[message.streamId]; - final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing + final streamName = stream?.name + ?? message.displayRecipient + ?? '(unknown channel)'; // TODO(log) streamWidget = GestureDetector( onTap: () => Navigator.push(context, diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 436e3569c6..c8261ba78f 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -49,6 +49,10 @@ extension MessageChecks on Subject { Subject get matchTopic => has((e) => e.matchTopic, 'matchTopic'); } +extension StreamMessageChecks on Subject { + Subject get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient'); +} + extension ReactionsChecks on Subject { Subject get total => has((e) => e.total, 'total'); Subject> get aggregated => has((e) => e.aggregated, 'aggregated'); diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index 3370fa2a42..351c3f77d8 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:checks/checks.dart'; +import 'package:json_annotation/json_annotation.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/model.dart'; @@ -149,6 +150,14 @@ void main() { check(m2).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]); }); + test('require displayRecipient on parse', () { + check(() => StreamMessage.fromJson(baseStreamJson()..['display_recipient'] = null)) + .throws(); + + check(() => StreamMessage.fromJson(baseStreamJson()..remove('display_recipient'))) + .throws(); + }); + // Code relevant to messageEditState is tested separately in the // MessageEditState group. }); diff --git a/test/example_data.dart b/test/example_data.dart index c191aaf8b8..41abc88bc1 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -463,28 +463,31 @@ UpdateMessageEvent updateMessageEditEvent( ); } -UpdateMessageEvent updateMessageMoveEvent( - List messages, { +UpdateMessageEvent _updateMessageMoveEvent( + List messageIds, { + required int origStreamId, int? newStreamId, - String? origTopic, + required String origTopic, String? newTopic, String? origContent, String? newContent, + required List flags, + PropagateMode propagateMode = PropagateMode.changeOne, }) { - assert(messages.isNotEmpty); - final origMessage = messages[0]; - final messageId = origMessage.id; + assert(newTopic != origTopic + || (newStreamId != null && newStreamId != origStreamId)); + assert(messageIds.isNotEmpty); return UpdateMessageEvent( id: 0, userId: selfUser.userId, renderingOnly: false, - messageId: messageId, - messageIds: messages.map((message) => message.id).toList(), - flags: origMessage.flags, + messageId: messageIds.first, + messageIds: messageIds, + flags: flags, editTimestamp: 1234567890, // TODO generate timestamp - origStreamId: origMessage is StreamMessage ? origMessage.streamId : null, + origStreamId: origStreamId, newStreamId: newStreamId, - propagateMode: null, + propagateMode: propagateMode, origTopic: origTopic, newTopic: newTopic, origContent: origContent, @@ -495,6 +498,58 @@ UpdateMessageEvent updateMessageMoveEvent( ); } +/// An [UpdateMessageEvent] where [origMessages] are moved to somewhere else. +UpdateMessageEvent updateMessageEventMoveFrom({ + required List origMessages, + int? newStreamId, + String? newTopic, + String? newContent, + PropagateMode propagateMode = PropagateMode.changeOne, +}) { + assert(origMessages.isNotEmpty); + final origMessage = origMessages.first; + // Only present on content change. + final origContent = (newContent != null) ? origMessage.content : null; + return _updateMessageMoveEvent(origMessages.map((e) => e.id).toList(), + origStreamId: origMessage.streamId, + newStreamId: newStreamId, + origTopic: origMessage.topic, + newTopic: newTopic, + origContent: origContent, + newContent: newContent, + flags: origMessage.flags, + propagateMode: propagateMode, + ); +} + +/// An [UpdateMessageEvent] where [newMessages] are moved from somewhere. +UpdateMessageEvent updateMessageEventMoveTo({ + required List newMessages, + int? origStreamId, + String? origTopic, + String? origContent, + PropagateMode propagateMode = PropagateMode.changeOne, +}) { + assert(newMessages.isNotEmpty); + final newMessage = newMessages.first; + // Only present on topic move. + final newTopic = (origTopic != null) ? newMessage.topic : null; + // Only present on channel move. + final newStreamId = (origStreamId != null) ? newMessage.streamId : null; + // Only present on content change. + final newContent = (origContent != null) ? newMessage.content : null; + return _updateMessageMoveEvent(newMessages.map((e) => e.id).toList(), + origStreamId: origStreamId ?? newMessage.streamId, + newStreamId: newStreamId, + origTopic: origTopic ?? newMessage.topic, + newTopic: newTopic, + origContent: origContent, + newContent: newContent, + flags: newMessage.flags, + propagateMode: propagateMode, + ); +} + UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( MessageFlag flag, Iterable messages, { diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index d0906d849b..641a2d8de1 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -60,8 +60,14 @@ extension TextChecks on Subject { Subject get style => has((t) => t.style, 'style'); } +extension TextEditingControllerChecks on Subject { + Subject get text => has((t) => t.text, 'text'); +} + extension TextFieldChecks on Subject { Subject get textCapitalization => has((t) => t.textCapitalization, 'textCapitalization'); + Subject get decoration => has((t) => t.decoration, 'decoration'); + Subject get controller => has((t) => t.controller, 'controller'); } extension TextStyleChecks on Subject { @@ -131,3 +137,7 @@ extension MaterialChecks on Subject { Subject get color => has((x) => x.color, 'color'); // TODO more } + +extension InputDecorationChecks on Subject { + Subject get hintText => has((x) => x.hintText, 'hintText'); +} diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 545074a97c..ce1f582b1e 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -16,6 +16,7 @@ import 'package:zulip/model/store.dart'; import '../api/fake_api.dart'; import '../api/model/model_checks.dart'; import '../example_data.dart' as eg; +import '../fake_async.dart'; import '../stdlib_checks.dart'; import 'content_checks.dart'; import 'recent_senders_test.dart' as recent_senders_test; @@ -465,6 +466,505 @@ void main() { }); }); + group('messagesMoved', () { + final stream = eg.stream(streamId: 1, name: 'test stream'); + final otherStream = eg.stream(streamId: 2, name: 'other stream'); + + void checkHasMessages(Iterable messages) { + check(model.messages.map((e) => e.id)).deepEquals(messages.map((e) => e.id)); + } + + Future prepareNarrow(Narrow narrow, List? messages) async { + await prepare(narrow: narrow); + for (final streamToAdd in [stream, otherStream]) { + final subscription = eg.subscription(streamToAdd); + await store.addStream(streamToAdd); + await store.addSubscription(subscription); + } + if (messages != null) { + await prepareMessages(foundOldest: false, messages: messages); + } + checkHasMessages(messages ?? []); + } + + group('in combined feed narrow', () { + const narrow = CombinedFeedNarrow(); + final initialMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + final movedMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + + test('internal move between channels', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: initialMessages, + newTopic: initialMessages[0].topic, + newStreamId: otherStream.streamId, + )); + checkHasMessages(initialMessages); + checkNotified(count: 2); + })); + + test('internal move between topics', () async { + await prepareNarrow(narrow, initialMessages + movedMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newTopic: 'new', + )); + checkHasMessages(initialMessages + movedMessages); + checkNotified(count: 2); + }); + }); + + group('in channel narrow', () { + final narrow = ChannelNarrow(stream.streamId); + final initialMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + final movedMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + final otherChannelMovedMessages = List.generate(5, (i) => eg.streamMessage(stream: otherStream, topic: 'topic')); + + test('channel -> channel: internal move', () async { + await prepareNarrow(narrow, initialMessages + movedMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newTopic: 'new', + )); + checkHasMessages(initialMessages + movedMessages); + checkNotified(count: 2); + }); + + test('old channel -> channel: refetch', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: 'orig topic', + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + check(model).fetched.isFalse(); + checkHasMessages([]); + checkNotifiedOnce(); + + async.elapse(const Duration(seconds: 2)); + checkHasMessages(initialMessages + movedMessages); + checkNotifiedOnce(); + })); + + test('channel -> new channel: remove moved messages', () async { + await prepareNarrow(narrow, initialMessages + movedMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newTopic: 'new', + newStreamId: otherStream.streamId, + )); + checkHasMessages(initialMessages); + checkNotifiedOnce(); + }); + + test('unrelated channel -> new channel: unaffected', () async { + await prepareNarrow(narrow, initialMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: otherChannelMovedMessages, + newStreamId: otherStream.streamId, + )); + checkHasMessages(initialMessages); + checkNotNotified(); + }); + + test('unrelated channel -> unrelated channel: unaffected', () async { + await prepareNarrow(narrow, initialMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: otherChannelMovedMessages, + newTopic: 'new', + )); + checkHasMessages(initialMessages); + checkNotNotified(); + }); + + void testMessageMove(PropagateMode propagateMode) => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages + movedMessages); + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newTopic: 'new', + newStreamId: otherStream.streamId, + propagateMode: propagateMode, + )); + checkNotifiedOnce(); + async.elapse(const Duration(seconds: 1)); + checkHasMessages(initialMessages); + check(model).narrow.equals(ChannelNarrow(stream.streamId)); + checkNotNotified(); + }); + + test('do not follow when propagateMode = changeOne', () { + testMessageMove(PropagateMode.changeOne); + }); + + test('do not follow when propagateMode = changeLater', () { + testMessageMove(PropagateMode.changeLater); + }); + + test('do not follow when propagateMode = changeAll', () { + testMessageMove(PropagateMode.changeAll); + }); + }); + + group('in topic narrow', () { + final narrow = TopicNarrow(stream.streamId, 'topic'); + final initialMessages = List.generate(5, (i) => eg.streamMessage(stream: stream, topic: 'topic')); + final movedMessages = List.generate(5, (i) => eg.streamMessage(stream: stream, topic: 'topic')); + final otherTopicMovedMessages = List.generate(5, (i) => eg.streamMessage(stream: stream, topic: 'other topic')); + final otherChannelMovedMessages = List.generate(5, (i) => eg.streamMessage(stream: otherStream, topic: 'topic')); + + group('moved into narrow: should refetch messages', () { + final testCases = [ + ('(old channel, topic) -> (channel, topic)', 200, null), + ('(channel, old topic) -> (channel, topic)', null, 'other'), + ('(old channel, old topic) -> (channel, topic)', 200, 'other'), + ]; + + for (final (description, origStreamId, origTopic) in testCases) { + test(description, () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origStreamId: origStreamId, + origTopic: origTopic, + newMessages: movedMessages, + )); + check(model).fetched.isFalse(); + checkHasMessages([]); + checkNotifiedOnce(); + + async.elapse(const Duration(seconds: 2)); + checkHasMessages(initialMessages + movedMessages); + checkNotifiedOnce(); + })); + } + }); + + group('moved from narrow: should remove moved messages', () { + final testCases = [ + ('(channel, topic) -> (new channel, topic)', 200, null), + ('(channel, topic) -> (channel, new topic)', null, 'new'), + ('(channel, topic) -> (new channel, new topic)', 200, 'new'), + ]; + + for (final (description, newStreamId, newTopic) in testCases) { + test(description, () async { + await prepareNarrow(narrow, initialMessages + movedMessages); + + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newStreamId: newStreamId, + newTopic: newTopic, + )); + checkHasMessages(initialMessages); + checkNotifiedOnce(); + }); + } + }); + + group('irrelevant moves', () { + test('(channel, old topic) -> (channel, unrelated topic)', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: 'other', + newMessages: otherTopicMovedMessages, + )); + check(model).fetched.isTrue(); + checkHasMessages(initialMessages); + checkNotNotified(); + })); + + test('(old channel, topic) - > (unrelated channel, topic)', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + await store.handleEvent(eg.updateMessageEventMoveTo( + origStreamId: 200, + newMessages: otherChannelMovedMessages, + )); + check(model).fetched.isTrue(); + checkHasMessages(initialMessages); + checkNotNotified(); + })); + }); + + void handleMoveEvent(PropagateMode propagateMode) => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages + movedMessages); + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: movedMessages, + newTopic: 'new', + newStreamId: otherStream.streamId, + propagateMode: propagateMode, + )); + checkNotifiedOnce(); + async.elapse(const Duration(seconds: 1)); + }); + + test('do not follow to the new narrow when propagateMode = changeOne', () { + handleMoveEvent(PropagateMode.changeOne); + checkNotNotified(); + checkHasMessages(initialMessages); + check(model).narrow.equals(TopicNarrow(stream.streamId, 'topic')); + }); + + test('follow to the new narrow when propagateMode = changeLater', () { + handleMoveEvent(PropagateMode.changeLater); + checkNotifiedOnce(); + checkHasMessages(movedMessages); + check(model).narrow.equals(TopicNarrow(otherStream.streamId, 'new')); + }); + + test('follow to the new narrow when propagateMode = changeAll', () { + handleMoveEvent(PropagateMode.changeAll); + checkNotifiedOnce(); + checkHasMessages(movedMessages); + check(model).narrow.equals(TopicNarrow(otherStream.streamId, 'new')); + }); + + test('handle move event before initial fetch', () => awaitFakeAsync((async) async { + await prepare(narrow: narrow); + final subscription = eg.subscription(stream); + await store.addStream(stream); + await store.addSubscription(subscription); + final followedMessage = eg.streamMessage(stream: stream, topic: 'new'); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: true, + messages: [followedMessage], + ).toJson()); + + check(model).fetched.isFalse(); + checkHasMessages([]); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: 'topic', + newMessages: [followedMessage], + propagateMode: PropagateMode.changeAll, + )); + check(model).narrow.equals(TopicNarrow(stream.streamId, 'new')); + + async.elapse(const Duration(seconds: 2)); + checkHasMessages([followedMessage]); + })); + }); + + group('fetch races', () { + final narrow = ChannelNarrow(stream.streamId); + final olderMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + final initialMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + final movedMessages = List.generate(5, (i) => eg.streamMessage(stream: stream)); + + test('fetchOlder, _reset, fetchOlder returns, move fetch finishes', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + connection.prepare(delay: const Duration(seconds: 1), json: olderResult( + anchor: model.messages[0].id, + foundOldest: true, + messages: olderMessages, + ).toJson()); + final fetchFuture = model.fetchOlder(); + check(model).fetchingOlder.isTrue(); + checkHasMessages(initialMessages); + checkNotifiedOnce(); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: movedMessages[0].topic, + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + check(model).fetchingOlder.isFalse(); + checkHasMessages([]); + checkNotifiedOnce(); + + await fetchFuture; + checkHasMessages([]); + checkNotNotified(); + + async.elapse(const Duration(seconds: 1)); + checkHasMessages(initialMessages + movedMessages); + checkNotifiedOnce(); + })); + + test('fetchOlder, _reset, move fetch finishes, fetchOlder returns', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + connection.prepare(delay: const Duration(seconds: 2), json: olderResult( + anchor: model.messages[0].id, + foundOldest: true, + messages: olderMessages, + ).toJson()); + final fetchFuture = model.fetchOlder(); + checkHasMessages(initialMessages); + check(model).fetchingOlder.isTrue(); + checkNotifiedOnce(); + + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: movedMessages[0].topic, + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + checkHasMessages([]); + check(model).fetchingOlder.isFalse(); + checkNotifiedOnce(); + + async.elapse(const Duration(seconds: 1)); + checkHasMessages(initialMessages + movedMessages); + checkNotifiedOnce(); + + await fetchFuture; + checkHasMessages(initialMessages + movedMessages); + checkNotNotified(); + })); + + test('fetchInitial, _reset, initial fetch finishes, move fetch finishes', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, null); + + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: initialMessages, + ).toJson()); + final fetchFuture = model.fetchInitial(); + checkHasMessages([]); + check(model).fetched.isFalse(); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: movedMessages[0].topic, + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + checkHasMessages([]); + check(model).fetched.isFalse(); + checkNotifiedOnce(); + + await fetchFuture; + checkHasMessages([]); + check(model).fetched.isFalse(); + checkNotNotified(); + + async.elapse(const Duration(seconds: 1)); + checkHasMessages(initialMessages + movedMessages); + checkNotifiedOnce(); + })); + + test('fetchInitial, _reset, move fetch finishes, initial fetch finishes', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, null); + + connection.prepare(delay: const Duration(seconds: 2), json: newestResult( + foundOldest: false, + messages: initialMessages, + ).toJson()); + final fetchFuture = model.fetchInitial(); + checkHasMessages([]); + check(model).fetched.isFalse(); + + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: movedMessages[0].topic, + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + checkHasMessages([]); + check(model).fetched.isFalse(); + + async.elapse(const Duration(seconds: 1)); + checkHasMessages(initialMessages + movedMessages); + check(model).fetched.isTrue(); + + await fetchFuture; + checkHasMessages(initialMessages + movedMessages); + })); + + test('fetchOlder #1, _reset, move fetch finishes, fetchOlder #2, ' + 'fetchOlder #1 finishes, fetchOlder #2 finishes', () => awaitFakeAsync((async) async { + await prepareNarrow(narrow, initialMessages); + + connection.prepare(delay: const Duration(seconds: 2), json: olderResult( + anchor: model.messages[0].id, + foundOldest: true, + messages: olderMessages, + ).toJson()); + final fetchFuture1 = model.fetchOlder(); + checkHasMessages(initialMessages); + check(model).fetchingOlder.isTrue(); + checkNotifiedOnce(); + + connection.prepare(delay: const Duration(seconds: 1), json: newestResult( + foundOldest: false, + messages: initialMessages + movedMessages, + ).toJson()); + await store.handleEvent(eg.updateMessageEventMoveTo( + origTopic: movedMessages[0].topic, + origStreamId: otherStream.streamId, + newMessages: movedMessages, + )); + checkHasMessages([]); + check(model).fetchingOlder.isFalse(); + checkNotifiedOnce(); + + async.elapse(const Duration(seconds: 1)); + checkNotifiedOnce(); + + connection.prepare(delay: const Duration(seconds: 2), json: olderResult( + anchor: model.messages[0].id, + foundOldest: true, + messages: olderMessages + ).toJson()); + final fetchFuture2 = model.fetchOlder(); + checkHasMessages(initialMessages + movedMessages); + check(model).fetchingOlder.isTrue(); + checkNotifiedOnce(); + + await fetchFuture1; + checkHasMessages(initialMessages + movedMessages); + // The older fetchOlder call should not override fetchingOlder set by + // the new fetchOlder call, nor should it notify the listeners. + check(model).fetchingOlder.isTrue(); + checkNotNotified(); + + await fetchFuture2; + checkHasMessages(olderMessages + initialMessages + movedMessages); + check(model).fetchingOlder.isFalse(); + checkNotifiedOnce(); + })); + }); + }); + group('regression tests for #455', () { test('reaction events handled once, even when message is in two message lists', () async { final stream = eg.stream(); diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 482d77b2c1..249317310c 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -273,75 +273,98 @@ void main() { }); group('Handle message edit state update', () { - final message = eg.streamMessage(); - final otherMessage = eg.streamMessage(); - - Future sendEvent(Message message, UpdateMessageEvent event) async { + late List origMessages; + + Future prepareOrigMessages({ + required String origTopic, + String? origContent, + }) async { + origMessages = List.generate(2, (i) => eg.streamMessage( + topic: origTopic, + content: origContent, + )); await prepare(); - await prepareMessages([message, otherMessage]); - - await store.handleEvent(event); - checkNotifiedOnce(); + await prepareMessages(origMessages); } test('message not moved update', () async { - await sendEvent(message, eg.updateMessageEditEvent(message)); - check(store).messages[message.id].editState.equals(MessageEditState.edited); - check(store).messages[otherMessage.id].editState.equals(MessageEditState.none); + await prepareOrigMessages(origTopic: 'origTopic'); + await store.handleEvent(eg.updateMessageEditEvent(origMessages[0])); + checkNotifiedOnce(); + check(store).messages[origMessages[0].id].editState.equals(MessageEditState.edited); + check(store).messages[origMessages[1].id].editState.equals(MessageEditState.none); }); test('message topic moved update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: 'old topic', + await prepareOrigMessages(origTopic: 'old topic'); + final originalDisplayRecipient = origMessages[0].displayRecipient!; + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, newTopic: 'new topic')); - check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.moved))); + checkNotified(count: 2); + check(store).messages.values.every(((message) => + message.isA() + ..editState.equals(MessageEditState.moved) + ..displayRecipient.equals(originalDisplayRecipient))); }); test('message topic resolved update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: 'new topic', + await prepareOrigMessages(origTopic: 'new topic'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, newTopic: '✔ new topic')); + checkNotified(count: 2); check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.none))); }); test('message topic unresolved update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: '✔ new topic', + await prepareOrigMessages(origTopic: '✔ new topic'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, newTopic: 'new topic')); + checkNotified(count: 2); check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.none))); }); test('message topic both resolved and edited update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: 'new topic', + await prepareOrigMessages(origTopic: 'new topic'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, newTopic: '✔ new topic 2')); + checkNotified(count: 2); check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.moved))); }); test('message topic both unresolved and edited update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: '✔ new topic', + await prepareOrigMessages(origTopic: '✔ new topic'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, newTopic: 'new topic 2')); + checkNotified(count: 2); check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.moved))); }); - test('message stream moved update', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: 'topic', - newTopic: 'topic', - newStreamId: 20)); - check(store).messages.values.every(((message) => message.editState.equals(MessageEditState.moved))); + test('message stream moved without topic change', () async { + await prepareOrigMessages(origTopic: 'topic'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, + newStreamId: 20)); + checkNotified(count: 2); + check(store).messages.values.every(((message) => + message.isA() + ..editState.equals(MessageEditState.moved) + ..displayRecipient.equals(null))); }); test('message is both moved and updated', () async { - await sendEvent(message, eg.updateMessageMoveEvent([message, otherMessage], - origTopic: 'topic', - newTopic: 'topic', - newStreamId: 20, - origContent: 'old content', + await prepareOrigMessages(origTopic: 'topic', origContent: 'old content'); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: origMessages, + newStreamId: 20, newContent: 'new content')); - check(store).messages[message.id].editState.equals(MessageEditState.edited); - check(store).messages[otherMessage.id].editState.equals(MessageEditState.moved); + checkNotified(count: 2); + check(store).messages[origMessages[0].id].editState.equals(MessageEditState.edited); + check(store).messages[origMessages[1].id].editState.equals(MessageEditState.moved); }); }); }); diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index d65eb2b558..16f50fdc63 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -546,7 +546,7 @@ void main() { route.isA() ..accountId.equals(account.id) ..page.isA() - .narrow.equals(SendableNarrow.ofMessage(message, + .initNarrow.equals(SendableNarrow.ofMessage(message, selfUserId: account.userId)); } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 3b61192f0a..a2e561e477 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -59,7 +59,7 @@ Future setupToMessageActionSheet(WidgetTester tester, { ).toJson()); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: MessageListPage(narrow: narrow))); + child: MessageListPage(initNarrow: narrow))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index b52eafb10f..dcf3fe8516 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -48,7 +48,7 @@ Future setupToComposeInput(WidgetTester tester, { prepareBoringImageHttpClient(); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: MessageListPage(narrow: DmNarrow( + child: MessageListPage(initNarrow: DmNarrow( allRecipientIds: [eg.selfUser.userId, eg.otherUser.userId], selfUserId: eg.selfUser.userId)))); diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index b62dd7502d..3fe1151b25 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -815,7 +815,7 @@ void main() { await tapText(tester, find.text('stream')); check(testBinding.takeLaunchUrlCalls()).isEmpty(); check(pushedRoutes).single.isA() - .page.isA().narrow.equals(const ChannelNarrow(1)); + .page.isA().initNarrow.equals(const ChannelNarrow(1)); }); testWidgets('invalid internal links are opened in browser', (tester) async { diff --git a/test/widgets/message_list_checks.dart b/test/widgets/message_list_checks.dart index 5daa91a1f4..6ce43a2d43 100644 --- a/test/widgets/message_list_checks.dart +++ b/test/widgets/message_list_checks.dart @@ -3,5 +3,5 @@ import 'package:zulip/model/narrow.dart'; import 'package:zulip/widgets/message_list.dart'; extension MessageListPageChecks on Subject { - Subject get narrow => has((x) => x.narrow, 'narrow'); + Subject get initNarrow => has((x) => x.initNarrow, 'initNarrow'); } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 3b13c158e4..071823bf65 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -14,6 +14,7 @@ import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; +import 'package:zulip/widgets/autocomplete.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/emoji_reaction.dart'; import 'package:zulip/widgets/icons.dart'; @@ -69,7 +70,7 @@ void main() { newestResult(foundOldest: foundOldest, messages: messages).toJson()); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: MessageListPage(narrow: narrow))); + child: MessageListPage(initNarrow: narrow))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); @@ -620,6 +621,95 @@ void main() { }); }); + group('Update Narrow on message move', () { + const topic = 'foo'; + final channel = eg.stream(name: 'move test stream'); + final otherChannel = eg.stream(name: 'other move test stream'); + final narrow = TopicNarrow(channel.streamId, topic); + + void prepareGetMessageResponse(List messages) { + connection.prepare(json: newestResult( + foundOldest: false, messages: messages).toJson()); + } + + void handleMessageMoveEvent(List messages, String newTopic, {int? newChannelId}) { + store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: messages, + newTopic: newTopic, + newStreamId: newChannelId, + propagateMode: PropagateMode.changeAll)); + } + + testWidgets('compose box send message after move', (WidgetTester tester) async { + final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); + await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel, otherChannel]); + + final channelContentInputFinder = find.descendant( + of: find.byType(ComposeAutocomplete), + matching: find.byType(TextField)); + + await tester.enterText(channelContentInputFinder, 'Some text'); + check(tester.widget(channelContentInputFinder)) + ..decoration.isNotNull().hintText.equals('Message #${channel.name} > $topic') + ..controller.isNotNull().text.equals('Some text'); + + prepareGetMessageResponse([message]); + handleMessageMoveEvent([message], 'new topic', newChannelId: otherChannel.streamId); + await tester.pump(const Duration(seconds: 1)); + check(tester.widget(channelContentInputFinder)) + ..decoration.isNotNull().hintText.equals('Message #${otherChannel.name} > new topic') + ..controller.isNotNull().text.equals('Some text'); + + connection.prepare(json: SendMessageResult(id: 1).toJson()); + await tester.tap(find.byIcon(Icons.send)); + await tester.pump(); + check(connection.lastRequest).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages') + ..bodyFields.deepEquals({ + 'type': 'stream', + 'to': '${otherChannel.streamId}', + 'topic': 'new topic', + 'content': 'Some text', + 'read_by_sender': 'true'}); + await tester.pumpAndSettle(); + }); + + testWidgets('Move to narrow with existing messages', (WidgetTester tester) async { + final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); + await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]); + check(find.textContaining('Existing message').evaluate()).length.equals(0); + check(find.textContaining('Message to move').evaluate()).length.equals(1); + + final existingMessage = eg.streamMessage( + stream: eg.stream(), topic: 'new topic', content: 'Existing message'); + prepareGetMessageResponse([existingMessage, message]); + handleMessageMoveEvent([message], 'new topic'); + await tester.pump(const Duration(seconds: 1)); + + check(find.textContaining('Existing message').evaluate()).length.equals(1); + check(find.textContaining('Message to move').evaluate()).length.equals(1); + }); + + testWidgets('show new topic in TopicNarrow after move', (tester) async { + final message = eg.streamMessage(stream: channel, topic: topic, content: 'Message to move'); + await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]); + + prepareGetMessageResponse([message]); + handleMessageMoveEvent([message], 'new topic'); + await tester.pump(const Duration(seconds: 1)); + + check(find.descendant( + of: find.byType(RecipientHeader), + matching: find.text('new topic')).evaluate() + ).length.equals(1); + check(find.descendant( + of: find.byType(MessageListAppBarTitle), + matching: find.text('${channel.name} > new topic')).evaluate() + ).length.equals(1); + }); + }); + group('recipient headers', () { group('StreamMessageRecipientHeader', () { final stream = eg.stream(name: 'stream name'); @@ -965,8 +1055,8 @@ void main() { }); testWidgets('edited and moved messages from events', (WidgetTester tester) async { - final message = eg.streamMessage(); - final message2 = eg.streamMessage(); + final message = eg.streamMessage(topic: 'old'); + final message2 = eg.streamMessage(topic: 'old'); await setupMessageListPage(tester, messages: [message, message2]); checkMarkersCount(edited: 0, moved: 0); @@ -974,8 +1064,8 @@ void main() { await tester.pump(); checkMarkersCount(edited: 1, moved: 0); - await store.handleEvent(eg.updateMessageMoveEvent( - [message, message2], origTopic: 'old', newTopic: 'new')); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [message, message2], newTopic: 'new')); await tester.pump(); checkMarkersCount(edited: 1, moved: 1); @@ -1019,19 +1109,25 @@ void main() { testWidgets('edit state updates do not affect layout', (WidgetTester tester) async { final messages = [ - eg.streamMessage(), + eg.streamMessage(topic: 'orig'), eg.streamMessage( + topic: 'orig', reactions: [eg.unicodeEmojiReaction, eg.realmEmojiReaction], flags: [MessageFlag.starred]), - eg.streamMessage(), + eg.streamMessage(topic: 'orig'), ]; final StreamMessage messageWithMarker = messages[1]; await setupMessageListPage(tester, messages: messages); final rectsBefore = captureMessageRects(tester, messages, messageWithMarker); checkMarkersCount(edited: 0, moved: 0); - await store.handleEvent(eg.updateMessageMoveEvent( - [messageWithMarker], origTopic: 'old', newTopic: messageWithMarker.topic)); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [store.messages[messageWithMarker.id] as StreamMessage], + newTopic: 'new')); + await tester.pump(); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [store.messages[messageWithMarker.id] as StreamMessage], + newTopic: 'orig')); await tester.pump(); check(captureMessageRects(tester, messages, messageWithMarker)) .deepEquals(rectsBefore); diff --git a/test/widgets/profile_test.dart b/test/widgets/profile_test.dart index 76435d32ab..651104498a 100644 --- a/test/widgets/profile_test.dart +++ b/test/widgets/profile_test.dart @@ -253,7 +253,7 @@ void main() { await tester.tap(targetWidget); check(pushedRoutes).last.isA().page .isA() - .narrow.equals(DmNarrow.withUser(1, selfUserId: eg.selfUser.userId)); + .initNarrow.equals(DmNarrow.withUser(1, selfUserId: eg.selfUser.userId)); }); testWidgets('page builds; user links render multiple avatars', (WidgetTester tester) async { diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 69817ec5ce..77ace1d7e7 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -322,7 +322,7 @@ void main() { check(pushedRoutes).last.isA().page .isA() - .narrow.equals(expectedNarrow); + .initNarrow.equals(expectedNarrow); } testWidgets('1:1', (WidgetTester tester) async {