Skip to content

Commit ce741cb

Browse files
committed
model [nfc]: Make displayRecipient nullable.
We want to only allow it to be nullable after deserialization, because the nullability does not belong to the API, and it exists only for later implementing invalidation of the field when a stream message is moved to a different stream. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 60d7246 commit ce741cb

File tree

5 files changed

+85
-51
lines changed

5 files changed

+85
-51
lines changed

lib/api/model/model.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,10 @@ class StreamMessage extends Message {
576576
@JsonKey(includeToJson: true)
577577
String get type => 'stream';
578578

579-
final String displayRecipient;
579+
// This is not nullable API-wise, but if the message move across streams we
580+
// might set [displayRecipient] to null to invalidate it.
581+
@JsonKey(required: true, disallowNullValue: true)
582+
String? displayRecipient;
580583
int streamId;
581584

582585
StreamMessage({

lib/api/model/model.g.dart

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

lib/widgets/message_list.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:intl/intl.dart';
88
import '../api/model/model.dart';
99
import '../api/model/narrow.dart';
1010
import '../api/route/messages.dart';
11+
import '../log.dart';
1112
import '../model/message_list.dart';
1213
import '../model/narrow.dart';
1314
import '../model/store.dart';
@@ -679,7 +680,9 @@ class StreamMessageRecipientHeader extends StatelessWidget {
679680
streamWidget = const SizedBox(width: 16);
680681
} else {
681682
final stream = store.streams[message.streamId];
682-
final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing
683+
final streamName = stream?.name
684+
?? message.displayRecipient
685+
?? '(unknown channel)'; // TODO(log)
683686

684687
streamWidget = GestureDetector(
685688
onTap: () => Navigator.push(context,

test/api/model/model_checks.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ extension MessageChecks on Subject<Message> {
4949
Subject<String?> get matchTopic => has((e) => e.matchTopic, 'matchTopic');
5050
}
5151

52+
extension StreamMessageChecks on Subject<StreamMessage> {
53+
Subject<String?> get displayRecipient => has((e) => e.displayRecipient, 'displayRecipient');
54+
}
55+
5256
extension ReactionsChecks on Subject<Reactions> {
5357
Subject<int> get total => has((e) => e.total, 'total');
5458
Subject<List<ReactionWithVotes>> get aggregated => has((e) => e.aggregated, 'aggregated');

test/api/model/model_test.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:convert';
22

33
import 'package:checks/checks.dart';
4+
import 'package:json_annotation/json_annotation.dart';
45
import 'package:test/scaffolding.dart';
56
import 'package:zulip/api/model/model.dart';
67

@@ -149,6 +150,14 @@ void main() {
149150
check(m2).flags.deepEquals([MessageFlag.read, MessageFlag.unknown]);
150151
});
151152

153+
test('require displayRecipient on parse', () {
154+
check(() => StreamMessage.fromJson(baseStreamJson()..['display_recipient'] = null))
155+
.throws<DisallowedNullValueException>();
156+
157+
check(() => StreamMessage.fromJson(baseStreamJson()..remove('display_recipient')))
158+
.throws<MissingRequiredKeysException>();
159+
});
160+
152161
// Code relevant to messageEditState is tested separately in the
153162
// MessageEditState group.
154163
});

0 commit comments

Comments
 (0)