Skip to content

Commit

Permalink
api [nfc]: Introduce TopicName extension type
Browse files Browse the repository at this point in the history
We'll use this to make a type-level distinction between a string
that's being used to name a topic, and just any old string.
That will help us in particular with zulip#1250 the "general chat"
feature, where the way we show a topic name to the user stops being
necessarily just the same string that it appears as in the API.

The next phase will be to migrate a bunch of places in our code
to refer to TopicName when that's what they mean, instead of just
String.

During this phase, a TopicName can be freely used as a String, but
not vice versa.  So we'll do the migrations mostly in order of the
data flow, from upstream to downstream.  That will allow us to do
them over a series of individually coherent commits, with a minimum
of occasions where we temporarily introduce a conversion that won't
be needed in the final result.

That means: first, test data; then topics returned from the API to
our code; then our internal models; then back to the API, this time
for topics passed to the API from our code.

After we have the type in place all over where it belongs, we'll
start making use of the distinction, and then enforcing it.
  • Loading branch information
gnprice authored and Gaurav-Kushwaha-1225 committed Jan 21, 2025
1 parent 0025ef3 commit 4162232
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 5 deletions.
10 changes: 9 additions & 1 deletion lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,14 @@ enum MessageFlag {
String toJson() => _$MessageFlagEnumMap[this]!;
}

/// The name of a Zulip topic.
// TODO(#1250): Migrate all implicit uses as String; remove "implements String".
extension type const TopicName(String _value) implements String {
TopicName.fromJson(this._value);

String toJson() => _value;
}

@JsonSerializable(fieldRename: FieldRename.snake)
class StreamMessage extends Message {
@override
Expand All @@ -674,7 +682,7 @@ class StreamMessage extends Message {
// that will need new UI that we'll design then as part of that feature,
// and ignoring the topics seems as good a fallback behavior as any.
@JsonKey(name: 'subject')
String topic;
TopicName topic;

StreamMessage({
required super.client,
Expand Down
2 changes: 1 addition & 1 deletion lib/api/model/model.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/model/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class MessageStoreImpl with MessageStore {
}

if (newTopic != null) {
message.topic = newTopic;
message.topic = TopicName(newTopic);
}

if (!wasResolveOrUnresolve
Expand Down
2 changes: 1 addition & 1 deletion test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ extension MessageChecks on Subject<Message> {

extension StreamMessageChecks on Subject<StreamMessage> {
Subject<String?> get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient');
Subject<String> get topic => has((e) => e.topic, 'topic');
Subject<TopicName> get topic => has((e) => e.topic, 'topic');
}

extension ReactionsChecks on Subject<Reactions> {
Expand Down
2 changes: 1 addition & 1 deletion test/api/model/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void main() {
check(Message.fromJson(baseStreamJson()
..['subject'] = 'hello'
)).isA<StreamMessage>()
.topic.equals('hello');
.topic.equals(const TopicName('hello'));
});

test('match_subject -> matchTopic', () {
Expand Down

0 comments on commit 4162232

Please sign in to comment.