Skip to content

Commit ad55496

Browse files
gnpricechrisbobbe
authored andcommitted
api: Explicitly ignore topics on DMs
The topic/subject field on DMs carries no information in the current API, and we already never actually look at it, as demonstrated by the lack of changes in the rest of the tree in this commit. Clean up the type definitions in the API a bit by making that explicit. This is NFC except in the case where a server were to not send this field at all for a DM (or to send a non-string value). In that case the old code would reject the server's response as malformed, and the new code after this commit would accept it because it doesn't look for the field.
1 parent 531af69 commit ad55496

File tree

5 files changed

+17
-11
lines changed

5 files changed

+17
-11
lines changed

lib/api/model/model.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -556,11 +556,11 @@ sealed class Message {
556556
final String senderFullName;
557557
final int senderId;
558558
final String senderRealmStr;
559-
@JsonKey(name: 'subject')
560-
String topic;
559+
561560
/// Poll data if "submessages" describe a poll, `null` otherwise.
562561
@JsonKey(name: 'submessages', readValue: _readPoll, fromJson: Poll.fromJson, toJson: Poll.toJson)
563562
Poll? poll;
563+
564564
final int timestamp;
565565
String get type;
566566

@@ -613,7 +613,6 @@ sealed class Message {
613613
required this.senderFullName,
614614
required this.senderId,
615615
required this.senderRealmStr,
616-
required this.topic,
617616
required this.timestamp,
618617
required this.flags,
619618
required this.matchContent,
@@ -667,8 +666,16 @@ class StreamMessage extends Message {
667666
// invalidated.
668667
@JsonKey(required: true, disallowNullValue: true)
669668
String? displayRecipient;
669+
670670
int streamId;
671671

672+
// The topic/subject is documented to be present on DMs too, just empty.
673+
// We ignore it on DMs; if a future server introduces distinct topics in DMs,
674+
// that will need new UI that we'll design then as part of that feature,
675+
// and ignoring the topics seems as good a fallback behavior as any.
676+
@JsonKey(name: 'subject')
677+
String topic;
678+
672679
StreamMessage({
673680
required super.client,
674681
required super.content,
@@ -683,13 +690,13 @@ class StreamMessage extends Message {
683690
required super.senderFullName,
684691
required super.senderId,
685692
required super.senderRealmStr,
686-
required super.topic,
687693
required super.timestamp,
688694
required super.flags,
689695
required super.matchContent,
690696
required super.matchTopic,
691697
required this.displayRecipient,
692698
required this.streamId,
699+
required this.topic,
693700
});
694701

695702
factory StreamMessage.fromJson(Map<String, dynamic> json) =>
@@ -786,7 +793,6 @@ class DmMessage extends Message {
786793
required super.senderFullName,
787794
required super.senderId,
788795
required super.senderRealmStr,
789-
required super.topic,
790796
required super.timestamp,
791797
required super.flags,
792798
required super.matchContent,

lib/api/model/model.g.dart

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

lib/widgets/action_sheet.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
633633

634634
@override void onPressed() async {
635635
final zulipLocalizations = ZulipLocalizations.of(pageContext);
636+
final message = this.message;
636637

637638
var composeBoxController = findMessageListPage().composeBoxController;
638639
// The compose box doesn't null out its controller; it's either always null

test/api/model/model_checks.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ extension MessageChecks on Subject<Message> {
3838
Subject<String> get senderFullName => has((e) => e.senderFullName, 'senderFullName');
3939
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
4040
Subject<String> get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr');
41-
Subject<String> get topic => has((e) => e.topic, 'topic');
4241
Subject<Poll?> get poll => has((e) => e.poll, 'poll');
4342
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
4443
Subject<String> get type => has((e) => e.type, 'type');
@@ -49,6 +48,7 @@ extension MessageChecks on Subject<Message> {
4948

5049
extension StreamMessageChecks on Subject<StreamMessage> {
5150
Subject<String?> get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient');
51+
Subject<String> get topic => has((e) => e.topic, 'topic');
5252
}
5353

5454
extension ReactionsChecks on Subject<Reactions> {

test/api/model/model_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ void main() {
8989
check(baseStreamJson()).not((it) => it.containsKey('topic'));
9090
check(Message.fromJson(baseStreamJson()
9191
..['subject'] = 'hello'
92-
)).topic.equals('hello');
92+
)).isA<StreamMessage>()
93+
.topic.equals('hello');
9394
});
9495

9596
test('match_subject -> matchTopic', () {

0 commit comments

Comments
 (0)