model [nfc]: Use TopicName type across our model code

gnprice · chrisbobbe · commit 219972a9b35a · 2025-01-15T16:02:45.000-08:00
We do this all together in one commit, to avoid trying to
disentangle the graph of how topics get passed around among the
various parts of our model.
diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart
@@ -485,7 +485,7 @@ class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery,
     required Narrow narrow,
   }) {
     int? streamId;
-    String? topic;
+    TopicName? topic;
     switch (narrow) {
       case ChannelNarrow():
         streamId = narrow.streamId;
@@ -506,7 +506,7 @@ class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery,
 
   static int _compareByRelevance(User userA, User userB, {
     required int? streamId,
-    required String? topic,
+    required TopicName? topic,
     required PerAccountStore store,
   }) {
     // TODO(#234): give preference to "all", "everyone" or "stream"
@@ -543,7 +543,7 @@ class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery,
   @visibleForTesting
   static int compareByRecency(User userA, User userB, {
     required int streamId,
-    required String? topic,
+    required TopicName? topic,
     required PerAccountStore store,
   }) {
     final recentSenders = store.recentSenders;
@@ -815,7 +815,7 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
   /// The channel/stream the eventual message will be sent to.
   final int streamId;
 
-  Iterable<String> _topics = [];
+  Iterable<TopicName> _topics = [];
   bool _isFetching = false;
 
   /// Fetches topics of the current stream narrow, expected to fetch
@@ -843,7 +843,7 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
     return results;
   }
 
-  TopicAutocompleteResult? _testTopic(TopicAutocompleteQuery query, String topic) {
+  TopicAutocompleteResult? _testTopic(TopicAutocompleteQuery query, TopicName topic) {
     if (query.testTopic(topic)) {
       return TopicAutocompleteResult(topic: topic);
     }
@@ -862,7 +862,7 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
 class TopicAutocompleteQuery extends AutocompleteQuery {
   TopicAutocompleteQuery(super.raw);
 
-  bool testTopic(String topic) {
+  bool testTopic(TopicName topic) {
     // TODO(#881): Sort by match relevance, like web does.
     return topic != raw && topic.toLowerCase().contains(raw.toLowerCase());
   }
@@ -883,7 +883,7 @@ class TopicAutocompleteQuery extends AutocompleteQuery {
 
 /// A topic chosen in an autocomplete interaction, via a [TopicAutocompleteView].
 class TopicAutocompleteResult extends AutocompleteResult {
-  final String topic;
+  final TopicName topic;
 
   TopicAutocompleteResult({required this.topic});
 }
diff --git a/lib/model/channel.dart b/lib/model/channel.dart
@@ -41,7 +41,7 @@ mixin ChannelStore {
   ///
   /// For policies directly applicable in the UI, see
   /// [isTopicVisibleInStream] and [isTopicVisible].
-  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic);
+  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic);
 
   /// The raw data structure underlying [topicVisibilityPolicy].
   ///
@@ -51,7 +51,7 @@ mixin ChannelStore {
   /// and on the other hand being a concrete, finite data structure that
   /// can be compared using `deepEquals`.
   @visibleForTesting
-  Map<int, Map<String, UserTopicVisibilityPolicy>> get debugTopicVisibility;
+  Map<int, Map<TopicName, UserTopicVisibilityPolicy>> get debugTopicVisibility;
 
   /// Whether this topic should appear when already focusing on its stream.
   ///
@@ -63,7 +63,7 @@ mixin ChannelStore {
   ///
   /// For UI contexts that are not specific to a particular stream, see
   /// [isTopicVisible].
-  bool isTopicVisibleInStream(int streamId, String topic) {
+  bool isTopicVisibleInStream(int streamId, TopicName topic) {
     return _isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic));
   }
 
@@ -100,7 +100,7 @@ mixin ChannelStore {
   ///
   /// For UI contexts that are specific to a particular stream, see
   /// [isTopicVisibleInStream].
-  bool isTopicVisible(int streamId, String topic) {
+  bool isTopicVisible(int streamId, TopicName topic) {
     return _isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic));
   }
 
@@ -171,7 +171,7 @@ class ChannelStoreImpl with ChannelStore {
       streams.putIfAbsent(stream.streamId, () => stream);
     }
 
-    final topicVisibility = <int, Map<String, UserTopicVisibilityPolicy>>{};
+    final topicVisibility = <int, Map<TopicName, 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)
@@ -204,12 +204,12 @@ class ChannelStoreImpl with ChannelStore {
   final Map<int, Subscription> subscriptions;
 
   @override
-  Map<int, Map<String, UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
+  Map<int, Map<TopicName, UserTopicVisibilityPolicy>> get debugTopicVisibility => topicVisibility;
 
-  final Map<int, Map<String, UserTopicVisibilityPolicy>> topicVisibility;
+  final Map<int, Map<TopicName, UserTopicVisibilityPolicy>> topicVisibility;
 
   @override
-  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic) {
+  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic) {
     return topicVisibility[streamId]?[topic] ?? UserTopicVisibilityPolicy.none;
   }
 
diff --git a/lib/model/internal_link.dart b/lib/model/internal_link.dart
@@ -1,6 +1,7 @@
 import 'package:flutter/foundation.dart';
 import 'package:json_annotation/json_annotation.dart';
 
+import '../api/model/model.dart';
 import '../api/model/narrow.dart';
 import 'narrow.dart';
 import 'store.dart';
@@ -178,7 +179,7 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
         if (topicElement != null) return null;
         final String? topic = decodeHashComponent(operand);
         if (topic == null) return null;
-        topicElement = ApiNarrowTopic(topic, negated: negated);
+        topicElement = ApiNarrowTopic(TopicName(topic), negated: negated);
 
       case _NarrowOperator.dm:
       case _NarrowOperator.pmWith:
@@ -223,7 +224,7 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
   } else if (streamElement != null) {
     final streamId = streamElement.operand;
     if (topicElement != null) {
-      return TopicNarrow(streamId, topicElement.operand);
+      return TopicNarrow(streamId, TopicName.fromJson(topicElement.operand));
     } else {
       return ChannelNarrow(streamId);
     }
diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart
@@ -686,8 +686,8 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   void messagesMoved({
     required int origStreamId,
     required int newStreamId,
-    required String origTopic,
-    required String newTopic,
+    required TopicName origTopic,
+    required TopicName newTopic,
     required List<int> messageIds,
     required PropagateMode propagateMode,
   }) {
diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart
@@ -99,7 +99,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {
   }
 
   final int streamId;
-  final String topic;
+  final TopicName topic;
 
   @override
   bool containsMessage(Message message) {
diff --git a/lib/model/recent_senders.dart b/lib/model/recent_senders.dart
@@ -16,7 +16,7 @@ class RecentSenders {
 
   // topicSenders[streamId][topic][senderId] = MessageIdTracker
   @visibleForTesting
-  final Map<int, Map<String, Map<int, MessageIdTracker>>> topicSenders = {};
+  final Map<int, Map<TopicName, Map<int, MessageIdTracker>>> topicSenders = {};
 
   /// The latest message the given user sent to the given stream,
   /// or null if no such message is known.
@@ -29,7 +29,7 @@ class RecentSenders {
   /// or null if no such message is known.
   int? latestMessageIdOfSenderInTopic({
     required int streamId,
-    required String topic,
+    required TopicName topic,
     required int senderId,
   }) => topicSenders[streamId]?[topic]?[senderId]?.maxId;
 
@@ -38,7 +38,7 @@ class RecentSenders {
   /// The messages must be sorted by [Message.id] ascending.
   void handleMessages(List<Message> messages) {
     final messagesByUserInStream = <(int, int), QueueList<int>>{};
-    final messagesByUserInTopic = <(int, String, int), QueueList<int>>{};
+    final messagesByUserInTopic = <(int, TopicName, int), QueueList<int>>{};
     for (final message in messages) {
       if (message is! StreamMessage) continue;
       final StreamMessage(:streamId, :topic, :senderId, id: int messageId) = message;
diff --git a/lib/model/store.dart b/lib/model/store.dart
@@ -466,10 +466,10 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
   @override
   Map<int, Subscription> get subscriptions => _channels.subscriptions;
   @override
-  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, String topic) =>
+  UserTopicVisibilityPolicy topicVisibilityPolicy(int streamId, TopicName topic) =>
     _channels.topicVisibilityPolicy(streamId, topic);
   @override
-  Map<int, Map<String, UserTopicVisibilityPolicy>> get debugTopicVisibility =>
+  Map<int, Map<TopicName, UserTopicVisibilityPolicy>> get debugTopicVisibility =>
     _channels.debugTopicVisibility;
 
   final ChannelStoreImpl _channels;
diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart
@@ -40,7 +40,7 @@ class Unreads extends ChangeNotifier {
     required int selfUserId,
     required ChannelStore channelStore,
   }) {
-    final streams = <int, Map<String, QueueList<int>>>{};
+    final streams = <int, Map<TopicName, QueueList<int>>>{};
     final dms = <DmNarrow, QueueList<int>>{};
     final mentions = Set.of(initial.mentions);
 
@@ -86,7 +86,7 @@ class Unreads extends ChangeNotifier {
   // int count;
 
   /// Unread stream messages, as: stream ID → topic → message IDs (sorted).
-  final Map<int, Map<String, QueueList<int>>> streams;
+  final Map<int, Map<TopicName, QueueList<int>>> streams;
 
   /// Unread DM messages, as: DM narrow → message IDs (sorted).
   final Map<DmNarrow, QueueList<int>> dms;
@@ -185,7 +185,7 @@ class Unreads extends ChangeNotifier {
     return c;
   }
 
-  int countInTopicNarrow(int streamId, String topic) {
+  int countInTopicNarrow(int streamId, TopicName topic) {
     final topics = streams[streamId];
     return topics?[topic]?.length ?? 0;
   }
@@ -365,7 +365,7 @@ class Unreads extends ChangeNotifier {
               _slowRemoveAllInDms(messageIdsSet);
             }
           case UpdateMessageFlagsRemoveEvent():
-            final newlyUnreadInStreams = <int, Map<String, QueueList<int>>>{};
+            final newlyUnreadInStreams = <int, Map<TopicName, QueueList<int>>>{};
             final newlyUnreadInDms = <DmNarrow, QueueList<int>>{};
             for (final messageId in event.messages) {
               final detail = event.messageDetails![messageId];
@@ -449,12 +449,12 @@ class Unreads extends ChangeNotifier {
     );
   }
 
-  void _addLastInStreamTopic(int messageId, int streamId, String topic) {
+  void _addLastInStreamTopic(int messageId, int streamId, TopicName topic) {
     ((streams[streamId] ??= {})[topic] ??= QueueList()).addLast(messageId);
   }
 
   // [messageIds] must be sorted ascending and without duplicates.
-  void _addAllInStreamTopic(QueueList<int> messageIds, int streamId, String topic) {
+  void _addAllInStreamTopic(QueueList<int> messageIds, int streamId, TopicName topic) {
     final topics = streams[streamId] ??= {};
     topics.update(topic,
       ifAbsent: () => messageIds,
@@ -469,7 +469,7 @@ class Unreads extends ChangeNotifier {
   void _slowRemoveAllInStreams(Set<int> idsToRemove) {
     final newlyEmptyStreams = <int>[];
     for (final MapEntry(key: streamId, value: topics) in streams.entries) {
-      final newlyEmptyTopics = <String>[];
+      final newlyEmptyTopics = <TopicName>[];
       for (final MapEntry(key: topic, value: messageIds) in topics.entries) {
         messageIds.removeWhere((id) => idsToRemove.contains(id));
         if (messageIds.isEmpty) {
@@ -488,7 +488,7 @@ class Unreads extends ChangeNotifier {
     }
   }
 
-  void _removeAllInStreamTopic(Set<int> incomingMessageIds, int streamId, String topic) {
+  void _removeAllInStreamTopic(Set<int> incomingMessageIds, int streamId, TopicName topic) {
     final topics = streams[streamId];
     if (topics == null) return;
     final messageIds = topics[topic];
diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart
@@ -6,6 +6,7 @@ import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart' hide Notification;
 
+import '../api/model/model.dart';
 import '../api/notifications.dart';
 import '../generated/l10n/zulip_localizations.dart';
 import '../host/android_notifications.dart';
@@ -538,8 +539,8 @@ class NotificationOpenPayload {
         case 'topic':
           final channelIdStr = url.queryParameters['channel_id']!;
           final channelId = int.parse(channelIdStr, radix: 10);
-          final topic = url.queryParameters['topic']!;
-          narrow = TopicNarrow(channelId, topic);
+          final topicStr = url.queryParameters['topic']!;
+          narrow = TopicNarrow(channelId, TopicName(topicStr));
         case 'dm':
           final allRecipientIdsStr = url.queryParameters['all_recipient_ids']!;
           final allRecipientIds = allRecipientIdsStr.split(',')
diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart
@@ -151,7 +151,7 @@ class ActionSheetCancelButton extends StatelessWidget {
 /// Show a sheet of actions you can take on a topic.
 void showTopicActionSheet(BuildContext context, {
   required int channelId,
-  required String topic,
+  required TopicName topic,
 }) {
   final store = PerAccountStoreWidget.of(context);
   final subscription = store.subscriptions[channelId];
diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart
@@ -90,7 +90,7 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
     ];
   }
 
-  void setTopic(String newTopic) {
+  void setTopic(TopicName newTopic) {
     value = TextEditingValue(text: newTopic);
   }
 }
@@ -490,7 +490,7 @@ class _StreamContentInputState extends State<_StreamContentInput> {
       ?? zulipLocalizations.composeBoxUnknownChannelName;
     return _ContentInput(
       narrow: widget.narrow,
-      destination: TopicNarrow(widget.narrow.streamId, _topicTextNormalized),
+      destination: TopicNarrow(widget.narrow.streamId, TopicName(_topicTextNormalized)),
       controller: widget.controller,
       hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
   }
@@ -1155,7 +1155,7 @@ class _StreamComposeBoxBody extends _ComposeBoxBody {
   @override Widget buildSendButton() => _SendButton(
     controller: controller,
     getDestination: () => StreamDestination(
-      narrow.streamId, controller.topic.textNormalized),
+      narrow.streamId, TopicName(controller.topic.textNormalized)),
   );
 }
 
diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart
@@ -132,7 +132,7 @@ class _InboxPageState extends State<InboxPageBody> with PerAccountStoreAwareStat
       });
 
     for (final MapEntry(key: streamId, value: topics) in sortedUnreadStreams) {
-      final topicItems = <(String, int, bool, int)>[];
+      final topicItems = <(TopicName, int, bool, int)>[];
       int countInStream = 0;
       bool streamHasMention = false;
       for (final MapEntry(key: topic, value: messageIds) in topics.entries) {
@@ -192,7 +192,7 @@ class _StreamSectionData extends _InboxSectionData {
   final int streamId;
   final int count;
   final bool hasMention;
-  final List<(String, int, bool, int)> items;
+  final List<(TopicName, int, bool, int)> items;
 
   const _StreamSectionData(this.streamId, this.count, this.hasMention, this.items);
 }
@@ -487,7 +487,7 @@ class _TopicItem extends StatelessWidget {
   });
 
   final int streamId;
-  final String topic;
+  final TopicName topic;
   final int count;
   final bool hasMention;
 
diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -334,7 +334,7 @@ class MessageListAppBarTitle extends StatelessWidget {
 
   Widget _buildTopicRow(BuildContext context, {
     required ZulipStream? stream,
-    required String topic,
+    required TopicName topic,
   }) {
     final store = PerAccountStoreWidget.of(context);
     final designVariables = DesignVariables.of(context);
diff --git a/test/example_data.dart b/test/example_data.dart
@@ -777,7 +777,7 @@ TypingEvent typingEvent(SendableNarrow narrow, TypingOp op, int senderId) {
       return TypingEvent(id: 0, op: op, senderId: senderId,
         messageType: MessageType.stream,
         streamId: narrow.streamId,
-        topic: TopicName(narrow.topic),
+        topic: narrow.topic,
         recipientIds: null);
     case DmNarrow():
       return TypingEvent(id: 0, op: op, senderId: senderId,
diff --git a/test/model/autocomplete_checks.dart b/test/model/autocomplete_checks.dart
@@ -1,4 +1,5 @@
 import 'package:checks/checks.dart';
+import 'package:zulip/api/model/model.dart';
 import 'package:zulip/model/autocomplete.dart';
 import 'package:zulip/widgets/compose_box.dart';
 
@@ -20,5 +21,5 @@ extension UserMentionAutocompleteResultChecks on Subject<UserMentionAutocomplete
 }
 
 extension TopicAutocompleteResultChecks on Subject<TopicAutocompleteResult> {
-  Subject<String> get topic => has((r) => r.topic, 'topic');
+  Subject<TopicName> get topic => has((r) => r.topic, 'topic');
 }
diff --git a/test/model/narrow_checks.dart b/test/model/narrow_checks.dart
diff --git a/test/model/recent_senders_test.dart b/test/model/recent_senders_test.dart
diff --git a/test/model/unreads_checks.dart b/test/model/unreads_checks.dart
diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart

Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`final int streamId;`
`102`		`- final String topic;`
	`102`	`+ final TopicName topic;`
`103`	`103`
`104`	`104`	`@override`
`105`	`105`	`bool containsMessage(Message message) {`