user_topics: Treat invalid visibility_policy values as null

Previously, UserTopicVisibilityPolicy included an .unknown value to
represent unexpected server values.
This change removes that value and instead invalid or missing
visibility_policy values deserialize as null. Did the same for UserTopicItem.visibilityPolicy, so unknown/missing values in the initial snapshot user_topics data are represented as null.handleUserTopicEvent- For events with visibilityPolicy == null, removed any existing entry for that topic and dropped the per‑stream map if it becomes empty.Tests added
Fixes #1074

Author: Cairo09

lib/api/model/events.dart

@@ -1051,7 +1051,8 @@ class UserTopicEvent extends Event {
   final int streamId;
   final TopicName topicName;
   final int lastUpdated;
-  final UserTopicVisibilityPolicy visibilityPolicy;
+  @JsonKey(unknownEnumValue: JsonKey.nullForUndefinedEnumValue)
+  final UserTopicVisibilityPolicy? visibilityPolicy;
 
   UserTopicEvent({
     required super.id,

lib/api/model/events.g.dart

@@ -370,8 +370,8 @@ class UserTopicItem {
   final int streamId;
   final TopicName topicName;
   final int lastUpdated;
-  @JsonKey(unknownEnumValue: UserTopicVisibilityPolicy.unknown)
-  final UserTopicVisibilityPolicy visibilityPolicy;
+  @JsonKey(unknownEnumValue: JsonKey.nullForUndefinedEnumValue)
+  final UserTopicVisibilityPolicy? visibilityPolicy;
 
   UserTopicItem({
     required this.streamId,

lib/api/model/initial_snapshot.dart

@@ -899,8 +899,8 @@ enum UserTopicVisibilityPolicy {
   none(apiValue: 0),
   muted(apiValue: 1),
   unmuted(apiValue: 2), // TODO(server-7) newly added
-  followed(apiValue: 3), // TODO(server-8) newly added
-  unknown(apiValue: null); // TODO(#1074) remove this
+  followed(apiValue: 3); // TODO(server-8) newly added
+   //Removed unknown (#1074)
 
   const UserTopicVisibilityPolicy({required this.apiValue});
 

lib/api/model/initial_snapshot.g.dart

@@ -119,7 +119,6 @@ Future<void> updateUserTopic(ApiConnection connection, {
   required TopicName topic,
   required UserTopicVisibilityPolicy visibilityPolicy,
 }) {
-  assert(visibilityPolicy != UserTopicVisibilityPolicy.unknown);
   assert(connection.zulipFeatureLevel! >= 170);
   return connection.post('updateUserTopic', (_) {}, 'user_topics', {
     'stream_id': streamId,

lib/api/model/model.dart

@@ -45,6 +45,16 @@ mixin ChannelStore on UserStore {
   /// All the channel folders, including archived ones, indexed by ID.
   Map<int, ChannelFolder> get channelFolders;
 
+  static bool _warnInvalidVisibilityPolicy(
+      UserTopicVisibilityPolicy? visibilityPolicy
+      ) {
+    if (visibilityPolicy == null) {
+      // Not a value we expect. Keep it out of our data structures. // TODO(log)
+      return true;
+    }
+    return false;
+  }
+
   static int compareChannelsByName(ZulipStream a, ZulipStream b) {
     // A user gave feedback wanting zulip-flutter to match web in putting
     // emoji-prefixed channels first; see #1202.
@@ -118,9 +128,14 @@ mixin ChannelStore on UserStore {
   UserTopicVisibilityEffect willChangeIfTopicVisibleInStream(UserTopicEvent event) {
     final streamId = event.streamId;
     final topic = event.topicName;
+    final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
+    if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
+      return UserTopicVisibilityEffect.none;
+    }
+    final UserTopicVisibilityPolicy policy = visibilityPolicy!;
     return UserTopicVisibilityEffect._fromBeforeAfter(
       _isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic)),
-      _isTopicVisibleInStream(event.visibilityPolicy));
+      _isTopicVisibleInStream(policy));
   }
 
   static bool _isTopicVisibleInStream(UserTopicVisibilityPolicy policy) {
@@ -132,9 +147,6 @@ mixin ChannelStore on UserStore {
       case UserTopicVisibilityPolicy.unmuted:
       case UserTopicVisibilityPolicy.followed:
         return true;
-      case UserTopicVisibilityPolicy.unknown:
-        assert(false);
-        return true;
     }
   }
 
@@ -155,9 +167,14 @@ mixin ChannelStore on UserStore {
   UserTopicVisibilityEffect willChangeIfTopicVisible(UserTopicEvent event) {
     final streamId = event.streamId;
     final topic = event.topicName;
+    final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
+    if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
+      return UserTopicVisibilityEffect.none;
+    }
+    final UserTopicVisibilityPolicy policy = visibilityPolicy!;
     return UserTopicVisibilityEffect._fromBeforeAfter(
       _isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic)),
-      _isTopicVisible(streamId, event.visibilityPolicy));
+      _isTopicVisible(streamId, policy));
   }
 
   bool _isTopicVisible(int streamId, UserTopicVisibilityPolicy policy) {
@@ -173,9 +190,6 @@ mixin ChannelStore on UserStore {
       case UserTopicVisibilityPolicy.unmuted:
       case UserTopicVisibilityPolicy.followed:
         return true;
-      case UserTopicVisibilityPolicy.unknown:
-        assert(false);
-        return true;
     }
   }
 
@@ -332,12 +346,13 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
 
     final topicVisibility = <int, TopicKeyedMap<UserTopicVisibilityPolicy>>{};
     for (final item in initialSnapshot.userTopics) {
-      if (_warnInvalidVisibilityPolicy(item.visibilityPolicy)) {
-        // Not a value we expect. Keep it out of our data structures. // TODO(log)
+      final UserTopicVisibilityPolicy? visibilityPolicy = item.visibilityPolicy;
+      if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
         continue;
       }
+      final UserTopicVisibilityPolicy policy = visibilityPolicy!;
       final forStream = topicVisibility.putIfAbsent(item.streamId, () => makeTopicKeyedMap());
-      forStream[item.topicName] = item.visibilityPolicy;
+      forStream[item.topicName] = policy;
     }
 
     return ChannelStoreImpl._(
@@ -378,13 +393,6 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
     return topicVisibility[streamId]?[topic] ?? UserTopicVisibilityPolicy.none;
   }
 
-  static bool _warnInvalidVisibilityPolicy(UserTopicVisibilityPolicy visibilityPolicy) {
-    if (visibilityPolicy == UserTopicVisibilityPolicy.unknown) {
-      // Not a value we expect. Keep it out of our data structures. // TODO(log)
-      return true;
-    }
-    return false;
-  }
 
   void handleChannelEvent(ChannelEvent event) {
     switch (event) {
@@ -558,13 +566,18 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
   }
 
   void handleUserTopicEvent(UserTopicEvent event) {
-    UserTopicVisibilityPolicy visibilityPolicy = event.visibilityPolicy;
-    if (_warnInvalidVisibilityPolicy(visibilityPolicy)) {
-      visibilityPolicy = UserTopicVisibilityPolicy.none;
+    final UserTopicVisibilityPolicy? visibilityPolicy = event.visibilityPolicy;
+    if (ChannelStore._warnInvalidVisibilityPolicy(visibilityPolicy)) {
+      final forStream = topicVisibility[event.streamId];
+      if (forStream == null) return;
+      forStream.remove(event.topicName);
+      if (forStream.isEmpty) {
+        topicVisibility.remove(event.streamId);
+      }
+      return;
     }
-    if (visibilityPolicy == UserTopicVisibilityPolicy.none) {
-      // This is the "zero value" for this type, which our data structure
-      // represents by leaving the topic out entirely.
+    final UserTopicVisibilityPolicy policy = visibilityPolicy!;
+    if (policy == UserTopicVisibilityPolicy.none) {
       final forStream = topicVisibility[event.streamId];
       if (forStream == null) return;
       forStream.remove(event.topicName);
@@ -573,7 +586,7 @@ class ChannelStoreImpl extends HasUserStore with ChannelStore {
       }
     } else {
       final forStream = topicVisibility.putIfAbsent(event.streamId, () => makeTopicKeyedMap());
-      forStream[event.topicName] = visibilityPolicy;
+      forStream[event.topicName] = policy;
     }
   }
 }

lib/api/route/channels.dart

@@ -725,10 +725,6 @@ void showTopicActionSheet(BuildContext context, {
         if (supportsFollowingTopics) {
           visibilityOptions.add(UserTopicVisibilityPolicy.none);
         }
-      case UserTopicVisibilityPolicy.unknown:
-        // TODO(#1074): This should be unreachable as we keep `unknown` out of
-        //   our data structures.
-        assert(false);
     }
   } else {
     // Channel is muted.
@@ -750,10 +746,6 @@ void showTopicActionSheet(BuildContext context, {
           if (supportsFollowingTopics) {
             visibilityOptions.add(UserTopicVisibilityPolicy.none);
           }
-        case UserTopicVisibilityPolicy.unknown:
-          // TODO(#1074): This should be unreachable as we keep `unknown` out of
-          //   our data structures.
-          assert(false);
       }
     }
   }
@@ -824,11 +816,6 @@ class UserTopicUpdateButton extends ActionSheetMenuItemButton {
         return ZulipIcons.unmute;
       case UserTopicVisibilityPolicy.followed:
         return ZulipIcons.follow;
-      case UserTopicVisibilityPolicy.unknown:
-        // TODO(#1074): This should be unreachable as we keep `unknown` out of
-        //   our data structures.
-        assert(false);
-        return ZulipIcons.inherit;
     }
   }
 
@@ -855,11 +842,6 @@ class UserTopicUpdateButton extends ActionSheetMenuItemButton {
         assert(false);
         return '';
 
-      case (_, UserTopicVisibilityPolicy.unknown):
-        // This case is unreachable (or should be) because we keep `unknown` out
-        // of our data structures. We plan to remove the `unknown` case in #1074.
-        assert(false);
-        return '';
     }
   }
 
@@ -885,11 +867,6 @@ class UserTopicUpdateButton extends ActionSheetMenuItemButton {
         assert(false);
         return '';
 
-      case (_, UserTopicVisibilityPolicy.unknown):
-        // This case is unreachable (or should be) because we keep `unknown` out
-        // of our data structures. We plan to remove the `unknown` case in #1074.
-        assert(false);
-        return '';
     }
   }
 

lib/model/channel.dart

@@ -226,10 +226,5 @@ IconData? iconDataForTopicVisibilityPolicy(UserTopicVisibilityPolicy policy) {
       return ZulipIcons.follow;
     case UserTopicVisibilityPolicy.none:
       return null;
-    case UserTopicVisibilityPolicy.unknown:
-      // This case is unreachable (or should be) because we keep `unknown` out
-      // of our data structures. We plan to remove the `unknown` case in #1074.
-      assert(false);
-      return null;
   }
 }

lib/widgets/action_sheet.dart

@@ -247,9 +247,6 @@ class _TopicItem extends StatelessWidget {
       case UserTopicVisibilityPolicy.unmuted:
       case UserTopicVisibilityPolicy.followed:
         opacity = 1;
-      case UserTopicVisibilityPolicy.unknown:
-        assert(false);
-        opacity = 1;
     }
 
     final visibilityIcon = iconDataForTopicVisibilityPolicy(visibilityPolicy);