diff --git a/lib/model/message.dart b/lib/model/message.dart index 2573cfadc6..f426383508 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -1,21 +1,180 @@ +import 'dart:async'; +import 'dart:collection'; import 'dart:convert'; import 'package:crypto/crypto.dart'; +import 'package:flutter/foundation.dart'; import '../api/model/events.dart'; import '../api/model/model.dart'; import '../api/route/messages.dart'; import '../log.dart'; +import 'binding.dart'; import 'message_list.dart'; import 'store.dart'; const _apiSendMessage = sendMessage; // Bit ugly; for alternatives, see: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20PerAccountStore.20methods/near/1545809 +/// The duration an outbox message stays hidden to the user. +/// +/// See [OutboxMessageState.waiting]. +const kLocalEchoDebounceDuration = Duration(milliseconds: 500); // TODO(#1441) find the right value for this + +/// The duration before an outbox message can be restored for resending, since +/// its creation. +/// +/// See [OutboxMessageState.waitPeriodExpired]. +const kSendMessageOfferRestoreWaitPeriod = Duration(seconds: 10); // TODO(#1441) find the right value for this + +/// States of an [OutboxMessage] since its creation from a +/// [MessageStore.sendMessage] call and before its eventual deletion. +/// +/// ``` +/// Got an [ApiRequestException]. +/// ┌──────┬──────────┬─────────────► failed +/// (create) │ │ │ │ +/// └► hidden waiting waitPeriodExpired ──┴──────────────► (delete) +/// │ ▲ │ ▲ User restores +/// └──────┘ └──────┘ the draft. +/// Debounce [sendMessage] request +/// timed out. not finished when +/// wait period timed out. +/// +/// Event received. +/// (any state) ─────────────────► (delete) +/// ``` +/// +/// During its lifecycle, it is guaranteed that the outbox message is deleted +/// as soon a message event with a matching [MessageEvent.localMessageId] +/// arrives. +enum OutboxMessageState { + /// The [sendMessage] HTTP request has started but the resulting + /// [MessageEvent] hasn't arrived, and nor has the request failed. In this + /// state, the outbox message is hidden to the user. + /// + /// This is the initial state when an [OutboxMessage] is created. + hidden, + + /// The [sendMessage] HTTP request has started but hasn't finished, and the + /// outbox message is shown to the user. + /// + /// This state can be reached after staying in [hidden] for + /// [kLocalEchoDebounceDuration]. + waiting, + + /// The [sendMessage] HTTP request did not finish in time and the user is + /// invited to retry it. + /// + /// This state can be reached when the request has not finished + /// [kSendMessageOfferRestoreWaitPeriod] since the outbox message's creation. + waitPeriodExpired, + + /// The message could not be delivered, and the user is invited to retry it. + /// + /// This state can be reached when we got an [ApiRequestException] from the + /// [sendMessage] HTTP request. + failed, +} + +/// An outstanding request to send a message, aka an outbox-message. +/// +/// This will be shown in the UI in the message list, as a placeholder +/// for the actual [Message] the request is anticipated to produce. +/// +/// A request remains "outstanding" even after the [sendMessage] HTTP request +/// completes, whether with success or failure. +/// The outbox-message persists until either the corresponding [MessageEvent] +/// arrives to replace it, or the user discards it (perhaps to try again). +/// For details, see the state diagram at [OutboxMessageState], +/// and [MessageStore.takeOutboxMessage]. +sealed class OutboxMessage extends MessageBase { + OutboxMessage({ + required this.localMessageId, + required int selfUserId, + required super.timestamp, + required this.contentMarkdown, + }) : _state = OutboxMessageState.hidden, + super(senderId: selfUserId); + + // TODO(dart): This has to be a plain static method, because factories/constructors + // do not support type parameters: https://github.com/dart-lang/language/issues/647 + static OutboxMessage fromConversation(Conversation conversation, { + required int localMessageId, + required int selfUserId, + required int timestamp, + required String contentMarkdown, + }) { + return switch (conversation) { + StreamConversation() => StreamOutboxMessage._( + localMessageId: localMessageId, + selfUserId: selfUserId, + timestamp: timestamp, + conversation: conversation, + contentMarkdown: contentMarkdown), + DmConversation() => DmOutboxMessage._( + localMessageId: localMessageId, + selfUserId: selfUserId, + timestamp: timestamp, + conversation: conversation, + contentMarkdown: contentMarkdown), + }; + } + + /// As in [MessageEvent.localMessageId]. + /// + /// This uniquely identifies this outbox message's corresponding message object + /// in events from the same event queue. + /// + /// See also: + /// * [MessageStoreImpl.sendMessage], where this ID is assigned. + final int localMessageId; + + @override + int? get id => null; + + final String contentMarkdown; + + OutboxMessageState get state => _state; + OutboxMessageState _state; + + /// Whether the [OutboxMessage] is hidden to [MessageListView] or not. + bool get hidden => state == OutboxMessageState.hidden; +} + +class StreamOutboxMessage extends OutboxMessage { + StreamOutboxMessage._({ + required super.localMessageId, + required super.selfUserId, + required super.timestamp, + required this.conversation, + required super.contentMarkdown, + }); + + @override + final StreamConversation conversation; +} + +class DmOutboxMessage extends OutboxMessage { + DmOutboxMessage._({ + required super.localMessageId, + required super.selfUserId, + required super.timestamp, + required this.conversation, + required super.contentMarkdown, + }) : assert(conversation.allRecipientIds.contains(selfUserId)); + + @override + final DmConversation conversation; +} + /// The portion of [PerAccountStore] for messages and message lists. mixin MessageStore { /// All known messages, indexed by [Message.id]. Map get messages; + /// [OutboxMessage]s sent by the user, indexed by [OutboxMessage.localMessageId]. + Map get outboxMessages; + Set get debugMessageListViews; void registerMessageList(MessageListView view); @@ -26,6 +185,15 @@ mixin MessageStore { required String content, }); + /// Remove from [outboxMessages] given the [localMessageId], and return + /// the removed [OutboxMessage]. + /// + /// The outbox message to be taken must exist. + /// + /// The state of the outbox message must be either [OutboxMessageState.failed] + /// or [OutboxMessageState.waitPeriodExpired]. + OutboxMessage takeOutboxMessage(int localMessageId); + /// Reconcile a batch of just-fetched messages with the store, /// mutating the list. /// @@ -66,6 +234,219 @@ mixin MessageStore { ({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId); } +/// Manages the outbox messages portion of [MessageStore]. +mixin _OutboxMessageStore on PerAccountStoreBase { + late final UnmodifiableMapView outboxMessages = + UnmodifiableMapView(_outboxMessages); + final Map _outboxMessages = {}; + + /// A map of timers to show outbox messages after a delay, + /// indexed by [OutboxMessage.localMessageId]. + /// + /// If the send message request fails within the time limit, + /// the outbox message's timer gets removed and cancelled. + final Map _outboxMessageDebounceTimers = {}; + + /// A map of timers to update outbox messages state to + /// [OutboxMessageState.waitPeriodExpired] if the [sendMessage] + /// request did not succeed in time, + /// indexed by [OutboxMessage.localMessageId]. + /// + /// If the send message request fails within the time limit, + /// the outbox message's timer gets removed and cancelled. + final Map _outboxMessageWaitPeriodTimers = {}; + + /// A fresh ID to use for [OutboxMessage.localMessageId], + /// unique within this instance. + int _nextLocalMessageId = 1; + + /// As in [MessageStoreImpl._messageListViews]. + Set get _messageListViews; + + /// As in [MessageStoreImpl._disposed]. + bool get _disposed; + + /// Update the state of the [OutboxMessage] with the given [localMessageId], + /// and notify listeners if necessary. + /// + /// The outbox message with [localMessageId] must exist. + void _updateOutboxMessage(int localMessageId, { + required OutboxMessageState newState, + }) { + assert(!_disposed); + final outboxMessage = outboxMessages[localMessageId]; + if (outboxMessage == null) { + throw StateError( + 'Removing unknown outbox message with localMessageId: $localMessageId'); + } + final oldState = outboxMessage.state; + // See [OutboxMessageState] for valid state transitions. + final isStateTransitionValid = switch (newState) { + OutboxMessageState.hidden => false, + OutboxMessageState.waiting => + oldState == OutboxMessageState.hidden, + OutboxMessageState.waitPeriodExpired => + oldState == OutboxMessageState.waiting, + OutboxMessageState.failed => + oldState == OutboxMessageState.hidden + || oldState == OutboxMessageState.waiting + || oldState == OutboxMessageState.waitPeriodExpired, + }; + if (!isStateTransitionValid) { + throw StateError('Unexpected state transition: $oldState -> $newState'); + } + + outboxMessage._state = newState; + for (final view in _messageListViews) { + if (oldState == OutboxMessageState.hidden) { + view.addOutboxMessage(outboxMessage); + } else { + view.notifyListenersIfOutboxMessagePresent(localMessageId); + } + } + } + + /// Send a message and create an entry of [OutboxMessage]. + Future _outboxSendMessage({ + required MessageDestination destination, + required String content, + required String? realmEmptyTopicDisplayName, + }) async { + assert(!_disposed); + final localMessageId = _nextLocalMessageId++; + assert(!outboxMessages.containsKey(localMessageId)); + + final conversation = switch (destination) { + StreamDestination(:final streamId, :final topic) => + StreamConversation( + streamId, + _processTopicLikeServer( + topic, realmEmptyTopicDisplayName: realmEmptyTopicDisplayName), + displayRecipient: null), + DmDestination(:final userIds) => DmConversation(allRecipientIds: userIds), + }; + + _outboxMessages[localMessageId] = OutboxMessage.fromConversation( + conversation, + localMessageId: localMessageId, + selfUserId: selfUserId, + timestamp: ZulipBinding.instance.utcNow().millisecondsSinceEpoch ~/ 1000, + contentMarkdown: content); + + _outboxMessageDebounceTimers[localMessageId] = Timer( + kLocalEchoDebounceDuration, + () => _handleOutboxDebounce(localMessageId)); + + _outboxMessageWaitPeriodTimers[localMessageId] = Timer( + kSendMessageOfferRestoreWaitPeriod, + () => _handleOutboxWaitPeriodExpired(localMessageId)); + + try { + await _apiSendMessage(connection, + destination: destination, + content: content, + readBySender: true, + queueId: queueId, + localId: localMessageId.toString()); + _outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel(); + } catch (e) { + if (_disposed) return; + if (!_outboxMessages.containsKey(localMessageId)) { + // The message event already arrived; the failure is probably due to + // networking issues. Don't rethrow; the send succeeded + // (we got the event) so we don't want to show an error dialog. + return; + } + _outboxMessageDebounceTimers.remove(localMessageId)?.cancel(); + _outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel(); + _updateOutboxMessage(localMessageId, newState: OutboxMessageState.failed); + rethrow; + } + } + + TopicName _processTopicLikeServer(TopicName topic, { + required String? realmEmptyTopicDisplayName, + }) { + return topic.processLikeServer( + // Processing this just once on creating the outbox message + // allows an uncommon bug, because either of these values can change. + // During the outbox message's life, a topic processed from + // "(no topic)" could become stale/wrong when zulipFeatureLevel + // changes; a topic processed from "general chat" could become + // stale/wrong when realmEmptyTopicDisplayName changes. + // + // Shrug. The same effect is caused by an unavoidable race: + // an admin could change the name of "general chat" + // (i.e. the value of realmEmptyTopicDisplayName) + // concurrently with the user making the send request, + // so that the setting in effect by the time the request arrives + // is different from the setting the client last heard about. + zulipFeatureLevel: zulipFeatureLevel, + realmEmptyTopicDisplayName: realmEmptyTopicDisplayName); + } + + void _handleOutboxDebounce(int localMessageId) { + assert(!_disposed); + assert(outboxMessages.containsKey(localMessageId), + 'The timer should have been canceled when the outbox message was removed.'); + _outboxMessageDebounceTimers.remove(localMessageId); + _updateOutboxMessage(localMessageId, newState: OutboxMessageState.waiting); + } + + void _handleOutboxWaitPeriodExpired(int localMessageId) { + assert(!_disposed); + assert(outboxMessages.containsKey(localMessageId), + 'The timer should have been canceled when the outbox message was removed.'); + assert(!_outboxMessageDebounceTimers.containsKey(localMessageId), + 'The debounce timer should have been removed before the wait period timer expires.'); + _outboxMessageWaitPeriodTimers.remove(localMessageId); + _updateOutboxMessage(localMessageId, newState: OutboxMessageState.waitPeriodExpired); + } + + OutboxMessage takeOutboxMessage(int localMessageId) { + assert(!_disposed); + final removed = _outboxMessages.remove(localMessageId); + _outboxMessageDebounceTimers.remove(localMessageId)?.cancel(); + _outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel(); + if (removed == null) { + throw StateError( + 'Removing unknown outbox message with localMessageId: $localMessageId'); + } + if (removed.state != OutboxMessageState.failed + && removed.state != OutboxMessageState.waitPeriodExpired + ) { + throw StateError('Unexpected state when restoring draft: ${removed.state}'); + } + for (final view in _messageListViews) { + view.removeOutboxMessage(removed); + } + return removed; + } + + void _handleMessageEventOutbox(MessageEvent event) { + if (event.localMessageId != null) { + final localMessageId = int.parse(event.localMessageId!, radix: 10); + // The outbox message can be missing if the user removes it (to be + // implemented in #1441) before the event arrives. + // Nothing to do in that case. + _outboxMessages.remove(localMessageId); + _outboxMessageDebounceTimers.remove(localMessageId)?.cancel(); + _outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel(); + } + } + + /// Cancel [_OutboxMessageStore]'s timers. + void _disposeOutboxMessages() { + assert(!_disposed); + for (final timer in _outboxMessageDebounceTimers.values) { + timer.cancel(); + } + for (final timer in _outboxMessageWaitPeriodTimers.values) { + timer.cancel(); + } + } +} + class _EditMessageRequestStatus { _EditMessageRequestStatus({ required this.hasError, @@ -78,15 +459,29 @@ class _EditMessageRequestStatus { final String newContent; } -class MessageStoreImpl extends PerAccountStoreBase with MessageStore { - MessageStoreImpl({required super.core}) - // There are no messages in InitialSnapshot, so we don't have - // a use case for initializing MessageStore with nonempty [messages]. - : messages = {}; +class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMessageStore { + MessageStoreImpl({required super.core, required String? realmEmptyTopicDisplayName}) + : _realmEmptyTopicDisplayName = realmEmptyTopicDisplayName, + // There are no messages in InitialSnapshot, so we don't have + // a use case for initializing MessageStore with nonempty [messages]. + messages = {}; + + /// The display name to use for empty topics. + /// + /// This should only be accessed when FL >= 334, since topics cannot + /// be empty otherwise. + // TODO(server-10) simplify this + String get realmEmptyTopicDisplayName { + assert(zulipFeatureLevel >= 334); + assert(_realmEmptyTopicDisplayName != null); // TODO(log) + return _realmEmptyTopicDisplayName ?? 'general chat'; + } + final String? _realmEmptyTopicDisplayName; // TODO(#668): update this realm setting @override final Map messages; + @override final Set _messageListViews = {}; @override @@ -94,12 +489,16 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { @override void registerMessageList(MessageListView view) { + assert(!_disposed); final added = _messageListViews.add(view); assert(added); } @override void unregisterMessageList(MessageListView view) { + // TODO: Add `assert(!_disposed);` here once we ensure [PerAccountStore] is + // only disposed after [MessageListView]s with references to it are + // disposed. See [dispose] for details. final removed = _messageListViews.remove(view); assert(removed); } @@ -122,6 +521,9 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { } } + @override + bool _disposed = false; + void dispose() { // Not disposing the [MessageListView]s here, because they are owned by // (i.e., they get [dispose]d by) the [_MessageListState], including in the @@ -137,21 +539,30 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { // [InheritedNotifier] to rebuild in the next frame) before the owner's // `dispose` or `onNewStore` is called. Discussion: // https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/MessageListView.20lifecycle/near/2086893 + + assert(!_disposed); + _disposeOutboxMessages(); + _disposed = true; } @override Future sendMessage({required MessageDestination destination, required String content}) { - // TODO implement outbox; see design at - // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739 - return _apiSendMessage(connection, - destination: destination, - content: content, - readBySender: true, - ); + assert(!_disposed); + if (!debugOutboxEnable) { + return _apiSendMessage(connection, + destination: destination, + content: content, + readBySender: true); + } + return _outboxSendMessage( + destination: destination, content: content, + // TODO move [TopicName.processLikeServer] to a substore, eliminating this + realmEmptyTopicDisplayName: _realmEmptyTopicDisplayName); } @override void reconcileMessages(List messages) { + assert(!_disposed); // What to do when some of the just-fetched messages are already known? // This is common and normal: in particular it happens when one message list // overlaps another, e.g. a stream and a topic within it. @@ -185,6 +596,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { required String originalRawContent, required String newContent, }) async { + assert(!_disposed); if (_editMessageRequests.containsKey(messageId)) { throw StateError('an edit request is already in progress'); } @@ -202,6 +614,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { } catch (e) { // TODO(log) if e is something unexpected + if (_disposed) return; + final status = _editMessageRequests[messageId]; if (status == null) { // The event actually arrived before this request failed @@ -216,6 +630,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { @override ({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId) { + assert(!_disposed); final status = _editMessageRequests.remove(messageId); _notifyMessageListViewsForOneMessage(messageId); if (status == null) { @@ -242,6 +657,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { // See [fetchedMessages] for reasoning. messages[event.message.id] = event.message; + _handleMessageEventOutbox(event); + for (final view in _messageListViews) { view.handleMessageEvent(event); } @@ -435,4 +852,29 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { // [Poll] is responsible for notifying the affected listeners. poll.handleSubmessageEvent(event); } + + /// In debug mode, controls whether outbox messages should be created when + /// [sendMessage] is called. + /// + /// Outside of debug mode, this is always true and the setter has no effect. + static bool get debugOutboxEnable { + bool result = true; + assert(() { + result = _debugOutboxEnable; + return true; + }()); + return result; + } + static bool _debugOutboxEnable = true; + static set debugOutboxEnable(bool value) { + assert(() { + _debugOutboxEnable = value; + return true; + }()); + } + + @visibleForTesting + static void debugReset() { + _debugOutboxEnable = true; + } } diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 2f8777d3ed..8fb5dc7f4a 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -10,6 +10,7 @@ import '../api/route/messages.dart'; import 'algorithms.dart'; import 'channel.dart'; import 'content.dart'; +import 'message.dart'; import 'narrow.dart'; import 'store.dart'; @@ -564,6 +565,20 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } + /// Add [outboxMessage] if it belongs to the view. + void addOutboxMessage(OutboxMessage outboxMessage) { + // TODO(#1441) implement this + } + + /// Remove the [outboxMessage] from the view. + /// + /// This is a no-op if the message is not found. + /// + /// This should only be called from [MessageStore.takeOutboxMessage]. + void removeOutboxMessage(OutboxMessage outboxMessage) { + // TODO(#1441) implement this + } + void handleUserTopicEvent(UserTopicEvent event) { switch (_canAffectVisibility(event)) { case VisibilityEffect.none: @@ -725,6 +740,11 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } + /// Notify listeners if the given outbox message is present in this view. + void notifyListenersIfOutboxMessagePresent(int localMessageId) { + // TODO(#1441) implement this + } + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// This will redo from scratch any computations we can, such as parsing diff --git a/lib/model/store.dart b/lib/model/store.dart index 5f5b19128b..2eea6aacd2 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -498,7 +498,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor typingStartedExpiryPeriod: Duration(milliseconds: initialSnapshot.serverTypingStartedExpiryPeriodMilliseconds), ), channels: channels, - messages: MessageStoreImpl(core: core), + messages: MessageStoreImpl(core: core, + realmEmptyTopicDisplayName: initialSnapshot.realmEmptyTopicDisplayName), unreads: Unreads( initial: initialSnapshot.unreadMsgs, core: core, @@ -733,6 +734,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor @override Map get messages => _messages.messages; @override + Map get outboxMessages => _messages.outboxMessages; + @override void registerMessageList(MessageListView view) => _messages.registerMessageList(view); @override @@ -744,6 +747,9 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor return _messages.sendMessage(destination: destination, content: content); } @override + OutboxMessage takeOutboxMessage(int localMessageId) => + _messages.takeOutboxMessage(localMessageId); + @override void reconcileMessages(List messages) { _messages.reconcileMessages(messages); // TODO(#649) notify [unreads] of the just-fetched messages diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 1a17f70f60..275d71ebbf 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -30,6 +30,7 @@ extension TopicNameChecks on Subject { } extension StreamConversationChecks on Subject { + Subject get topic => has((x) => x.topic, 'topic'); Subject get displayRecipient => has((x) => x.displayRecipient, 'displayRecipient'); } diff --git a/test/example_data.dart b/test/example_data.dart index e0f44f9ddc..53484b8706 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -637,8 +637,8 @@ UserTopicEvent userTopicEvent( ); } -MessageEvent messageEvent(Message message) => - MessageEvent(id: 0, message: message, localMessageId: null); +MessageEvent messageEvent(Message message, {int? localMessageId}) => + MessageEvent(id: 0, message: message, localMessageId: localMessageId?.toString()); DeleteMessageEvent deleteMessageEvent(List messages) { assert(messages.isNotEmpty); diff --git a/test/fake_async_checks.dart b/test/fake_async_checks.dart new file mode 100644 index 0000000000..51c653123a --- /dev/null +++ b/test/fake_async_checks.dart @@ -0,0 +1,6 @@ +import 'package:checks/checks.dart'; +import 'package:fake_async/fake_async.dart'; + +extension FakeTimerChecks on Subject { + Subject get duration => has((t) => t.duration, 'duration'); +} diff --git a/test/model/message_checks.dart b/test/model/message_checks.dart new file mode 100644 index 0000000000..b56cd89a79 --- /dev/null +++ b/test/model/message_checks.dart @@ -0,0 +1,9 @@ +import 'package:checks/checks.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/message.dart'; + +extension OutboxMessageChecks on Subject> { + Subject get localMessageId => has((x) => x.localMessageId, 'localMessageId'); + Subject get state => has((x) => x.state, 'state'); + Subject get hidden => has((x) => x.hidden, 'hidden'); +} diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 1809f0888b..024a0618ae 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -1,14 +1,17 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; import 'package:checks/checks.dart'; import 'package:crypto/crypto.dart'; +import 'package:fake_async/fake_async.dart'; import 'package:http/http.dart' as http; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/submessage.dart'; import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/model/message.dart'; import 'package:zulip/model/message_list.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; @@ -18,12 +21,17 @@ import '../api/model/model_checks.dart'; import '../api/model/submessage_checks.dart'; import '../example_data.dart' as eg; import '../fake_async.dart'; +import '../fake_async_checks.dart'; import '../stdlib_checks.dart'; +import 'binding.dart'; +import 'message_checks.dart'; import 'message_list_test.dart'; import 'store_checks.dart'; import 'test_store.dart'; void main() { + TestZulipBinding.ensureInitialized(); + // These "late" variables are the common state operated on by each test. // Each test case calls [prepare] to initialize them. late Subscription subscription; @@ -42,10 +50,16 @@ void main() { void checkNotifiedOnce() => checkNotified(count: 1); /// Initialize [store] and the rest of the test state. - Future prepare({Narrow narrow = const CombinedFeedNarrow()}) async { - final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId); + Future prepare({ + Narrow narrow = const CombinedFeedNarrow(), + ZulipStream? stream, + int? zulipFeatureLevel, + }) async { + stream ??= eg.stream(streamId: eg.defaultStreamMessageStreamId); subscription = eg.subscription(stream); - store = eg.store(); + final selfAccount = eg.selfAccount.copyWith(zulipFeatureLevel: zulipFeatureLevel); + store = eg.store(account: selfAccount, + initialSnapshot: eg.initialSnapshot(zulipFeatureLevel: zulipFeatureLevel)); await store.addStream(stream); await store.addSubscription(subscription); connection = store.connection as FakeApiConnection; @@ -54,8 +68,12 @@ void main() { ..addListener(() { notifiedCount++; }); + addTearDown(messageList.dispose); check(messageList).fetched.isFalse(); checkNotNotified(); + + // This cleans up possibly pending timers from [MessageStoreImpl]. + addTearDown(store.dispose); } /// Perform the initial message fetch for [messageList]. @@ -76,6 +94,314 @@ void main() { checkNotified(count: messageList.fetched ? messages.length : 0); } + test('dispose cancels pending timers', () => awaitFakeAsync((async) async { + final stream = eg.stream(); + final store = eg.store(); + await store.addStream(stream); + await store.addSubscription(eg.subscription(stream)); + + (store.connection as FakeApiConnection).prepare( + json: SendMessageResult(id: 1).toJson(), + delay: const Duration(seconds: 1)); + unawaited(store.sendMessage( + destination: StreamDestination(stream.streamId, eg.t('topic')), + content: 'content')); + check(async.pendingTimers).deepEquals(>[ + (it) => it.isA().duration.equals(kLocalEchoDebounceDuration), + (it) => it.isA().duration.equals(kSendMessageOfferRestoreWaitPeriod), + (it) => it.isA().duration.equals(const Duration(seconds: 1)), + ]); + + store.dispose(); + check(async.pendingTimers).single.duration.equals(const Duration(seconds: 1)); + })); + + group('sendMessage', () { + final stream = eg.stream(); + final streamDestination = StreamDestination(stream.streamId, eg.t('some topic')); + late StreamMessage message; + + test('outbox messages get unique localMessageId', () async { + await prepare(stream: stream); + await prepareMessages([]); + + for (int i = 0; i < 10; i++) { + connection.prepare(json: SendMessageResult(id: 1).toJson()); + await store.sendMessage(destination: streamDestination, content: 'content'); + } + // [store.outboxMessages] has the same number of keys (localMessageId) + // as the number of sent messages, which are guaranteed to be distinct. + check(store.outboxMessages).keys.length.equals(10); + }); + + void checkState({required OutboxMessageState expectedState}) { + check(store.outboxMessages).values.single.state.equals(expectedState); + } + + Future prepareOutboxMessage({ + MessageDestination? destination, + int? zulipFeatureLevel, + }) async { + message = eg.streamMessage(stream: stream); + await prepare(stream: stream, zulipFeatureLevel: zulipFeatureLevel); + await prepareMessages([eg.streamMessage(stream: stream)]); + connection.prepare(json: SendMessageResult(id: 1).toJson()); + await store.sendMessage( + destination: destination ?? streamDestination, content: 'content'); + } + + late Future outboxMessageFailFuture; + Future prepareOutboxMessageToFailAfterDelay(Duration delay) async { + message = eg.streamMessage(stream: stream); + await prepare(stream: stream); + await prepareMessages([eg.streamMessage(stream: stream)]); + connection.prepare(apiException: eg.apiBadRequest(), delay: delay); + outboxMessageFailFuture = store.sendMessage( + destination: streamDestination, content: 'content'); + } + + test('smoke DM: hidden -> waiting -> (delete)', () => awaitFakeAsync((async) async { + await prepareOutboxMessage(destination: DmDestination( + userIds: [eg.selfUser.userId, eg.otherUser.userId])); + checkState(expectedState: OutboxMessageState.hidden); + + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + await store.handleEvent(eg.messageEvent( + eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]), + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + })); + + test('smoke stream message: hidden -> waiting -> (delete)', () => awaitFakeAsync((async) async { + await prepareOutboxMessage(); + checkState(expectedState: OutboxMessageState.hidden); + + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + })); + + test('hidden -> waiting', () => awaitFakeAsync((async) async { + await prepareOutboxMessage(); + checkState(expectedState: OutboxMessageState.hidden); + + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + // Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after + // the send request was initiated. + async.elapse( + kSendMessageOfferRestoreWaitPeriod - kLocalEchoDebounceDuration); + async.flushTimers(); + // The outbox message should stay in the waiting state; + // it should not transition to waitPeriodExpired. + checkState(expectedState: OutboxMessageState.waiting); + })); + + group('… -> failed', () { + test('hidden -> failed', () => awaitFakeAsync((async) async { + await prepareOutboxMessageToFailAfterDelay(Duration.zero); + checkState(expectedState: OutboxMessageState.hidden); + + await check(outboxMessageFailFuture).throws(); + checkState(expectedState: OutboxMessageState.failed); + + // Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after + // the send request was initiated. + async.elapse(kSendMessageOfferRestoreWaitPeriod); + async.flushTimers(); + // The outbox message should stay in the failed state; + // it should not transition to waitPeriodExpired. + checkState(expectedState: OutboxMessageState.failed); + })); + + test('waiting -> failed', () => awaitFakeAsync((async) async { + await prepareOutboxMessageToFailAfterDelay( + kLocalEchoDebounceDuration + Duration(seconds: 1)); + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + await check(outboxMessageFailFuture).throws(); + async.elapse(Duration(seconds: 1)); + checkState(expectedState: OutboxMessageState.failed); + })); + + test('waitPeriodExpired -> failed', () => awaitFakeAsync((async) async { + await prepareOutboxMessageToFailAfterDelay( + kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1)); + async.elapse(kSendMessageOfferRestoreWaitPeriod); + checkState(expectedState: OutboxMessageState.waitPeriodExpired); + + await check(outboxMessageFailFuture).throws(); + async.elapse(Duration(seconds: 1)); + checkState(expectedState: OutboxMessageState.failed); + })); + }); + + group('… -> (delete)', () { + test('hidden -> (delete) because event received', () => awaitFakeAsync((async) async { + await prepareOutboxMessage(); + checkState(expectedState: OutboxMessageState.hidden); + + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + })); + + test('hidden -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async { + // Set up an error to fail `sendMessage` with a delay, leaving time for + // the message event to arrive. + await prepareOutboxMessageToFailAfterDelay(const Duration(seconds: 1)); + checkState(expectedState: OutboxMessageState.hidden); + + // Handle the message event while the message is being sent. + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + + // Complete the send request. There should be no error despite + // the send request failure, because the outbox message is not + // in the store any more. + await check(outboxMessageFailFuture).completes(); + async.elapse(const Duration(seconds: 1)); + })); + + test('waiting -> (delete) because event received', () => awaitFakeAsync((async) async { + await prepareOutboxMessage(); + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + })); + + test('waiting -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async { + // Set up an error to fail `sendMessage` with a delay, leaving time for + // the message event to arrive. + await prepareOutboxMessageToFailAfterDelay( + kLocalEchoDebounceDuration + Duration(seconds: 1)); + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + // Handle the message event while the message is being sent. + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + + // Complete the send request. There should be no error despite + // the send request failure, because the outbox message is not + // in the store any more. + await check(outboxMessageFailFuture).completes(); + })); + + test('waiting -> waitPeriodExpired -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async { + // Set up an error to fail `sendMessage` with a delay, leaving time for + // the message event to arrive. + await prepareOutboxMessageToFailAfterDelay( + kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1)); + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + async.elapse(kSendMessageOfferRestoreWaitPeriod - kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waitPeriodExpired); + + // Handle the message event while the message is being sent. + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + + // Complete the send request. There should be no error despite + // the send request failure, because the outbox message is not + // in the store any more. + await check(outboxMessageFailFuture).completes(); + })); + + test('waiting -> waitPeriodExpired -> (delete) because outbox message was taken', () => awaitFakeAsync((async) async { + // Set up an error to fail `sendMessage` with a delay, leaving time for + // the outbox message to be taken (by the user, presumably). + await prepareOutboxMessageToFailAfterDelay( + kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1)); + async.elapse(kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waiting); + + async.elapse(kSendMessageOfferRestoreWaitPeriod - kLocalEchoDebounceDuration); + checkState(expectedState: OutboxMessageState.waitPeriodExpired); + + store.takeOutboxMessage(store.outboxMessages.keys.single); + check(store.outboxMessages).isEmpty(); + })); + + test('failed -> (delete) because event received', () => awaitFakeAsync((async) async { + await prepareOutboxMessageToFailAfterDelay(Duration.zero); + await check(outboxMessageFailFuture).throws(); + checkState(expectedState: OutboxMessageState.failed); + + await store.handleEvent(eg.messageEvent(message, + localMessageId: store.outboxMessages.keys.single)); + check(store.outboxMessages).isEmpty(); + })); + + test('failed -> (delete) because outbox message was taken', () => awaitFakeAsync((async) async { + await prepareOutboxMessageToFailAfterDelay(Duration.zero); + await check(outboxMessageFailFuture).throws(); + checkState(expectedState: OutboxMessageState.failed); + + store.takeOutboxMessage(store.outboxMessages.keys.single); + check(store.outboxMessages).isEmpty(); + })); + }); + + test('when sending to "(no topic)", process topic like the server does when creating outbox message', () => awaitFakeAsync((async) async { + await prepareOutboxMessage( + destination: StreamDestination(stream.streamId, TopicName('(no topic)')), + zulipFeatureLevel: 370); + async.elapse(kLocalEchoDebounceDuration); + check(store.outboxMessages.values.single) + .conversation.isA().topic.equals(eg.t('')); + })); + + test('legacy: when sending to "(no topic)", process topic like the server does when creating outbox message', () => awaitFakeAsync((async) async { + await prepareOutboxMessage( + destination: StreamDestination(stream.streamId, TopicName('(no topic)')), + zulipFeatureLevel: 369); + async.elapse(kLocalEchoDebounceDuration); + check(store.outboxMessages.values.single) + .conversation.isA().topic.equals(eg.t('(no topic)')); + })); + + test('set timestamp to now when creating outbox messages', () => awaitFakeAsync( + initialTime: eg.timeInPast, + (async) async { + await prepareOutboxMessage(); + check(store.outboxMessages.values.single) + .timestamp.equals(eg.utcTimestamp(eg.timeInPast)); + }, + )); + }); + + test('takeOutboxMessage', () async { + final stream = eg.stream(); + await prepare(stream: stream); + await prepareMessages([]); + + for (int i = 0; i < 10; i++) { + connection.prepare(apiException: eg.apiBadRequest()); + await check(store.sendMessage( + destination: StreamDestination(stream.streamId, eg.t('topic')), + content: 'content')).throws(); + } + + final localMessageIds = store.outboxMessages.keys.toList(); + store.takeOutboxMessage(localMessageIds.removeAt(5)); + check(store.outboxMessages.keys).deepEquals(localMessageIds); + }); + group('reconcileMessages', () { test('from empty', () async { await prepare(); diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart index 06c82ed117..9d68873670 100644 --- a/test/model/narrow_test.dart +++ b/test/model/narrow_test.dart @@ -2,38 +2,37 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/message.dart'; import 'package:zulip/model/narrow.dart'; import '../example_data.dart' as eg; import 'narrow_checks.dart'; -/// A [MessageBase] subclass for testing. -// TODO(#1441): switch to outbox-messages instead -sealed class _TestMessage extends MessageBase { - @override - final int? id = null; - - _TestMessage() : super(senderId: eg.selfUser.userId, timestamp: 123456789); -} - -class _TestStreamMessage extends _TestMessage { - @override - final StreamConversation conversation; - - _TestStreamMessage({required ZulipStream stream, required String topic}) - : conversation = StreamConversation( - stream.streamId, TopicName(topic), displayRecipient: null); -} - -class _TestDmMessage extends _TestMessage { - @override - final DmConversation conversation; - - _TestDmMessage({required List allRecipientIds}) - : conversation = DmConversation(allRecipientIds: allRecipientIds); -} - void main() { + int nextLocalMessageId = 1; + + StreamOutboxMessage streamOutboxMessage({ + required ZulipStream stream, + required String topic, + }) { + return OutboxMessage.fromConversation( + StreamConversation( + stream.streamId, TopicName(topic), displayRecipient: null), + localMessageId: nextLocalMessageId++, + selfUserId: eg.selfUser.userId, + timestamp: 123456789, + contentMarkdown: 'content') as StreamOutboxMessage; + } + + DmOutboxMessage dmOutboxMessage({required List allRecipientIds}) { + return OutboxMessage.fromConversation( + DmConversation(allRecipientIds: allRecipientIds), + localMessageId: nextLocalMessageId++, + selfUserId: allRecipientIds[0], + timestamp: 123456789, + contentMarkdown: 'content') as DmOutboxMessage; + } + group('SendableNarrow', () { test('ofMessage: stream message', () { final message = eg.streamMessage(); @@ -61,11 +60,11 @@ void main() { eg.streamMessage(stream: stream, topic: 'topic'))).isTrue(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [1]))).isFalse(); + dmOutboxMessage(allRecipientIds: [1]))).isFalse(); check(narrow.containsMessage( - _TestStreamMessage(stream: otherStream, topic: 'topic'))).isFalse(); + streamOutboxMessage(stream: otherStream, topic: 'topic'))).isFalse(); check(narrow.containsMessage( - _TestStreamMessage(stream: stream, topic: 'topic'))).isTrue(); + streamOutboxMessage(stream: stream, topic: 'topic'))).isTrue(); }); }); @@ -91,13 +90,13 @@ void main() { eg.streamMessage(stream: stream, topic: 'topic'))).isTrue(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [1]))).isFalse(); + dmOutboxMessage(allRecipientIds: [1]))).isFalse(); check(narrow.containsMessage( - _TestStreamMessage(stream: otherStream, topic: 'topic'))).isFalse(); + streamOutboxMessage(stream: otherStream, topic: 'topic'))).isFalse(); check(narrow.containsMessage( - _TestStreamMessage(stream: stream, topic: 'topic2'))).isFalse(); + streamOutboxMessage(stream: stream, topic: 'topic2'))).isFalse(); check(narrow.containsMessage( - _TestStreamMessage(stream: stream, topic: 'topic'))).isTrue(); + streamOutboxMessage(stream: stream, topic: 'topic'))).isTrue(); }); }); @@ -223,13 +222,13 @@ void main() { final narrow = DmNarrow(allRecipientIds: [1, 2], selfUserId: 2); check(narrow.containsMessage( - _TestStreamMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); + streamOutboxMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [2]))).isFalse(); + dmOutboxMessage(allRecipientIds: [2]))).isFalse(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [2, 3]))).isFalse(); + dmOutboxMessage(allRecipientIds: [2, 3]))).isFalse(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [1, 2]))).isTrue(); + dmOutboxMessage(allRecipientIds: [1, 2]))).isTrue(); }); }); @@ -245,9 +244,9 @@ void main() { eg.streamMessage(flags: [MessageFlag.wildcardMentioned]))).isTrue(); check(narrow.containsMessage( - _TestStreamMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); + streamOutboxMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [eg.selfUser.userId]))).isFalse(); + dmOutboxMessage(allRecipientIds: [eg.selfUser.userId]))).isFalse(); }); }); @@ -261,9 +260,9 @@ void main() { eg.streamMessage(flags:[MessageFlag.starred]))).isTrue(); check(narrow.containsMessage( - _TestStreamMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); + streamOutboxMessage(stream: eg.stream(), topic: 'topic'))).isFalse(); check(narrow.containsMessage( - _TestDmMessage(allRecipientIds: [eg.selfUser.userId]))).isFalse(); + dmOutboxMessage(allRecipientIds: [eg.selfUser.userId]))).isFalse(); }); }); } diff --git a/test/model/store_test.dart b/test/model/store_test.dart index ef0a7a72be..d5eb4c739f 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -569,7 +569,8 @@ void main() { group('PerAccountStore.sendMessage', () { test('smoke', () async { - final store = eg.store(); + final store = eg.store(initialSnapshot: eg.initialSnapshot( + queueId: 'fb67bf8a-c031-47cc-84cf-ed80accacda8')); final connection = store.connection as FakeApiConnection; final stream = eg.stream(); connection.prepare(json: SendMessageResult(id: 12345).toJson()); @@ -585,6 +586,8 @@ void main() { 'topic': 'world', 'content': 'hello', 'read_by_sender': 'true', + 'queue_id': 'fb67bf8a-c031-47cc-84cf-ed80accacda8', + 'local_id': store.outboxMessages.keys.single.toString(), }); }); }); diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 679f4de190..11467cea7d 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -15,6 +15,7 @@ import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/localizations.dart'; +import 'package:zulip/model/message.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; @@ -295,6 +296,8 @@ void main() { Future prepareWithContent(WidgetTester tester, String content) async { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); final narrow = ChannelNarrow(channel.streamId); await prepareComposeBox(tester, narrow: narrow, streams: [channel]); @@ -332,6 +335,8 @@ void main() { Future prepareWithTopic(WidgetTester tester, String topic) async { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); final narrow = ChannelNarrow(channel.streamId); await prepareComposeBox(tester, narrow: narrow, streams: [channel]); @@ -723,6 +728,8 @@ void main() { }); testWidgets('hitting send button sends a "typing stopped" notice', (tester) async { + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); await prepareComposeBox(tester, narrow: narrow, streams: [channel]); await checkStartTyping(tester, narrow); @@ -829,6 +836,8 @@ void main() { }) async { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; await prepareComposeBox(tester, narrow: eg.topicNarrow(123, 'some topic'), @@ -883,6 +892,8 @@ void main() { }) async { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); channel = eg.stream(); final narrow = ChannelNarrow(channel.streamId); @@ -1419,6 +1430,8 @@ void main() { int msgIdInNarrow(Narrow narrow) => msgInNarrow(narrow).id; Future prepareEditMessage(WidgetTester tester, {required Narrow narrow}) async { + MessageStoreImpl.debugOutboxEnable = false; + addTearDown(MessageStoreImpl.debugReset); await prepareComposeBox(tester, narrow: narrow, streams: [channel]); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 69a7204827..3dae4b4be2 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -905,7 +905,8 @@ void main() { connection.prepare(json: SendMessageResult(id: 1).toJson()); await tester.tap(find.byIcon(ZulipIcons.send)); - await tester.pump(); + await tester.pump(Duration.zero); + final localMessageId = store.outboxMessages.keys.single; check(connection.lastRequest).isA() ..method.equals('POST') ..url.path.equals('/api/v1/messages') @@ -914,8 +915,12 @@ void main() { 'to': '${otherChannel.streamId}', 'topic': 'new topic', 'content': 'Some text', - 'read_by_sender': 'true'}); - await tester.pumpAndSettle(); + 'read_by_sender': 'true', + 'queue_id': store.queueId, + 'local_id': localMessageId.toString()}); + // Remove the outbox message and its timers created when sending message. + await store.handleEvent( + eg.messageEvent(message, localMessageId: localMessageId)); }); testWidgets('Move to narrow with existing messages', (tester) async {