From 278f9d3f44f49b5c0c576ef78f94460d3d18045f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Jun 2024 13:27:20 -0700 Subject: [PATCH 1/6] api: Ignore stream_name on UpdateMessageEvent This field is obsoleted by `stream_id`. --- lib/api/model/events.dart | 3 +-- lib/api/model/events.g.dart | 2 -- test/example_data.dart | 2 -- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 5c93c553b4..17972e647e 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -619,7 +619,7 @@ class UpdateMessageEvent extends Event { final List messageIds; final List flags; final int? editTimestamp; // TODO(server-5) - final String? streamName; + // final String? streamName; // ignore final int? streamId; final int? newStreamId; final PropagateMode? propagateMode; @@ -641,7 +641,6 @@ class UpdateMessageEvent extends Event { required this.messageIds, required this.flags, required this.editTimestamp, - required this.streamName, required this.streamId, required this.newStreamId, required this.propagateMode, diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index d20ff78d48..4e406e6661 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -363,7 +363,6 @@ UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => .map((e) => $enumDecode(_$MessageFlagEnumMap, e)) .toList(), editTimestamp: (json['edit_timestamp'] as num?)?.toInt(), - streamName: json['stream_name'] as String?, streamId: (json['stream_id'] as num?)?.toInt(), newStreamId: (json['new_stream_id'] as num?)?.toInt(), propagateMode: @@ -386,7 +385,6 @@ Map _$UpdateMessageEventToJson(UpdateMessageEvent instance) => 'message_ids': instance.messageIds, 'flags': instance.flags, 'edit_timestamp': instance.editTimestamp, - 'stream_name': instance.streamName, 'stream_id': instance.streamId, 'new_stream_id': instance.newStreamId, 'propagate_mode': _$PropagateModeEnumMap[instance.propagateMode], diff --git a/test/example_data.dart b/test/example_data.dart index 67ba1aaa3c..393c5fc383 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -395,7 +395,6 @@ UpdateMessageEvent updateMessageEditEvent( int? messageId, List? flags, int? editTimestamp, - String? streamName, String? renderedContent, bool isMeMessage = false, }) { @@ -408,7 +407,6 @@ UpdateMessageEvent updateMessageEditEvent( messageIds: [messageId], flags: flags ?? origMessage.flags, editTimestamp: editTimestamp ?? 1234567890, // TODO generate timestamp - streamName: streamName, streamId: origMessage is StreamMessage ? origMessage.streamId : null, newStreamId: null, propagateMode: null, From 4a4f934b26440fcbce4af6817ac6a7c366cae50e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Jun 2024 13:38:23 -0700 Subject: [PATCH 2/6] test [nfc]: Add all package:checks getters for Message --- test/api/model/model_checks.dart | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index c5ffd1f0e1..b69b799384 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -38,14 +38,24 @@ extension StreamColorSwatchChecks on Subject { } extension MessageChecks on Subject { - Subject get id => has((e) => e.id, 'id'); + Subject get client => has((e) => e.client, 'client'); Subject get content => has((e) => e.content, 'content'); + Subject get contentType => has((e) => e.contentType, 'contentType'); + Subject get id => has((e) => e.id, 'id'); Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); Subject get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp'); Subject get reactions => has((e) => e.reactions, 'reactions'); + Subject get recipientId => has((e) => e.recipientId, 'recipientId'); + Subject get senderEmail => has((e) => e.senderEmail, 'senderEmail'); + Subject get senderFullName => has((e) => e.senderFullName, 'senderFullName'); + Subject get senderId => has((e) => e.senderId, 'senderId'); + Subject get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr'); + Subject get subject => has((e) => e.subject, 'subject'); + Subject get timestamp => has((e) => e.timestamp, 'timestamp'); + Subject get type => has((e) => e.type, 'type'); Subject> get flags => has((e) => e.flags, 'flags'); - - // TODO accessors for other fields + Subject get matchContent => has((e) => e.matchContent, 'matchContent'); + Subject get matchSubject => has((e) => e.matchSubject, 'matchSubject'); } extension ReactionsChecks on Subject { From 8778efe1aa092943c964521cc6599c312c0b6fd9 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Jun 2024 13:31:59 -0700 Subject: [PATCH 3/6] 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. --- lib/api/model/model.dart | 9 +++++---- lib/api/model/model.g.dart | 8 ++++---- lib/model/message_list.dart | 6 +++--- lib/model/narrow.dart | 4 ++-- lib/model/unreads.dart | 2 +- lib/widgets/action_sheet.dart | 2 +- lib/widgets/message_list.dart | 2 +- test/api/model/model_checks.dart | 2 +- test/api/model/model_test.dart | 10 ++++++++++ test/example_data.dart | 2 +- test/model/message_list_test.dart | 4 ++-- test/model/narrow_test.dart | 2 +- test/model/unreads_test.dart | 6 +++--- test/notifications/display_test.dart | 8 ++++---- test/widgets/message_list_test.dart | 6 +++--- 15 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 3f0f8a377d..35fb654e22 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -646,7 +646,8 @@ sealed class Message { final String senderFullName; final int senderId; final String senderRealmStr; - final String subject; // TODO call it "topic" internally; also similar others + @JsonKey(name: 'subject') + final String topic; // final List submessages; // TODO handle final int timestamp; String get type; @@ -685,7 +686,7 @@ sealed class Message { required this.senderFullName, required this.senderId, required this.senderRealmStr, - required this.subject, + required this.topic, required this.timestamp, required this.flags, required this.matchContent, @@ -750,7 +751,7 @@ class StreamMessage extends Message { required super.senderFullName, required super.senderId, required super.senderRealmStr, - required super.subject, + required super.topic, required super.timestamp, required super.flags, required super.matchContent, @@ -852,7 +853,7 @@ class DmMessage extends Message { required super.senderFullName, required super.senderId, required super.senderRealmStr, - required super.subject, + required super.topic, required super.timestamp, required super.flags, required super.matchContent, diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 44cd4adf38..231d663158 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -274,7 +274,7 @@ StreamMessage _$StreamMessageFromJson(Map json) => senderFullName: json['sender_full_name'] as String, senderId: (json['sender_id'] as num).toInt(), senderRealmStr: json['sender_realm_str'] as String, - subject: json['subject'] as String, + topic: json['subject'] as String, timestamp: (json['timestamp'] as num).toInt(), flags: Message._flagsFromJson(json['flags']), matchContent: json['match_content'] as String?, @@ -297,7 +297,7 @@ Map _$StreamMessageToJson(StreamMessage instance) => 'sender_full_name': instance.senderFullName, 'sender_id': instance.senderId, 'sender_realm_str': instance.senderRealmStr, - 'subject': instance.subject, + 'subject': instance.topic, 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, @@ -333,7 +333,7 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( senderFullName: json['sender_full_name'] as String, senderId: (json['sender_id'] as num).toInt(), senderRealmStr: json['sender_realm_str'] as String, - subject: json['subject'] as String, + topic: json['subject'] as String, timestamp: (json['timestamp'] as num).toInt(), flags: Message._flagsFromJson(json['flags']), matchContent: json['match_content'] as String?, @@ -355,7 +355,7 @@ Map _$DmMessageToJson(DmMessage instance) => { 'sender_full_name': instance.senderFullName, 'sender_id': instance.senderId, 'sender_realm_str': instance.senderRealmStr, - 'subject': instance.subject, + 'subject': instance.topic, 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 09c780f35e..b5a921bed6 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -244,7 +244,7 @@ mixin _MessageSequence { bool haveSameRecipient(Message prevMessage, Message message) { if (prevMessage is StreamMessage && message is StreamMessage) { if (prevMessage.streamId != message.streamId) return false; - if (prevMessage.subject != message.subject) return false; + if (prevMessage.topic != message.topic) return false; } else if (prevMessage is DmMessage && message is DmMessage) { if (!_equalIdSequences(prevMessage.allRecipientIds, message.allRecipientIds)) { return false; @@ -335,14 +335,14 @@ class MessageListView with ChangeNotifier, _MessageSequence { case CombinedFeedNarrow(): return switch (message) { StreamMessage() => - store.isTopicVisible(message.streamId, message.subject), + store.isTopicVisible(message.streamId, message.topic), DmMessage() => true, }; case StreamNarrow(:final streamId): assert(message is StreamMessage && message.streamId == streamId); if (message is! StreamMessage) return false; - return store.isTopicVisibleInStream(streamId, message.subject); + return store.isTopicVisibleInStream(streamId, message.topic); case TopicNarrow(): case DmNarrow(): diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index d68c876190..9c8cacd06d 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -95,7 +95,7 @@ class TopicNarrow extends Narrow implements SendableNarrow { const TopicNarrow(this.streamId, this.topic); factory TopicNarrow.ofMessage(StreamMessage message) { - return TopicNarrow(message.streamId, message.subject); + return TopicNarrow(message.streamId, message.topic); } final int streamId; @@ -104,7 +104,7 @@ class TopicNarrow extends Narrow implements SendableNarrow { @override bool containsMessage(Message message) { return (message is StreamMessage - && message.streamId == streamId && message.subject == topic); + && message.streamId == streamId && message.topic == topic); } @override diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index a355dcdc2b..c1c375c657 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -214,7 +214,7 @@ class Unreads extends ChangeNotifier { switch (message) { case StreamMessage(): - _addLastInStreamTopic(message.id, message.streamId, message.subject); + _addLastInStreamTopic(message.id, message.streamId, message.topic); case DmMessage(): final narrow = DmNarrow.ofMessage(message, selfUserId: selfUserId); _addLastInDm(message.id, narrow); diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index f185f16d03..424d666016 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -300,7 +300,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { && topicController.textNormalized == kNoTopicTopic && message is StreamMessage ) { - topicController.value = TextEditingValue(text: message.subject); + topicController.value = TextEditingValue(text: message.topic); } final tag = composeBoxController.contentController .registerQuoteAndReplyStart(PerAccountStoreWidget.of(messageListContext), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 15cf168129..77af198785 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -657,7 +657,7 @@ class StreamMessageRecipientHeader extends StatelessWidget { // https://github.com/zulip/zulip-mobile/issues/5511 final store = PerAccountStoreWidget.of(context); - final topic = message.subject; + final topic = message.topic; final subscription = store.subscriptions[message.streamId]; final Color backgroundColor; diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index b69b799384..1a0785d942 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -50,7 +50,7 @@ extension MessageChecks on Subject { Subject get senderFullName => has((e) => e.senderFullName, 'senderFullName'); Subject get senderId => has((e) => e.senderId, 'senderId'); Subject get senderRealmStr => has((e) => e.senderRealmStr, 'senderRealmStr'); - Subject get subject => has((e) => e.subject, 'subject'); + Subject get topic => has((e) => e.topic, 'topic'); Subject get timestamp => has((e) => e.timestamp, 'timestamp'); Subject get type => has((e) => e.type, 'type'); Subject> get flags => has((e) => e.flags, 'flags'); diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index dc215e71f7..16fa732e7e 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -551,6 +551,16 @@ void main() { }); group('Message', () { + Map baseStreamJson() => + deepToJson(eg.streamMessage()) as Map; + + test('subject -> topic', () { + check(baseStreamJson()).not((it) => it.containsKey('topic')); + check(Message.fromJson(baseStreamJson() + ..['subject'] = 'hello' + )).topic.equals('hello'); + }); + test('no crash on unrecognized flag', () { final m1 = Message.fromJson( (deepToJson(eg.streamMessage()) as Map) diff --git a/test/example_data.dart b/test/example_data.dart index 393c5fc383..c932176297 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -439,7 +439,7 @@ UpdateMessageFlagsRemoveEvent updateMessageFlagsRemoveEvent( type: MessageType.stream, mentioned: mentioned, streamId: message.streamId, - topic: message.subject, + topic: message.topic, userIds: null, ), DmMessage() => UpdateMessageFlagsMessageDetail( diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index c818970c30..de0c7f32a7 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -873,10 +873,10 @@ void checkInvariants(MessageListView model) { if (message is! StreamMessage) continue; switch (model.narrow) { case CombinedFeedNarrow(): - check(model.store.isTopicVisible(message.streamId, message.subject)) + check(model.store.isTopicVisible(message.streamId, message.topic)) .isTrue(); case StreamNarrow(): - check(model.store.isTopicVisibleInStream(message.streamId, message.subject)) + check(model.store.isTopicVisibleInStream(message.streamId, message.topic)) .isTrue(); case TopicNarrow(): case DmNarrow(): diff --git a/test/model/narrow_test.dart b/test/model/narrow_test.dart index 5d671173b9..af75b3e6c2 100644 --- a/test/model/narrow_test.dart +++ b/test/model/narrow_test.dart @@ -27,7 +27,7 @@ void main() { final stream = eg.stream(); final message = eg.streamMessage(stream: stream); final actual = TopicNarrow.ofMessage(message); - check(actual).equals(TopicNarrow(stream.streamId, message.subject)); + check(actual).equals(TopicNarrow(stream.streamId, message.topic)); }); }); diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index e971d2b740..e72b3cbca4 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -68,7 +68,7 @@ void main() { switch (message) { case StreamMessage(): final perTopic = expectedStreams[message.streamId] ??= {}; - final messageIds = perTopic[message.subject] ??= QueueList(); + final messageIds = perTopic[message.topic] ??= QueueList(); messageIds.add(message.id); case DmMessage(): final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId); @@ -445,7 +445,7 @@ void main() { messageType: MessageType.stream, messageIds: [message.id], streamId: message.streamId, - topic: message.subject, + topic: message.topic, ), DmMessage() => DeleteMessageEvent( id: 0, @@ -506,7 +506,7 @@ void main() { messageIds: [message.id], messageType: MessageType.stream, streamId: message.streamId, - topic: message.subject, + topic: message.topic, )); // TODO improve implementation; then: // checkNotNotified(); diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 5c352123d3..b1a7183b87 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -181,8 +181,8 @@ void main() { final stream = eg.stream(); final message = eg.streamMessage(stream: stream); await checkNotifications(async, messageFcmMessage(message, streamName: stream.name), - expectedTitle: '#${stream.name} > ${message.subject}', - expectedTagComponent: 'stream:${message.streamId}:${message.subject}'); + expectedTitle: '#${stream.name} > ${message.topic}', + expectedTagComponent: 'stream:${message.streamId}:${message.topic}'); })); test('stream message, stream name omitted', () => awaitFakeAsync((async) async { @@ -190,8 +190,8 @@ void main() { final stream = eg.stream(); final message = eg.streamMessage(stream: stream); await checkNotifications(async, messageFcmMessage(message, streamName: null), - expectedTitle: '#(unknown channel) > ${message.subject}', - expectedTagComponent: 'stream:${message.streamId}:${message.subject}'); + expectedTitle: '#(unknown channel) > ${message.topic}', + expectedTagComponent: 'stream:${message.streamId}:${message.topic}'); })); test('group DM: 3 users', () => awaitFakeAsync((async) async { diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 87054b8250..d54b7c1cd5 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -705,7 +705,7 @@ void main() { testWidgets('from unread to read', (WidgetTester tester) async { final message = eg.streamMessage(flags: []); final unreadMsgs = eg.unreadMsgs(streams:[ - UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId, unreadMessageIds: [message.id]) + UnreadStreamSnapshot(topic: message.topic, streamId: message.streamId, unreadMessageIds: [message.id]) ]); await setupMessageListPage(tester, messages: [message], unreadMsgs: unreadMsgs); check(isMarkAsReadButtonVisible(tester)).isTrue(); @@ -723,7 +723,7 @@ void main() { testWidgets("messages don't shift position", (WidgetTester tester) async { final message = eg.streamMessage(flags: []); final unreadMsgs = eg.unreadMsgs(streams:[ - UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId, + UnreadStreamSnapshot(topic: message.topic, streamId: message.streamId, unreadMessageIds: [message.id]) ]); await setupMessageListPage(tester, @@ -748,7 +748,7 @@ void main() { group('onPressed behavior', () { final message = eg.streamMessage(flags: []); final unreadMsgs = eg.unreadMsgs(streams: [ - UnreadStreamSnapshot(streamId: message.streamId, topic: message.subject, + UnreadStreamSnapshot(streamId: message.streamId, topic: message.topic, unreadMessageIds: [message.id]), ]); From 98a2e7cb40849bbc58492b428c14464af4d16393 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Jun 2024 13:32:25 -0700 Subject: [PATCH 4/6] api [nfc]: Rename Message.matchTopic, from matchSubject Much like the `topic` field in the previous commit. --- lib/api/model/events.dart | 4 ++-- lib/api/model/model.dart | 9 +++++---- lib/api/model/model.g.dart | 8 ++++---- test/api/model/model_checks.dart | 2 +- test/api/model/model_test.dart | 7 +++++++ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 17972e647e..fddbe4b86f 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -584,8 +584,8 @@ class MessageEvent extends Event { // // The other difference in the server API between message objects in these // events and in the get-messages results is that `matchContent` and - // `matchSubject` are absent here. Already [Message.matchContent] and - // [Message.matchSubject] are optional, so no action is needed on that. + // `matchTopic` are absent here. Already [Message.matchContent] and + // [Message.matchTopic] are optional, so no action is needed on that. final Message message; MessageEvent({required super.id, required this.message}); diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 35fb654e22..5b24b4dd7e 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -657,7 +657,8 @@ sealed class Message { @JsonKey(fromJson: _flagsFromJson) List flags; // Unrecognized flags won't roundtrip through {to,from}Json. final String? matchContent; - final String? matchSubject; + @JsonKey(name: 'match_subject') + final String? matchTopic; static Reactions? _reactionsFromJson(dynamic json) { final list = (json as List); @@ -690,7 +691,7 @@ sealed class Message { required this.timestamp, required this.flags, required this.matchContent, - required this.matchSubject, + required this.matchTopic, }); factory Message.fromJson(Map json) { @@ -755,7 +756,7 @@ class StreamMessage extends Message { required super.timestamp, required super.flags, required super.matchContent, - required super.matchSubject, + required super.matchTopic, required this.displayRecipient, required this.streamId, }); @@ -857,7 +858,7 @@ class DmMessage extends Message { required super.timestamp, required super.flags, required super.matchContent, - required super.matchSubject, + required super.matchTopic, required this.displayRecipient, }); diff --git a/lib/api/model/model.g.dart b/lib/api/model/model.g.dart index 231d663158..b835a7f953 100644 --- a/lib/api/model/model.g.dart +++ b/lib/api/model/model.g.dart @@ -278,7 +278,7 @@ StreamMessage _$StreamMessageFromJson(Map json) => timestamp: (json['timestamp'] as num).toInt(), flags: Message._flagsFromJson(json['flags']), matchContent: json['match_content'] as String?, - matchSubject: json['match_subject'] as String?, + matchTopic: json['match_subject'] as String?, displayRecipient: json['display_recipient'] as String, streamId: (json['stream_id'] as num).toInt(), ); @@ -301,7 +301,7 @@ Map _$StreamMessageToJson(StreamMessage instance) => 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, - 'match_subject': instance.matchSubject, + 'match_subject': instance.matchTopic, 'type': instance.type, 'display_recipient': instance.displayRecipient, 'stream_id': instance.streamId, @@ -337,7 +337,7 @@ DmMessage _$DmMessageFromJson(Map json) => DmMessage( timestamp: (json['timestamp'] as num).toInt(), flags: Message._flagsFromJson(json['flags']), matchContent: json['match_content'] as String?, - matchSubject: json['match_subject'] as String?, + matchTopic: json['match_subject'] as String?, displayRecipient: const DmRecipientListConverter() .fromJson(json['display_recipient'] as List), ); @@ -359,7 +359,7 @@ Map _$DmMessageToJson(DmMessage instance) => { 'timestamp': instance.timestamp, 'flags': instance.flags, 'match_content': instance.matchContent, - 'match_subject': instance.matchSubject, + 'match_subject': instance.matchTopic, 'type': instance.type, 'display_recipient': const DmRecipientListConverter().toJson(instance.displayRecipient), diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 1a0785d942..d15bca341c 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -55,7 +55,7 @@ extension MessageChecks on Subject { Subject get type => has((e) => e.type, 'type'); Subject> get flags => has((e) => e.flags, 'flags'); Subject get matchContent => has((e) => e.matchContent, 'matchContent'); - Subject get matchSubject => has((e) => e.matchSubject, 'matchSubject'); + Subject get matchTopic => has((e) => e.matchTopic, 'matchTopic'); } extension ReactionsChecks on Subject { diff --git a/test/api/model/model_test.dart b/test/api/model/model_test.dart index 16fa732e7e..537b8988b4 100644 --- a/test/api/model/model_test.dart +++ b/test/api/model/model_test.dart @@ -561,6 +561,13 @@ void main() { )).topic.equals('hello'); }); + test('match_subject -> matchTopic', () { + check(baseStreamJson()).not((it) => it.containsKey('match_topic')); + check(Message.fromJson(baseStreamJson() + ..['match_subject'] = 'yo' + )).matchTopic.equals('yo'); + }); + test('no crash on unrecognized flag', () { final m1 = Message.fromJson( (deepToJson(eg.streamMessage()) as Map) From dd78af1e304e7cd258d8e65a23db815ba4d0ccf7 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 18 Jun 2024 14:11:32 -0700 Subject: [PATCH 5/6] api [nfc]: Rename subject/topic on UpdateMessageEvent This completes the renaming from "subject" to "topic" in our code. --- lib/api/model/events.dart | 12 ++++++++---- lib/api/model/events.g.dart | 8 ++++---- test/api/model/events_checks.dart | 19 +++++++++++++++++++ test/api/model/events_test.dart | 24 ++++++++++++++++++++++++ test/example_data.dart | 4 ++-- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index fddbe4b86f..602c7d6392 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -623,8 +623,12 @@ class UpdateMessageEvent extends Event { final int? streamId; final int? newStreamId; final PropagateMode? propagateMode; - final String? origSubject; - final String? subject; + + @JsonKey(name: 'orig_subject') + final String? origTopic; + @JsonKey(name: 'subject') + final String? topic; + // final List topicLinks; // TODO handle final String? origContent; final String? origRenderedContent; @@ -644,8 +648,8 @@ class UpdateMessageEvent extends Event { required this.streamId, required this.newStreamId, required this.propagateMode, - required this.origSubject, - required this.subject, + required this.origTopic, + required this.topic, required this.origContent, required this.origRenderedContent, required this.content, diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 4e406e6661..873c047239 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -367,8 +367,8 @@ UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => newStreamId: (json['new_stream_id'] as num?)?.toInt(), propagateMode: $enumDecodeNullable(_$PropagateModeEnumMap, json['propagate_mode']), - origSubject: json['orig_subject'] as String?, - subject: json['subject'] as String?, + origTopic: json['orig_subject'] as String?, + topic: json['subject'] as String?, origContent: json['orig_content'] as String?, origRenderedContent: json['orig_rendered_content'] as String?, content: json['content'] as String?, @@ -388,8 +388,8 @@ Map _$UpdateMessageEventToJson(UpdateMessageEvent instance) => 'stream_id': instance.streamId, 'new_stream_id': instance.newStreamId, 'propagate_mode': _$PropagateModeEnumMap[instance.propagateMode], - 'orig_subject': instance.origSubject, - 'subject': instance.subject, + 'orig_subject': instance.origTopic, + 'subject': instance.topic, 'orig_content': instance.origContent, 'orig_rendered_content': instance.origRenderedContent, 'content': instance.content, diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index cffa2b4074..ae54f75b19 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -27,6 +27,25 @@ extension MessageEventChecks on Subject { Subject get message => has((e) => e.message, 'message'); } +extension UpdateMessageEventChecks on Subject { + Subject get userId => has((e) => e.userId, 'userId'); + Subject get renderingOnly => has((e) => e.renderingOnly, 'renderingOnly'); + Subject get messageId => has((e) => e.messageId, 'messageId'); + Subject> get messageIds => has((e) => e.messageIds, 'messageIds'); + Subject> get flags => has((e) => e.flags, 'flags'); + Subject get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp'); + Subject get streamId => has((e) => e.streamId, 'streamId'); + Subject get newStreamId => has((e) => e.newStreamId, 'newStreamId'); + Subject get propagateMode => has((e) => e.propagateMode, 'propagateMode'); + Subject get origTopic => has((e) => e.origTopic, 'origTopic'); + Subject get topic => has((e) => e.topic, 'topic'); + Subject get origContent => has((e) => e.origContent, 'origContent'); + Subject get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent'); + Subject get content => has((e) => e.content, 'content'); + Subject get renderedContent => has((e) => e.renderedContent, 'renderedContent'); + Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); +} + extension HeartbeatEventChecks on Subject { // No properties not covered by Event. } diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart index 7d5fceaa3c..1352f9ea1c 100644 --- a/test/api/model/events_test.dart +++ b/test/api/model/events_test.dart @@ -66,6 +66,30 @@ void main() { check(mkEvent([MessageFlag.read])).message.flags.deepEquals([MessageFlag.read]); }); + group('update_message', () { + final message = eg.streamMessage(); + final baseJson = { + 'id': 1, + 'type': 'update_message', + 'user_id': eg.selfUser.userId, + 'rendering_only': false, + 'message_id': message.id, + 'message_ids': [message.id], + 'flags': [], + 'edit_timestamp': 1718741351, + 'stream_id': eg.stream().streamId, + }; + + test('orig_subject -> origTopic, subject -> topic', () { + check(Event.fromJson({ ...baseJson, + 'orig_subject': 'foo', + 'subject': 'bar', + }) as UpdateMessageEvent) + ..origTopic.equals('foo') + ..topic.equals('bar'); + }); + }); + test('delete_message: require streamId and topic for stream messages', () { check(() => DeleteMessageEvent.fromJson({ 'id': 1, diff --git a/test/example_data.dart b/test/example_data.dart index c932176297..c72f8ad970 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -410,8 +410,8 @@ UpdateMessageEvent updateMessageEditEvent( streamId: origMessage is StreamMessage ? origMessage.streamId : null, newStreamId: null, propagateMode: null, - origSubject: null, - subject: null, + origTopic: null, + topic: null, origContent: 'some probably-mismatched old Markdown', origRenderedContent: origMessage.content, content: 'some probably-mismatched new Markdown', From 6c949eaf8711d6693ed41d15ec80e4fa21f76c35 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 5 May 2024 15:16:15 -0700 Subject: [PATCH 6/6] api [nfc]: Rationalize "orig"/"new" field names on UpdateMessageEvent In the server API here, "stream_id" is the old value and contrasts with "new_stream_id", but "subject" is the new value and contrasts with "orig_subject". This makes them confusing to read, and is the way it is only for historical reasons: I believe the story is that before Zulip Server 3.0 added support for moving topics between streams, three of those properties were already present and then "new_stream_id" was squeezed in at the best name still available. Fixing that in the API will be a pain because it'll need a migration path for compatibility. But we can fix it here within our code. --- lib/api/model/events.dart | 15 +++++++++++---- lib/api/model/events.g.dart | 8 ++++---- test/api/model/events_checks.dart | 4 ++-- test/api/model/events_test.dart | 13 +++++++++++-- test/example_data.dart | 4 ++-- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 602c7d6392..f8062cc343 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -617,24 +617,31 @@ class UpdateMessageEvent extends Event { final bool? renderingOnly; // TODO(server-5) final int messageId; final List messageIds; + final List flags; final int? editTimestamp; // TODO(server-5) + // final String? streamName; // ignore - final int? streamId; + + @JsonKey(name: 'stream_id') + final int? origStreamId; final int? newStreamId; + final PropagateMode? propagateMode; @JsonKey(name: 'orig_subject') final String? origTopic; @JsonKey(name: 'subject') - final String? topic; + final String? newTopic; // final List topicLinks; // TODO handle + final String? origContent; final String? origRenderedContent; // final int? prevRenderedContentVersion; // deprecated final String? content; final String? renderedContent; + final bool? isMeMessage; UpdateMessageEvent({ @@ -645,11 +652,11 @@ class UpdateMessageEvent extends Event { required this.messageIds, required this.flags, required this.editTimestamp, - required this.streamId, + required this.origStreamId, required this.newStreamId, required this.propagateMode, required this.origTopic, - required this.topic, + required this.newTopic, required this.origContent, required this.origRenderedContent, required this.content, diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 873c047239..e31846d7fb 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -363,12 +363,12 @@ UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => .map((e) => $enumDecode(_$MessageFlagEnumMap, e)) .toList(), editTimestamp: (json['edit_timestamp'] as num?)?.toInt(), - streamId: (json['stream_id'] as num?)?.toInt(), + origStreamId: (json['stream_id'] as num?)?.toInt(), newStreamId: (json['new_stream_id'] as num?)?.toInt(), propagateMode: $enumDecodeNullable(_$PropagateModeEnumMap, json['propagate_mode']), origTopic: json['orig_subject'] as String?, - topic: json['subject'] as String?, + newTopic: json['subject'] as String?, origContent: json['orig_content'] as String?, origRenderedContent: json['orig_rendered_content'] as String?, content: json['content'] as String?, @@ -385,11 +385,11 @@ Map _$UpdateMessageEventToJson(UpdateMessageEvent instance) => 'message_ids': instance.messageIds, 'flags': instance.flags, 'edit_timestamp': instance.editTimestamp, - 'stream_id': instance.streamId, + 'stream_id': instance.origStreamId, 'new_stream_id': instance.newStreamId, 'propagate_mode': _$PropagateModeEnumMap[instance.propagateMode], 'orig_subject': instance.origTopic, - 'subject': instance.topic, + 'subject': instance.newTopic, 'orig_content': instance.origContent, 'orig_rendered_content': instance.origRenderedContent, 'content': instance.content, diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index ae54f75b19..0f55f68931 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -34,11 +34,11 @@ extension UpdateMessageEventChecks on Subject { Subject> get messageIds => has((e) => e.messageIds, 'messageIds'); Subject> get flags => has((e) => e.flags, 'flags'); Subject get editTimestamp => has((e) => e.editTimestamp, 'editTimestamp'); - Subject get streamId => has((e) => e.streamId, 'streamId'); + Subject get origStreamId => has((e) => e.origStreamId, 'origStreamId'); Subject get newStreamId => has((e) => e.newStreamId, 'newStreamId'); Subject get propagateMode => has((e) => e.propagateMode, 'propagateMode'); Subject get origTopic => has((e) => e.origTopic, 'origTopic'); - Subject get topic => has((e) => e.topic, 'topic'); + Subject get newTopic => has((e) => e.newTopic, 'newTopic'); Subject get origContent => has((e) => e.origContent, 'origContent'); Subject get origRenderedContent => has((e) => e.origRenderedContent, 'origRenderedContent'); Subject get content => has((e) => e.content, 'content'); diff --git a/test/api/model/events_test.dart b/test/api/model/events_test.dart index 1352f9ea1c..e55a0cf17d 100644 --- a/test/api/model/events_test.dart +++ b/test/api/model/events_test.dart @@ -80,13 +80,22 @@ void main() { 'stream_id': eg.stream().streamId, }; - test('orig_subject -> origTopic, subject -> topic', () { + test('stream_id -> origStreamId', () { + check(Event.fromJson({ ...baseJson, + 'stream_id': 1, + 'new_stream_id': 2, + }) as UpdateMessageEvent) + ..origStreamId.equals(1) + ..newStreamId.equals(2); + }); + + test('orig_subject -> origTopic, subject -> newTopic', () { check(Event.fromJson({ ...baseJson, 'orig_subject': 'foo', 'subject': 'bar', }) as UpdateMessageEvent) ..origTopic.equals('foo') - ..topic.equals('bar'); + ..newTopic.equals('bar'); }); }); diff --git a/test/example_data.dart b/test/example_data.dart index c72f8ad970..36fc60fec0 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -407,11 +407,11 @@ UpdateMessageEvent updateMessageEditEvent( messageIds: [messageId], flags: flags ?? origMessage.flags, editTimestamp: editTimestamp ?? 1234567890, // TODO generate timestamp - streamId: origMessage is StreamMessage ? origMessage.streamId : null, + origStreamId: origMessage is StreamMessage ? origMessage.streamId : null, newStreamId: null, propagateMode: null, origTopic: null, - topic: null, + newTopic: null, origContent: 'some probably-mismatched old Markdown', origRenderedContent: origMessage.content, content: 'some probably-mismatched new Markdown',