msglist: In unsubscribed channel, clear outbox message on send success

This fixes half of the "third buggy behavior" described in #1798:
the send-message case. We'll fix it for the edit-message case too,
coming up.

Fixes-partly: #1798

Author: chrisbobbe

lib/model/message.dart

@@ -889,13 +889,18 @@ const kSendMessageOfferRestoreWaitPeriod = Duration(seconds: 10);  // TODO(#1441
 ///         timed out.   not finished when
 ///                      wait period timed out.
 ///
-///              Event received.
-/// (any state) ─────────────────► (delete)
+///              Event received, or [sendMessage]
+///              request succeeds and we're sending to
+///              an unsubscribed channel.
+/// (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.
+/// If we're sending to an unsubscribed channel, we don't expect an event
+/// (see "third buggy behavior" in #1798) so in that case
+/// the outbox message is deleted when the [sendMessage] request succeeds.
 enum OutboxMessageState {
   /// The [sendMessage] HTTP request has started but the resulting
   /// [MessageEvent] hasn't arrived, and nor has the request failed.  In this
@@ -1148,6 +1153,19 @@ mixin _OutboxMessageStore on HasChannelStore {
       // The message event already arrived; nothing to do.
       return;
     }
+
+    if (destination is StreamDestination && subscriptions[destination.streamId] == null) {
+      // We don't expect an event (we're sending to an unsubscribed channel);
+      // clear the loading spinner.
+      _outboxMessages.remove(localMessageId);
+      _outboxMessageDebounceTimers.remove(localMessageId)?.cancel();
+      _outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel();
+      for (final view in _messageListViews) {
+        view.notifyListenersIfOutboxMessagePresent(localMessageId);
+      }
+      return;
+    }
+
     // The send request succeeded, so the message was definitely sent.
     // Cancel the timer that would have had us start presuming that the
     // send might have failed.

test/model/message_test.dart

@@ -53,6 +53,7 @@ void main() {
 
   /// Initialize [store] and the rest of the test state.
   Future<void> prepare({
+    Narrow narrow = const CombinedFeedNarrow(),
     ZulipStream? stream,
     bool isChannelSubscribed = true,
     int? zulipFeatureLevel,
@@ -71,7 +72,7 @@ void main() {
     connection = store.connection as FakeApiConnection;
     notifiedCount = 0;
     messageList = MessageListView.init(store: store,
-        narrow: const CombinedFeedNarrow(),
+        narrow: narrow,
         anchor: AnchorCode.newest)
       ..addListener(() {
         notifiedCount++;
@@ -172,11 +173,17 @@ void main() {
       check(store.outboxMessages).values.single.state;
 
     Future<void> prepareOutboxMessage({
+      Narrow narrow = const CombinedFeedNarrow(),
       MessageDestination? destination,
+      bool isChannelSubscribed = true,
       int? zulipFeatureLevel,
     }) async {
       message = eg.streamMessage(stream: stream);
-      await prepare(stream: stream, zulipFeatureLevel: zulipFeatureLevel);
+      await prepare(
+        narrow: narrow,
+        stream: stream,
+        isChannelSubscribed: isChannelSubscribed,
+        zulipFeatureLevel: zulipFeatureLevel);
       await prepareMessages([eg.streamMessage(stream: stream)]);
       connection.prepare(json: SendMessageResult(id: 1).toJson());
       await store.sendMessage(
@@ -368,6 +375,16 @@ void main() {
         checkNotNotified();
       }));
 
+      test('hidden -> (delete) because send succeeded in unsubscribed channel', () => awaitFakeAsync((async) async {
+        await prepareOutboxMessage(
+          // Not CombinedFeedNarrow,
+          // which always hides outbox messages in unsubscribed channels.
+          narrow: ChannelNarrow(stream.streamId),
+          isChannelSubscribed: false);
+        check(store.outboxMessages).isEmpty();
+        checkNotNotified();
+      }));
+
       test('waiting -> (delete) because event received', () => awaitFakeAsync((async) async {
         await prepareOutboxMessage();
         async.elapse(kLocalEchoDebounceDuration);
@@ -400,6 +417,28 @@ void main() {
         checkNotNotified();
       }));
 
+      test('waiting -> (delete) because send succeeded in unsubscribed channel', () => awaitFakeAsync((async) async {
+        await prepare(
+          // Not CombinedFeedNarrow,
+          // which always hides outbox messages in unsubscribed channels.
+          narrow: ChannelNarrow(stream.streamId),
+          stream: stream,
+          isChannelSubscribed: false);
+        await prepareMessages([eg.streamMessage(stream: stream)]);
+        connection.prepare(json: SendMessageResult(id: 1).toJson(),
+          delay: kLocalEchoDebounceDuration + Duration(seconds: 1));
+        final future = store.sendMessage(
+          destination: streamDestination, content: 'content');
+        async.elapse(kLocalEchoDebounceDuration);
+        checkState().equals(OutboxMessageState.waiting);
+        checkNotifiedOnce();
+
+        async.elapse(Duration(seconds: 1));
+        await check(future).completes();
+        check(store.outboxMessages).isEmpty();
+        checkNotifiedOnce();
+      }));
+
       test('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.
@@ -435,6 +474,27 @@ void main() {
         checkNotifiedOnce();
       }));
 
+      test('waitPeriodExpired -> (delete) because send succeeded in unsubscribed channel', () => awaitFakeAsync((async) async {
+        await prepare(
+          // Not CombinedFeedNarrow,
+          // which always hides outbox messages in unsubscribed channels.
+          narrow: ChannelNarrow(stream.streamId),
+          isChannelSubscribed: false);
+        await prepareMessages([eg.streamMessage(stream: stream)]);
+        connection.prepare(json: SendMessageResult(id: 1).toJson(),
+          delay: kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
+        final future = store.sendMessage(
+          destination: streamDestination, content: 'content');
+        async.elapse(kSendMessageOfferRestoreWaitPeriod);
+        checkState().equals(OutboxMessageState.waitPeriodExpired);
+        checkNotified(count: 2);
+
+        async.elapse(Duration(seconds: 1));
+        await check(future).completes();
+        check(store.outboxMessages).isEmpty();
+        checkNotifiedOnce();
+      }));
+
       test('failed -> (delete) because event received', () => awaitFakeAsync((async) async {
         await prepareOutboxMessageToFailAfterDelay(Duration.zero);
         await check(outboxMessageFailFuture).throws();

test/widgets/compose_box_test.dart

@@ -1022,9 +1022,9 @@ void main() {
         of: find.byType(MessageWithPossibleSender),
         matching: find.text('hello world'))
       ).findsOne();
-      // TODO(#1798) There's actually a lingering OutboxMessageWithPossibleSender,
-      //   too (the "third buggy behavior" in #1798). We'll fix that soon,
-      //   and it'll be convenient to test that here too.
+      // Regression coverage for the "third buggy behavior"
+      // in https://github.com/zulip/zulip-flutter/issues/1798 .
+      check(find.byType(OutboxMessageWithPossibleSender)).findsNothing();
     });
   });