Skip to content

Commit 8778efe

Browse files
gnpricegnprice
committed
api [nfc]: Rename Message.topic, from "subject"
The name "subject" is long obsolete for Zulip topics. It persists in a few places in the API, but we should keep it out of our code. We had it here on the Message type really just because this is a part of the code that I wrote in the very first hours of the prototype of this app, before I'd read up enough on the json_serialization package to find `JsonKey.name`. There's a TODO referring to a few other similar names in the API; we'll handle those shortly.
1 parent 4a4f934 commit 8778efe

15 files changed

+42
-31
lines changed

‎lib/api/model/model.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,8 @@ sealed class Message {
646646
final String senderFullName;
647647
final int senderId;
648648
final String senderRealmStr;
649-
final String subject; // TODO call it "topic" internally; also similar others
649+
@JsonKey(name: 'subject')
650+
final String topic;
650651
// final List<string> submessages; // TODO handle
651652
final int timestamp;
652653
String get type;
@@ -685,7 +686,7 @@ sealed class Message {
685686
required this.senderFullName,
686687
required this.senderId,
687688
required this.senderRealmStr,
688-
required this.subject,
689+
required this.topic,
689690
required this.timestamp,
690691
required this.flags,
691692
required this.matchContent,
@@ -750,7 +751,7 @@ class StreamMessage extends Message {
750751
required super.senderFullName,
751752
required super.senderId,
752753
required super.senderRealmStr,
753-
required super.subject,
754+
required super.topic,
754755
required super.timestamp,
755756
required super.flags,
756757
required super.matchContent,
@@ -852,7 +853,7 @@ class DmMessage extends Message {
852853
required super.senderFullName,
853854
required super.senderId,
854855
required super.senderRealmStr,
855-
required super.subject,
856+
required super.topic,
856857
required super.timestamp,
857858
required super.flags,
858859
required super.matchContent,

‎lib/api/model/model.g.dart

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

‎lib/model/message_list.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ mixin _MessageSequence {
244244
bool haveSameRecipient(Message prevMessage, Message message) {
245245
if (prevMessage is StreamMessage && message is StreamMessage) {
246246
if (prevMessage.streamId != message.streamId) return false;
247-
if (prevMessage.subject != message.subject) return false;
247+
if (prevMessage.topic != message.topic) return false;
248248
} else if (prevMessage is DmMessage && message is DmMessage) {
249249
if (!_equalIdSequences(prevMessage.allRecipientIds, message.allRecipientIds)) {
250250
return false;
@@ -335,14 +335,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
335335
case CombinedFeedNarrow():
336336
return switch (message) {
337337
StreamMessage() =>
338-
store.isTopicVisible(message.streamId, message.subject),
338+
store.isTopicVisible(message.streamId, message.topic),
339339
DmMessage() => true,
340340
};
341341

342342
case StreamNarrow(:final streamId):
343343
assert(message is StreamMessage && message.streamId == streamId);
344344
if (message is! StreamMessage) return false;
345-
return store.isTopicVisibleInStream(streamId, message.subject);
345+
return store.isTopicVisibleInStream(streamId, message.topic);
346346

347347
case TopicNarrow():
348348
case DmNarrow():

‎lib/model/narrow.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {
9595
const TopicNarrow(this.streamId, this.topic);
9696

9797
factory TopicNarrow.ofMessage(StreamMessage message) {
98-
return TopicNarrow(message.streamId, message.subject);
98+
return TopicNarrow(message.streamId, message.topic);
9999
}
100100

101101
final int streamId;
@@ -104,7 +104,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {
104104
@override
105105
bool containsMessage(Message message) {
106106
return (message is StreamMessage
107-
&& message.streamId == streamId && message.subject == topic);
107+
&& message.streamId == streamId && message.topic == topic);
108108
}
109109

110110
@override

‎lib/model/unreads.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ class Unreads extends ChangeNotifier {
214214

215215
switch (message) {
216216
case StreamMessage():
217-
_addLastInStreamTopic(message.id, message.streamId, message.subject);
217+
_addLastInStreamTopic(message.id, message.streamId, message.topic);
218218
case DmMessage():
219219
final narrow = DmNarrow.ofMessage(message, selfUserId: selfUserId);
220220
_addLastInDm(message.id, narrow);

‎lib/widgets/action_sheet.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
300300
&& topicController.textNormalized == kNoTopicTopic
301301
&& message is StreamMessage
302302
) {
303-
topicController.value = TextEditingValue(text: message.subject);
303+
topicController.value = TextEditingValue(text: message.topic);
304304
}
305305
final tag = composeBoxController.contentController
306306
.registerQuoteAndReplyStart(PerAccountStoreWidget.of(messageListContext),

‎lib/widgets/message_list.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
657657
// https://github.com/zulip/zulip-mobile/issues/5511
658658
final store = PerAccountStoreWidget.of(context);
659659

660-
final topic = message.subject;
660+
final topic = message.topic;
661661

662662
final subscription = store.subscriptions[message.streamId];
663663
final Color backgroundColor;

‎test/api/model/model_checks.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ extension MessageChecks on Subject<Message> {
5050
Subject<String> get senderFullName => has((e) => e.senderFullName, 'senderFullName');
5151
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
5252
Subject<String> get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr');
53-
Subject<String> get subject => has((e) => e.subject, 'subject');
53+
Subject<String> get topic => has((e) => e.topic, 'topic');
5454
Subject<int> get timestamp => has((e) => e.timestamp, 'timestamp');
5555
Subject<String> get type => has((e) => e.type, 'type');
5656
Subject<List<MessageFlag>> get flags => has((e) => e.flags, 'flags');

‎test/api/model/model_test.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,16 @@ void main() {
551551
});
552552

553553
group('Message', () {
554+
Map<String, dynamic> baseStreamJson() =>
555+
deepToJson(eg.streamMessage()) as Map<String, dynamic>;
556+
557+
test('subject -> topic', () {
558+
check(baseStreamJson()).not((it) => it.containsKey('topic'));
559+
check(Message.fromJson(baseStreamJson()
560+
..['subject'] = 'hello'
561+
)).topic.equals('hello');
562+
});
563+
554564
test('no crash on unrecognized flag', () {
555565
final m1 = Message.fromJson(
556566
(deepToJson(eg.streamMessage()) as Map<String, dynamic>)

‎test/example_data.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent(
439439
type: MessageType.stream,
440440
mentioned: mentioned,
441441
streamId: message.streamId,
442-
topic: message.subject,
442+
topic: message.topic,
443443
userIds: null,
444444
),
445445
DmMessage() => UpdateMessageFlagsMessageDetail(

‎test/model/message_list_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -873,10 +873,10 @@ void checkInvariants(MessageListView model) {
873873
if (message is! StreamMessage) continue;
874874
switch (model.narrow) {
875875
case CombinedFeedNarrow():
876-
check(model.store.isTopicVisible(message.streamId, message.subject))
876+
check(model.store.isTopicVisible(message.streamId, message.topic))
877877
.isTrue();
878878
case StreamNarrow():
879-
check(model.store.isTopicVisibleInStream(message.streamId, message.subject))
879+
check(model.store.isTopicVisibleInStream(message.streamId, message.topic))
880880
.isTrue();
881881
case TopicNarrow():
882882
case DmNarrow():

‎test/model/narrow_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ void main() {
2727
final stream = eg.stream();
2828
final message = eg.streamMessage(stream: stream);
2929
final actual = TopicNarrow.ofMessage(message);
30-
check(actual).equals(TopicNarrow(stream.streamId, message.subject));
30+
check(actual).equals(TopicNarrow(stream.streamId, message.topic));
3131
});
3232
});
3333

‎test/model/unreads_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void main() {
6868
switch (message) {
6969
case StreamMessage():
7070
final perTopic = expectedStreams[message.streamId] ??= {};
71-
final messageIds = perTopic[message.subject] ??= QueueList();
71+
final messageIds = perTopic[message.topic] ??= QueueList();
7272
messageIds.add(message.id);
7373
case DmMessage():
7474
final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
@@ -445,7 +445,7 @@ void main() {
445445
messageType: MessageType.stream,
446446
messageIds: [message.id],
447447
streamId: message.streamId,
448-
topic: message.subject,
448+
topic: message.topic,
449449
),
450450
DmMessage() => DeleteMessageEvent(
451451
id: 0,
@@ -506,7 +506,7 @@ void main() {
506506
messageIds: [message.id],
507507
messageType: MessageType.stream,
508508
streamId: message.streamId,
509-
topic: message.subject,
509+
topic: message.topic,
510510
));
511511
// TODO improve implementation; then:
512512
// checkNotNotified();

‎test/notifications/display_test.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,17 +181,17 @@ void main() {
181181
final stream = eg.stream();
182182
final message = eg.streamMessage(stream: stream);
183183
await checkNotifications(async, messageFcmMessage(message, streamName: stream.name),
184-
expectedTitle: '#${stream.name} > ${message.subject}',
185-
expectedTagComponent: 'stream:${message.streamId}:${message.subject}');
184+
expectedTitle: '#${stream.name} > ${message.topic}',
185+
expectedTagComponent: 'stream:${message.streamId}:${message.topic}');
186186
}));
187187

188188
test('stream message, stream name omitted', () => awaitFakeAsync((async) async {
189189
await init();
190190
final stream = eg.stream();
191191
final message = eg.streamMessage(stream: stream);
192192
await checkNotifications(async, messageFcmMessage(message, streamName: null),
193-
expectedTitle: '#(unknown channel) > ${message.subject}',
194-
expectedTagComponent: 'stream:${message.streamId}:${message.subject}');
193+
expectedTitle: '#(unknown channel) > ${message.topic}',
194+
expectedTagComponent: 'stream:${message.streamId}:${message.topic}');
195195
}));
196196

197197
test('group DM: 3 users', () => awaitFakeAsync((async) async {

‎test/widgets/message_list_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ void main() {
705705
testWidgets('from unread to read', (WidgetTester tester) async {
706706
final message = eg.streamMessage(flags: []);
707707
final unreadMsgs = eg.unreadMsgs(streams:[
708-
UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId, unreadMessageIds: [message.id])
708+
UnreadStreamSnapshot(topic: message.topic, streamId: message.streamId, unreadMessageIds: [message.id])
709709
]);
710710
await setupMessageListPage(tester, messages: [message], unreadMsgs: unreadMsgs);
711711
check(isMarkAsReadButtonVisible(tester)).isTrue();
@@ -723,7 +723,7 @@ void main() {
723723
testWidgets("messages don't shift position", (WidgetTester tester) async {
724724
final message = eg.streamMessage(flags: []);
725725
final unreadMsgs = eg.unreadMsgs(streams:[
726-
UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId,
726+
UnreadStreamSnapshot(topic: message.topic, streamId: message.streamId,
727727
unreadMessageIds: [message.id])
728728
]);
729729
await setupMessageListPage(tester,
@@ -748,7 +748,7 @@ void main() {
748748
group('onPressed behavior', () {
749749
final message = eg.streamMessage(flags: []);
750750
final unreadMsgs = eg.unreadMsgs(streams: [
751-
UnreadStreamSnapshot(streamId: message.streamId, topic: message.subject,
751+
UnreadStreamSnapshot(streamId: message.streamId, topic: message.topic,
752752
unreadMessageIds: [message.id]),
753753
]);
754754

0 commit comments

Comments
 (0)