model: Treat topics case-insensitively in topic-visibility

Worth noting some helpful feedback from Greg in review, about the
additions to test/model/channel_test.dart:

zulip#1608 (comment)
> I think my ideal version of these tests would have most of the new
> checks in this file be separate test cases, rather than added to
> existing tests. This also doesn't need a full matrix with the
> other variables: for example, this check can be done with any one
> of the non-`none` policies, rather than all three.
>
> These are fine, though. No need to rewrite them — we have
> higher-priority things to do.

Author: chrisbobbe

lib/model/channel.dart

@@ -43,6 +43,8 @@ mixin ChannelStore {
   ///
   /// For policies directly applicable in the UI, see
   /// [isTopicVisibleInStream] and [isTopicVisible].
+  ///
+  /// Topics are treated case-insensitively; see [TopicName.isSameAs].
   UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic);
 
   /// The raw data structure underlying [topicVisibilityPolicy].
@@ -173,13 +175,13 @@ class ChannelStoreImpl with ChannelStore {
       streams.putIfAbsent(stream.streamId, () => stream);
     }
 
-    final topicVisibility = <int, Map<TopicName, UserTopicVisibilityPolicy>>{};
+    final topicVisibility = <int, TopicKeyedMap<UserTopicVisibilityPolicy>>{};
     for (final item in initialSnapshot.userTopics ?? const <UserTopicItem>[]) {
       if (_warnInvalidVisibilityPolicy(item.visibilityPolicy)) {
         // Not a value we expect. Keep it out of our data structures. // TODO(log)
         continue;
       }
-      final forStream = topicVisibility.putIfAbsent(item.streamId, () => {});
+      final forStream = topicVisibility.putIfAbsent(item.streamId, () => makeTopicKeyedMap());
       forStream[item.topicName] = item.visibilityPolicy;
     }
 
@@ -206,9 +208,9 @@ class ChannelStoreImpl with ChannelStore {
   final Map<int, Subscription> subscriptions;
 
   @override
-  Map<int, Map<TopicName, UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
+  Map<int, TopicKeyedMap<UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
 
-  final Map<int, Map<TopicName, UserTopicVisibilityPolicy>> topicVisibility;
+  final Map<int, TopicKeyedMap<UserTopicVisibilityPolicy>> topicVisibility;
 
   @override
   UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic) {
@@ -366,7 +368,7 @@ class ChannelStoreImpl with ChannelStore {
         topicVisibility.remove(event.streamId);
       }
     } else {
-      final forStream = topicVisibility.putIfAbsent(event.streamId, () => {});
+      final forStream = topicVisibility.putIfAbsent(event.streamId, () => makeTopicKeyedMap());
       forStream[event.topicName] = visibilityPolicy;
     }
   }

test/model/channel_test.dart

@@ -1,4 +1,3 @@
-
 import 'package:checks/checks.dart';
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/model/events.dart';
@@ -163,6 +162,10 @@ void main() {
           await store.setUserTopic(stream1, 'topic', policy);
           check(store.topicVisibilityPolicy(stream1.streamId, eg.t('topic')))
             .equals(policy);
+
+          // Case-insensitive
+          check(store.topicVisibilityPolicy(stream1.streamId, eg.t('ToPiC')))
+            .equals(policy);
         }
       });
     });
@@ -198,6 +201,10 @@ void main() {
         await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
         check(store.isTopicVisibleInStream(stream1.streamId, eg.t('topic'))).isFalse();
         check(store.isTopicVisible        (stream1.streamId, eg.t('topic'))).isFalse();
+
+        // Case-insensitive
+        check(store.isTopicVisibleInStream(stream1.streamId, eg.t('ToPiC'))).isFalse();
+        check(store.isTopicVisible        (stream1.streamId, eg.t('ToPiC'))).isFalse();
       });
 
       test('with policy unmuted', () async {
@@ -207,6 +214,10 @@ void main() {
         await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unmuted);
         check(store.isTopicVisibleInStream(stream1.streamId, eg.t('topic'))).isTrue();
         check(store.isTopicVisible        (stream1.streamId, eg.t('topic'))).isTrue();
+
+        // Case-insensitive
+        check(store.isTopicVisibleInStream(stream1.streamId, eg.t('tOpIc'))).isTrue();
+        check(store.isTopicVisible        (stream1.streamId, eg.t('tOpIc'))).isTrue();
       });
 
       test('with policy followed', () async {
@@ -216,20 +227,40 @@ void main() {
         await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.followed);
         check(store.isTopicVisibleInStream(stream1.streamId, eg.t('topic'))).isTrue();
         check(store.isTopicVisible        (stream1.streamId, eg.t('topic'))).isTrue();
+
+        // Case-insensitive
+        check(store.isTopicVisibleInStream(stream1.streamId, eg.t('TOPIC'))).isTrue();
+        check(store.isTopicVisible        (stream1.streamId, eg.t('TOPIC'))).isTrue();
       });
     });
 
     group('willChangeIfTopicVisible/InStream', () {
       UserTopicEvent mkEvent(UserTopicVisibilityPolicy policy) =>
         eg.userTopicEvent(stream1.streamId, 'topic', policy);
 
+      // For testing case-insensitivity
+      UserTopicEvent mkEventDifferentlyCased(UserTopicVisibilityPolicy policy) =>
+        eg.userTopicEvent(stream1.streamId, 'ToPiC', policy);
+
+      assert(() {
+        // (sanity check on mkEvent and mkEventDifferentlyCased)
+        final event1 = mkEvent(UserTopicVisibilityPolicy.followed);
+        final event2 = mkEventDifferentlyCased(UserTopicVisibilityPolicy.followed);
+        return event1.topicName.isSameAs(event2.topicName)
+          && event1.topicName.apiName != event2.topicName.apiName;
+      }());
+
       void checkChanges(PerAccountStore store,
           UserTopicVisibilityPolicy newPolicy,
           UserTopicVisibilityEffect expectedInStream,
           UserTopicVisibilityEffect expectedOverall) {
         final event = mkEvent(newPolicy);
         check(store.willChangeIfTopicVisibleInStream(event)).equals(expectedInStream);
         check(store.willChangeIfTopicVisible        (event)).equals(expectedOverall);
+
+        final event2 = mkEventDifferentlyCased(newPolicy);
+        check(store.willChangeIfTopicVisibleInStream(event2)).equals(expectedInStream);
+        check(store.willChangeIfTopicVisible        (event2)).equals(expectedOverall);
       }
 
       test('stream not muted, policy none -> followed, no change', () async {
@@ -366,6 +397,12 @@ void main() {
         compareTopicVisibility(store, [
           eg.userTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.unmuted),
         ]);
+
+        // case-insensitivity
+        await store.setUserTopic(stream1, 'ToPiC', UserTopicVisibilityPolicy.followed);
+        compareTopicVisibility(store, [
+          eg.userTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.followed),
+        ]);
       });
 
       test('remove, with others in stream', () async {
@@ -381,15 +418,17 @@ void main() {
       test('remove, as last in stream', () async {
         final store = eg.store();
         await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
-        await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.none);
+        // case-insensitivity
+        await store.setUserTopic(stream1, 'ToPiC', UserTopicVisibilityPolicy.none);
         compareTopicVisibility(store, [
         ]);
       });
 
       test('treat unknown enum value as removing', () async {
         final store = eg.store();
         await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
-        await store.setUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unknown);
+        // case-insensitivity
+        await store.setUserTopic(stream1, 'ToPiC', UserTopicVisibilityPolicy.unknown);
         compareTopicVisibility(store, [
         ]);
       });
@@ -406,7 +445,8 @@ void main() {
         ]));
       check(store.topicVisibilityPolicy(stream.streamId, eg.t('topic 1')))
         .equals(UserTopicVisibilityPolicy.muted);
-      check(store.topicVisibilityPolicy(stream.streamId, eg.t('topic 2')))
+      // case-insensitivity
+      check(store.topicVisibilityPolicy(stream.streamId, eg.t('ToPiC 2')))
         .equals(UserTopicVisibilityPolicy.unmuted);
       check(store.topicVisibilityPolicy(stream.streamId, eg.t('topic 3')))
         .equals(UserTopicVisibilityPolicy.followed);

test/widgets/inbox_test.dart

@@ -9,6 +9,7 @@ import 'package:zulip/widgets/color.dart';
 import 'package:zulip/widgets/home.dart';
 import 'package:zulip/widgets/icons.dart';
 import 'package:zulip/widgets/channel_colors.dart';
+import 'package:zulip/widgets/unread_count_badge.dart';
 
 import '../example_data.dart' as eg;
 import '../flutter_checks.dart';
@@ -58,6 +59,7 @@ void main() {
     List<Subscription>? subscriptions,
     List<User>? users,
     required List<Message> unreadMessages,
+    List<Message>? otherMessages,
     NavigatorObserver? navigatorObserver,
   }) async {
     addTearDown(testBinding.reset);
@@ -362,6 +364,39 @@ void main() {
           parent: findRowByLabel(tester, topic),
           icon: ZulipIcons.unmute)).isTrue();
       });
+
+      testWidgets('unmuted (topics treated case-insensitively)', (tester) async {
+        // Case-insensitivity of both topic-visibility and unreads data
+        // TODO(#1065) this belongs in test/model/ once the inbox page has
+        //   its own view-model
+
+        final message1 = eg.streamMessage(stream: channel, topic: 'aaa');
+        final message2 = eg.streamMessage(stream: channel, topic: 'AaA', flags: [MessageFlag.read]);
+        final message3 = eg.streamMessage(stream: channel, topic: 'aAa', flags: [MessageFlag.read]);
+        await setupPage(tester,
+          users: [eg.selfUser, eg.otherUser],
+          streams: [channel],
+          subscriptions: [eg.subscription(channel, isMuted: true)],
+          unreadMessages: [message1]);
+        await store.setUserTopic(channel, 'aaa', UserTopicVisibilityPolicy.unmuted);
+        await tester.pump();
+
+        check(find.descendant(
+          of: find.byWidget(findRowByLabel(tester, 'aaa')!),
+          matching: find.widgetWithText(UnreadCountBadge, '1'))).findsOne();
+
+        await store.handleEvent(eg.updateMessageFlagsRemoveEvent(MessageFlag.read, [message2]));
+        await tester.pump();
+        check(find.descendant(
+          of: find.byWidget(findRowByLabel(tester, 'aaa')!),
+          matching: find.widgetWithText(UnreadCountBadge, '2'))).findsOne();
+
+        await store.handleEvent(eg.updateMessageFlagsRemoveEvent(MessageFlag.read, [message3]));
+        await tester.pump();
+        check(find.descendant(
+          of: find.byWidget(findRowByLabel(tester, 'aaa')!),
+          matching: find.widgetWithText(UnreadCountBadge, '3'))).findsOne();
+      });
     });
 
     group('collapsing', () {