Skip to content

Commit fcdf45e

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 e40dd90 commit fcdf45e

File tree

5 files changed

+85
-51
lines changed

5 files changed

+85
-51
lines changed

lib/api/model/model.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,11 @@ 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 moves across channels,
580+
// [displayRecipient] still refers to the original channel and it has to be
581+
// invalidated.
582+
@JsonKey(required: true, disallowNullValue: true)
583+
String? displayRecipient;
580584
final int streamId;
581585

582586
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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,9 @@ class StreamMessageRecipientHeader extends StatelessWidget {
978978
streamWidget = const SizedBox(width: 16);
979979
} else {
980980
final stream = store.streams[message.streamId];
981-
final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing
981+
final streamName = stream?.name
982+
?? message.displayRecipient
983+
?? '(unknown channel)'; // TODO(log)
982984

983985
streamWidget = GestureDetector(
984986
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)