Skip to content

Commit 40978dc

Browse files
committed
api [nfc]: Pull out Conversation
This will help support outbox messages in MessageListView later. We extracted Conversation instead of using MessageDestination because they are foundamentally different. Conversation is the identifier for the conversation that contains the message from, for example, get-messages or message events, but MessageDestination is specifically for send-message.
1 parent 87588b0 commit 40978dc

File tree

5 files changed

+138
-84
lines changed

5 files changed

+138
-84
lines changed

lib/api/model/model.dart

Lines changed: 118 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'package:json_annotation/json_annotation.dart';
22

3+
import '../../model/algorithms.dart';
34
import 'events.dart';
45
import 'initial_snapshot.dart';
56
import 'reaction.dart';
@@ -531,6 +532,99 @@ String? tryParseEmojiCodeToUnicode(String emojiCode) {
531532
}
532533
}
533534

535+
/// The name of a Zulip topic.
536+
// TODO(dart): Can we forbid calling Object members on this extension type?
537+
// (The lack of "implements Object" ought to do that, but doesn't.)
538+
// In particular an interpolation "foo > $topic" is a bug we'd like to catch.
539+
// TODO(dart): Can we forbid using this extension type as a key in a Map?
540+
// (The lack of "implements Object" arguably should do that, but doesn't.)
541+
// Using as a Map key is almost certainly a bug because it won't case-fold;
542+
// see for example #739, #980, #1205.
543+
extension type const TopicName(String _value) {
544+
/// The canonical form of the resolved-topic prefix.
545+
// This is RESOLVED_TOPIC_PREFIX in web:
546+
// https://github.com/zulip/zulip/blob/1fac99733/web/shared/src/resolved_topic.ts
547+
static const resolvedTopicPrefix = '✔ ';
548+
549+
/// Pattern for an arbitrary resolved-topic prefix.
550+
///
551+
/// These always begin with [resolvedTopicPrefix]
552+
/// but can be weird and go on longer, like "✔ ✔✔ ".
553+
// This is RESOLVED_TOPIC_PREFIX_RE in web:
554+
// https://github.com/zulip/zulip/blob/1fac99733/web/shared/src/resolved_topic.ts#L4-L12
555+
static final resolvedTopicPrefixRegexp = RegExp(r'^✔ [ ✔]*');
556+
557+
/// The string this topic is identified by in the Zulip API.
558+
///
559+
/// This should be used in constructing HTTP requests to the server,
560+
/// but rarely for other purposes. See [displayName] and [canonicalize].
561+
String get apiName => _value;
562+
563+
/// The string this topic is displayed as to the user in our UI.
564+
///
565+
/// At the moment this always equals [apiName].
566+
/// In the future this will become null for the "general chat" topic (#1250),
567+
/// so that UI code can identify when it needs to represent the topic
568+
/// specially in the way prescribed for "general chat".
569+
// TODO(#1250) carry out that plan
570+
String get displayName => _value;
571+
572+
/// The key to use for "same topic as" comparisons.
573+
String canonicalize() => apiName.toLowerCase();
574+
575+
/// Whether the topic starts with [resolvedTopicPrefix].
576+
bool get isResolved => _value.startsWith(resolvedTopicPrefix);
577+
578+
/// This [TopicName] plus the [resolvedTopicPrefix] prefix.
579+
TopicName resolve() => TopicName(resolvedTopicPrefix + _value);
580+
581+
/// A [TopicName] with [resolvedTopicPrefixRegexp] stripped if present.
582+
TopicName unresolve() =>
583+
TopicName(_value.replaceFirst(resolvedTopicPrefixRegexp, ''));
584+
585+
/// Whether [this] and [other] have the same canonical form,
586+
/// using [canonicalize].
587+
bool isSameAs(TopicName other) => canonicalize() == other.canonicalize();
588+
589+
TopicName.fromJson(this._value);
590+
591+
String toJson() => apiName;
592+
}
593+
594+
/// As in [StreamMessage.conversation] and [DmMessage.conversation].
595+
///
596+
/// Different from [MessageDestination], this information comes from
597+
/// [getMessages] or [getEvents], identifying the conversation that contains a
598+
/// message.
599+
sealed class Conversation {}
600+
601+
/// The conversation a stream message is in.
602+
@JsonSerializable(fieldRename: FieldRename.snake, createToJson: false)
603+
class StreamConversation extends Conversation {
604+
StreamConversation(this.streamId, this.topic);
605+
606+
int streamId;
607+
608+
@JsonKey(name: 'subject')
609+
TopicName topic;
610+
611+
factory StreamConversation.fromJson(Map<String, dynamic> json) =>
612+
_$StreamConversationFromJson(json);
613+
}
614+
615+
/// The conversation a DM message is in.
616+
class DmConversation extends Conversation {
617+
DmConversation({required this.allRecipientIds})
618+
: assert(isSortedWithoutDuplicates(allRecipientIds.toList()));
619+
620+
/// The user IDs of all users in the conversation, sorted numerically.
621+
///
622+
/// This lists the sender as well as all (other) participants, and it
623+
/// lists each user just once. In particular the self-user is always
624+
/// included.
625+
final List<int> allRecipientIds;
626+
}
627+
534628
/// As in the get-messages response.
535629
///
536630
/// https://zulip.com/api/get-messages#response
@@ -655,65 +749,6 @@ enum MessageFlag {
655749
String toJson() => _$MessageFlagEnumMap[this]!;
656750
}
657751

658-
/// The name of a Zulip topic.
659-
// TODO(dart): Can we forbid calling Object members on this extension type?
660-
// (The lack of "implements Object" ought to do that, but doesn't.)
661-
// In particular an interpolation "foo > $topic" is a bug we'd like to catch.
662-
// TODO(dart): Can we forbid using this extension type as a key in a Map?
663-
// (The lack of "implements Object" arguably should do that, but doesn't.)
664-
// Using as a Map key is almost certainly a bug because it won't case-fold;
665-
// see for example #739, #980, #1205.
666-
extension type const TopicName(String _value) {
667-
/// The canonical form of the resolved-topic prefix.
668-
// This is RESOLVED_TOPIC_PREFIX in web:
669-
// https://github.com/zulip/zulip/blob/1fac99733/web/shared/src/resolved_topic.ts
670-
static const resolvedTopicPrefix = '✔ ';
671-
672-
/// Pattern for an arbitrary resolved-topic prefix.
673-
///
674-
/// These always begin with [resolvedTopicPrefix]
675-
/// but can be weird and go on longer, like "✔ ✔✔ ".
676-
// This is RESOLVED_TOPIC_PREFIX_RE in web:
677-
// https://github.com/zulip/zulip/blob/1fac99733/web/shared/src/resolved_topic.ts#L4-L12
678-
static final resolvedTopicPrefixRegexp = RegExp(r'^✔ [ ✔]*');
679-
680-
/// The string this topic is identified by in the Zulip API.
681-
///
682-
/// This should be used in constructing HTTP requests to the server,
683-
/// but rarely for other purposes. See [displayName] and [canonicalize].
684-
String get apiName => _value;
685-
686-
/// The string this topic is displayed as to the user in our UI.
687-
///
688-
/// At the moment this always equals [apiName].
689-
/// In the future this will become null for the "general chat" topic (#1250),
690-
/// so that UI code can identify when it needs to represent the topic
691-
/// specially in the way prescribed for "general chat".
692-
// TODO(#1250) carry out that plan
693-
String get displayName => _value;
694-
695-
/// The key to use for "same topic as" comparisons.
696-
String canonicalize() => apiName.toLowerCase();
697-
698-
/// Whether the topic starts with [resolvedTopicPrefix].
699-
bool get isResolved => _value.startsWith(resolvedTopicPrefix);
700-
701-
/// This [TopicName] plus the [resolvedTopicPrefix] prefix.
702-
TopicName resolve() => TopicName(resolvedTopicPrefix + _value);
703-
704-
/// A [TopicName] with [resolvedTopicPrefixRegexp] stripped if present.
705-
TopicName unresolve() =>
706-
TopicName(_value.replaceFirst(resolvedTopicPrefixRegexp, ''));
707-
708-
/// Whether [this] and [other] have the same canonical form,
709-
/// using [canonicalize].
710-
bool isSameAs(TopicName other) => canonicalize() == other.canonicalize();
711-
712-
TopicName.fromJson(this._value);
713-
714-
String toJson() => apiName;
715-
}
716-
717752
@JsonSerializable(fieldRename: FieldRename.snake)
718753
class StreamMessage extends Message {
719754
@override
@@ -726,14 +761,22 @@ class StreamMessage extends Message {
726761
@JsonKey(required: true, disallowNullValue: true)
727762
String? displayRecipient;
728763

729-
int streamId;
764+
@JsonKey(includeToJson: true)
765+
int get streamId => conversation.streamId;
730766

731767
// The topic/subject is documented to be present on DMs too, just empty.
732768
// We ignore it on DMs; if a future server introduces distinct topics in DMs,
733769
// that will need new UI that we'll design then as part of that feature,
734770
// and ignoring the topics seems as good a fallback behavior as any.
735-
@JsonKey(name: 'subject')
736-
TopicName topic;
771+
@JsonKey(name: 'subject', includeToJson: true)
772+
TopicName get topic => conversation.topic;
773+
774+
@JsonKey(readValue: _readConversation, includeToJson: false)
775+
StreamConversation conversation;
776+
777+
static Map<String, dynamic> _readConversation(Map<dynamic, dynamic> json, String key) {
778+
return json as Map<String, dynamic>;
779+
}
737780

738781
StreamMessage({
739782
required super.client,
@@ -754,8 +797,7 @@ class StreamMessage extends Message {
754797
required super.matchContent,
755798
required super.matchTopic,
756799
required this.displayRecipient,
757-
required this.streamId,
758-
required this.topic,
800+
required this.conversation,
759801
});
760802

761803
factory StreamMessage.fromJson(Map<String, dynamic> json) =>
@@ -781,20 +823,23 @@ class DmMessage extends Message {
781823
/// included.
782824
// TODO(server): Document that it's all users. That statement is based on
783825
// reverse-engineering notes in zulip-mobile:src/api/modelTypes.js at PmMessage.
784-
@JsonKey(name: 'display_recipient', fromJson: _allRecipientIdsFromJson, toJson: _allRecipientIdsToJson)
785-
final List<int> allRecipientIds;
826+
@JsonKey(name: 'display_recipient', toJson: _allRecipientIdsToJson, includeToJson: true)
827+
List<int> get allRecipientIds => conversation.allRecipientIds;
786828

787-
static List<int> _allRecipientIdsFromJson(Object? json) {
788-
return (json as List<dynamic>).map(
789-
(element) => ((element as Map<String, dynamic>)['id'] as num).toInt()
790-
).toList(growable: false)
791-
..sort();
792-
}
829+
@JsonKey(name: 'display_recipient', fromJson: _conversationFromJson, includeToJson: false)
830+
final DmConversation conversation;
793831

794832
static List<Map<String, dynamic>> _allRecipientIdsToJson(List<int> allRecipientIds) {
795833
return allRecipientIds.map((element) => {'id': element}).toList();
796834
}
797835

836+
static DmConversation _conversationFromJson(List<dynamic> json) {
837+
return DmConversation(allRecipientIds: json.map(
838+
(element) => ((element as Map<String, dynamic>)['id'] as num).toInt()
839+
).toList(growable: false)
840+
..sort());
841+
}
842+
798843
DmMessage({
799844
required super.client,
800845
required super.content,
@@ -813,7 +858,7 @@ class DmMessage extends Message {
813858
required super.flags,
814859
required super.matchContent,
815860
required super.matchTopic,
816-
required this.allRecipientIds,
861+
required this.conversation,
817862
});
818863

819864
factory DmMessage.fromJson(Map<String, dynamic> json) =>

lib/api/model/model.g.dart

Lines changed: 12 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/model/message.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,14 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
211211
}
212212

213213
if (newStreamId != origStreamId) {
214-
message.streamId = newStreamId;
214+
message.conversation.streamId = newStreamId;
215215
// See [StreamMessage.displayRecipient] on why the invalidation is
216216
// needed.
217217
message.displayRecipient = null;
218218
}
219219

220220
if (newTopic != origTopic) {
221-
message.topic = newTopic;
221+
message.conversation.topic = newTopic;
222222
}
223223

224224
if (!wasResolveOrUnresolve

lib/model/narrow.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ class DmNarrow extends Narrow implements SendableNarrow {
235235
/// See also:
236236
/// * [otherRecipientIds], an alternate way of identifying the conversation.
237237
/// * [DmMessage.allRecipientIds], which provides this same format.
238+
/// * [DmConversation.allRecipientIds], which also provides this same format.
238239
final List<int> allRecipientIds;
239240

240241
/// The user ID of the self-user.

test/api/model/model_checks.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ extension UserChecks on Subject<User> {
2424
extension ZulipStreamChecks on Subject<ZulipStream> {
2525
}
2626

27+
extension TopicNameChecks on Subject<TopicName> {
28+
Subject<String> get apiName => has((x) => x.apiName, 'apiName');
29+
Subject<String> get displayName => has((x) => x.displayName, 'displayName');
30+
}
31+
2732
extension MessageChecks on Subject<Message> {
2833
Subject<String> get client => has((e) => e.client, 'client');
2934
Subject<String> get content => has((e) => e.content, 'content');
@@ -46,11 +51,6 @@ extension MessageChecks on Subject<Message> {
4651
Subject<String?> get matchTopic => has((e) => e.matchTopic, 'matchTopic');
4752
}
4853

49-
extension TopicNameChecks on Subject<TopicName> {
50-
Subject<String> get apiName => has((x) => x.apiName, 'apiName');
51-
Subject<String> get displayName => has((x) => x.displayName, 'displayName');
52-
}
53-
5454
extension StreamMessageChecks on Subject<StreamMessage> {
5555
Subject<String?> get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient');
5656
Subject<int> get streamId => has((e) => e.streamId, 'streamId');

0 commit comments

Comments
 (0)