channel: Maintain maps consistently when removing subscriptions

When a subscription is removed, the data in streams and streamsByName
are left as instances of `Subscription`.  This is inconsistent with the
data store's assumption that unsubscribed channels should have plain
`ZulipStream` instances.

This doesn't seem to have user-facing effects, though.  The assertion
errors only affect debug builds.

CZO discussion:
  https://chat.zulip.org/#narrow/channel/48-mobile/topic/Unsubscribe.20then.20resubscribe.20to.20channel/with/2160241

Author: PIG208

lib/api/model/model.dart

@@ -363,6 +363,24 @@ class ZulipStream {
     required this.streamWeeklyTraffic,
   });
 
+  /// Construct a plain [ZulipStream] from [subscription].
+  factory ZulipStream.fromSubscription(Subscription subscription) {
+    return ZulipStream(
+      streamId: subscription.streamId,
+      name: subscription.name,
+      description: subscription.description,
+      renderedDescription: subscription.renderedDescription,
+      dateCreated: subscription.dateCreated,
+      firstMessageId: subscription.firstMessageId,
+      inviteOnly: subscription.inviteOnly,
+      isWebPublic: subscription.isWebPublic,
+      historyPublicToSubscribers: subscription.historyPublicToSubscribers,
+      messageRetentionDays: subscription.messageRetentionDays,
+      channelPostPolicy: subscription.channelPostPolicy,
+      streamWeeklyTraffic: subscription.streamWeeklyTraffic,
+    );
+  }
+
   factory ZulipStream.fromJson(Map<String, dynamic> json) =>
     _$ZulipStreamFromJson(json);
 

lib/model/channel.dart

@@ -301,7 +301,16 @@ class ChannelStoreImpl with ChannelStore {
 
       case SubscriptionRemoveEvent():
         for (final streamId in event.streamIds) {
-          subscriptions.remove(streamId);
+          assert(streams.containsKey(streamId));
+          assert(streams[streamId] is Subscription);
+          assert(streamsByName.containsKey(streams[streamId]!.name));
+          assert(streamsByName[streams[streamId]!.name] is Subscription);
+          assert(subscriptions.containsKey(streamId));
+          final subscription = subscriptions.remove(streamId);
+          if (subscription == null) continue; // TODO(log)
+          final stream = ZulipStream.fromSubscription(subscription);
+          streams[streamId] = stream;
+          streamsByName[subscription.name] = stream;
         }
 
       case SubscriptionUpdateEvent():

test/model/channel_test.dart

@@ -68,6 +68,23 @@ void main() {
       ));
       checkUnified(store);
     });
+
+    test('unsubscribed then subscribed by events', () async {
+      // Regression test for: https://chat.zulip.org/#narrow/channel/48-mobile/topic/Unsubscribe.20then.20resubscribe.20to.20channel/with/2160241
+      final stream = eg.stream();
+      final store = eg.store();
+      await store.addStream(stream);
+      await store.addSubscription(eg.subscription(stream));
+      checkUnified(store);
+
+      await store.handleEvent(SubscriptionRemoveEvent(id: 1,
+        streamIds: [stream.streamId]));
+      checkUnified(store);
+
+      await store.handleEvent(SubscriptionAddEvent(id: 1,
+        subscriptions: [eg.subscription(stream)]));
+      checkUnified(store);
+    });
   });
 
   group('SubscriptionEvent', () {