From 450d476d6474bab4e5d63644f90ec5422d7a158e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 25 Mar 2025 16:19:57 -0400 Subject: [PATCH 1/5] api [nfc]: Use JsonSerializable with MessageEvent It will make it easier to add additional fields on the event later. --- lib/api/model/events.dart | 18 ++++++++---------- lib/api/model/events.g.dart | 10 ++++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 985f5b5803..53ea879330 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -665,8 +665,7 @@ class UserTopicEvent extends Event { } /// A Zulip event of type `message`: https://zulip.com/api/get-events#message -// TODO use [JsonSerializable] here too, using its customization features, -// in order to skip the boilerplate in [fromJson] and [toJson]. +@JsonSerializable(fieldRename: FieldRename.snake) class MessageEvent extends Event { @override @JsonKey(includeToJson: true) @@ -680,24 +679,23 @@ class MessageEvent extends Event { // events and in the get-messages results is that `matchContent` and // `matchTopic` are absent here. Already [Message.matchContent] and // [Message.matchTopic] are optional, so no action is needed on that. + @JsonKey(readValue: _readMessageValue, includeToJson: false) final Message message; MessageEvent({required super.id, required this.message}); - factory MessageEvent.fromJson(Map json) => MessageEvent( - id: json['id'] as int, - message: Message.fromJson({ - ...json['message'] as Map, - 'flags': (json['flags'] as List).map((e) => e as String).toList(), - }), - ); + static Map _readMessageValue(Map json, String key) => + {...json['message'] as Map, 'flags': json['flags']}; + + factory MessageEvent.fromJson(Map json) => + _$MessageEventFromJson(json); @override Map toJson() { final messageJson = message.toJson(); final flags = messageJson['flags']; messageJson.remove('flags'); - return {'id': id, 'type': type, 'message': messageJson, 'flags': flags}; + return {..._$MessageEventToJson(this), 'message': messageJson, 'flags': flags}; } } diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index c5b56d013a..1a69c027b4 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -428,6 +428,16 @@ const _$UserTopicVisibilityPolicyEnumMap = { UserTopicVisibilityPolicy.unknown: null, }; +MessageEvent _$MessageEventFromJson(Map json) => MessageEvent( + id: (json['id'] as num).toInt(), + message: Message.fromJson( + MessageEvent._readMessageValue(json, 'message') as Map, + ), +); + +Map _$MessageEventToJson(MessageEvent instance) => + {'id': instance.id, 'type': instance.type}; + UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => UpdateMessageEvent( id: (json['id'] as num).toInt(), From 1c077cc4c691bc6fca420a7ed1242eef0c47cd88 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 2 Apr 2025 16:26:47 -0400 Subject: [PATCH 2/5] test [nfc]: Switch from store.handleEvent to store.addMessage(s) This leaves some tests that handle MessageEvent untouched, as they conceptually aren't just setting up the store, and the helper adds a layer of indirection (or they simply don't have access to a PerAccountStore). --- test/model/message_list_test.dart | 21 +++++++------------ test/model/message_test.dart | 4 +--- test/widgets/home_test.dart | 5 ++--- test/widgets/inbox_test.dart | 2 +- test/widgets/poll_test.dart | 3 +-- .../widgets/recent_dm_conversations_test.dart | 4 +--- 6 files changed, 14 insertions(+), 25 deletions(-) diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 92c08ddd6b..f41b2be59a 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -367,8 +367,7 @@ void main() { List.generate(30, (i) => eg.streamMessage(stream: stream))); check(model).messages.length.equals(30); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(stream: stream))); + await store.addMessage(eg.streamMessage(stream: stream)); checkNotifiedOnce(); check(model).messages.length.equals(31); }); @@ -381,8 +380,7 @@ void main() { check(model).messages.length.equals(30); final otherStream = eg.stream(); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(stream: otherStream))); + await store.addMessage(eg.streamMessage(stream: otherStream)); checkNotNotified(); check(model).messages.length.equals(30); }); @@ -390,8 +388,7 @@ void main() { test('MessageEvent, before fetch', () async { final stream = eg.stream(); await prepare(narrow: ChannelNarrow(stream.streamId)); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(stream: stream))); + await store.addMessage(eg.streamMessage(stream: stream)); checkNotNotified(); check(model).fetched.isFalse(); }); @@ -1314,7 +1311,7 @@ void main() { } final message = eg.streamMessage(stream: stream, topic: 'hello'); - await store.handleEvent(MessageEvent(id: 0, message: message)); + await store.addMessage(message); await store.handleEvent( eg.reactionEvent(eg.unicodeEmojiReaction, ReactionOp.add, message.id)); @@ -1396,8 +1393,7 @@ void main() { await prepare(narrow: ChannelNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: List.generate(30, (i) => eg.streamMessage(stream: stream))); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(stream: stream))); + await store.addMessage(eg.streamMessage(stream: stream)); checkNotifiedOnce(); check(model).messages.length.equals(31); @@ -1644,8 +1640,7 @@ void main() { await prepare(narrow: ChannelNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: []); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(stream: stream))); + await store.addMessage(eg.streamMessage(stream: stream)); // Each [checkNotifiedOnce] call ensures there's been a [checkInvariants] // call, where the [ContentNode] gets checked. The additional checks to // make this test explicit. @@ -1659,13 +1654,13 @@ void main() { await prepare(narrow: ChannelNarrow(stream.streamId)); await prepareMessages(foundOldest: true, messages: []); - await store.handleEvent(MessageEvent(id: 0, message: eg.streamMessage( + await store.addMessage(eg.streamMessage( stream: stream, sender: eg.selfUser, submessages: [ eg.submessage(senderId: eg.selfUser.userId, content: eg.pollWidgetData(question: 'question', options: ['A'])), - ]))); + ])); // Each [checkNotifiedOnce] call ensures there's been a [checkInvariants] // call, where the value of the [Poll] gets checked. The additional // checks make this test explicit. diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 9c9a940d42..7b4e52d9a2 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -71,9 +71,7 @@ void main() { } Future addMessages(Iterable messages) async { - for (final m in messages) { - await store.handleEvent(MessageEvent(id: 0, message: m)); - } + await store.addMessages(messages); checkNotified(count: messageList.fetched ? messages.length : 0); } diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index e940d2ce77..3c8db1dfcf 100644 --- a/test/widgets/home_test.dart +++ b/test/widgets/home_test.dart @@ -2,7 +2,6 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/api/model/events.dart'; import 'package:zulip/model/actions.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; @@ -72,8 +71,8 @@ void main () { testWidgets('preserve states when switching between views', (tester) async { await prepare(tester); await store.addUser(eg.otherUser); - await store.handleEvent(MessageEvent( - id: 0, message: eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]))); + await store.addMessage( + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); await tester.pump(); check(find.byIcon(ZulipIcons.arrow_down)).findsExactly(2); diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index 7b2be9176e..9fa5de1bf3 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -70,7 +70,7 @@ void main() { for (final message in unreadMessages) { assert(!message.flags.contains(MessageFlag.read)); - await store.handleEvent(MessageEvent(id: 1, message: message)); + await store.addMessage(message); } await tester.pumpWidget(TestZulipApp( diff --git a/test/widgets/poll_test.dart b/test/widgets/poll_test.dart index bd6ae3f92f..a6bd74c77e 100644 --- a/test/widgets/poll_test.dart +++ b/test/widgets/poll_test.dart @@ -5,7 +5,6 @@ import 'package:http/http.dart' as http; import 'package:flutter/widgets.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/submessage.dart'; import 'package:zulip/model/store.dart'; @@ -40,7 +39,7 @@ void main() { message = eg.streamMessage( sender: eg.selfUser, submessages: [eg.submessage(content: submessageContent)]); - await store.handleEvent(MessageEvent(id: 0, message: message)); + await store.addMessage(message); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, child: PollWidget(messageId: message.id, poll: message.poll!))); await tester.pump(); diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 0d440accec..44322ccea1 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -38,9 +38,7 @@ Future setupPage(WidgetTester tester, { await store.addUser(user); } - for (final dmMessage in dmMessages) { - await store.handleEvent(MessageEvent(id: 1, message: dmMessage)); - } + await store.addMessages(dmMessages); if (newNameForSelfUser != null) { await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.selfUser.userId, From e9eabdc02c8d488ab57b901ed36e790489426d49 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 25 Mar 2025 16:54:31 -0400 Subject: [PATCH 3/5] test [nfc]: Add messageEvent MessageEvent constructor calls are generally quite short, and there are no boring required fields other than an integer id. However, as we add localMessageId to the event, it would be expected to have a boring value most of the time, so prepare for that. A neat side-effect is that we can cut some imports of lib/api/model/. (Some places that use non-zero event IDs now use 0, but we don't care much about this value anyway.) --- test/example_data.dart | 3 ++ test/model/message_list_test.dart | 44 ++++++++++---------- test/model/message_test.dart | 10 ++--- test/model/recent_dm_conversations_test.dart | 17 ++++---- test/model/test_store.dart | 2 +- test/model/unreads_test.dart | 10 ++--- test/widgets/message_list_test.dart | 2 +- 7 files changed, 45 insertions(+), 43 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index af1be189d4..89f16c63e7 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -622,6 +622,9 @@ UserTopicEvent userTopicEvent( ); } +MessageEvent messageEvent(Message message) => + MessageEvent(id: 0, message: message); + DeleteMessageEvent deleteMessageEvent(List messages) { assert(messages.isNotEmpty); final streamId = messages.first.streamId; diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index f41b2be59a..6f00d83dc6 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -1450,28 +1450,28 @@ void main() { .deepEquals(expected..insertAll(0, [101, 103, 105])); // … and on MessageEvent. - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 301, stream: stream1, topic: 'A'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 301, stream: stream1, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 302, stream: stream1, topic: 'B'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 302, stream: stream1, topic: 'B'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 303, stream: stream2, topic: 'C'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 303, stream: stream2, topic: 'C'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(303)); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 304, stream: stream2, topic: 'D'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 304, stream: stream2, topic: 'D'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); - await store.handleEvent(MessageEvent(id: 0, - message: eg.dmMessage(id: 305, from: eg.otherUser, to: [eg.selfUser]))); + await store.handleEvent(eg.messageEvent( + eg.dmMessage(id: 305, from: eg.otherUser, to: [eg.selfUser]))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(305)); }); @@ -1507,18 +1507,18 @@ void main() { .deepEquals(expected..insertAll(0, [101, 102])); // … and on MessageEvent. - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 301, stream: stream, topic: 'A'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 301, stream: stream, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 302, stream: stream, topic: 'B'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 302, stream: stream, topic: 'B'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(302)); - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 303, stream: stream, topic: 'C'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 303, stream: stream, topic: 'C'))); checkNotNotified(); check(model.messages.map((m) => m.id)).deepEquals(expected); }); @@ -1549,8 +1549,8 @@ void main() { .deepEquals(expected..insertAll(0, [101])); // … and on MessageEvent. - await store.handleEvent(MessageEvent(id: 0, - message: eg.streamMessage(id: 301, stream: stream, topic: 'A'))); + await store.handleEvent(eg.messageEvent( + eg.streamMessage(id: 301, stream: stream, topic: 'A'))); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301)); }); @@ -1589,7 +1589,7 @@ void main() { // … and on MessageEvent. final messages = getMessages(301); for (var i = 0; i < 3; i += 1) { - await store.handleEvent(MessageEvent(id: 0, message: messages[i])); + await store.handleEvent(eg.messageEvent(messages[i])); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301 + i)); } @@ -1627,7 +1627,7 @@ void main() { // … and on MessageEvent. final messages = getMessages(301); for (var i = 0; i < 2; i += 1) { - await store.handleEvent(MessageEvent(id: 0, message: messages[i])); + await store.handleEvent(eg.messageEvent(messages[i])); checkNotifiedOnce(); check(model.messages.map((m) => m.id)).deepEquals(expected..add(301 + i)); } @@ -1718,11 +1718,11 @@ void main() { checkNotified(count: 2); // Then test MessageEvent, where a new header is needed… - await store.handleEvent(MessageEvent(id: 0, message: streamMessage(13))); + await store.handleEvent(eg.messageEvent(streamMessage(13))); checkNotifiedOnce(); // … and where it's not. - await store.handleEvent(MessageEvent(id: 0, message: streamMessage(14))); + await store.handleEvent(eg.messageEvent(streamMessage(14))); checkNotifiedOnce(); // Then test UpdateMessageEvent edits, where a header is and remains needed… diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 7b4e52d9a2..5b4457f30b 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -129,7 +129,7 @@ void main() { check(store.messages).isEmpty(); final newMessage = eg.streamMessage(); - await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + await store.handleEvent(eg.messageEvent(newMessage)); check(store.messages).deepEquals({ newMessage.id: newMessage, }); @@ -148,7 +148,7 @@ void main() { }); final newMessage = eg.streamMessage(); - await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + await store.handleEvent(eg.messageEvent(newMessage)); check(store.messages).deepEquals({ for (final m in messages) m.id: m, newMessage.id: newMessage, @@ -162,7 +162,7 @@ void main() { check(store.messages).deepEquals({1: message}); final newMessage = eg.streamMessage(id: 1, content: '

bar

'); - await store.handleEvent(MessageEvent(id: 1, message: newMessage)); + await store.handleEvent(eg.messageEvent(newMessage)); check(store.messages).deepEquals({1: newMessage}); }); }); @@ -859,7 +859,7 @@ void main() { ]); await prepare(); - await store.handleEvent(MessageEvent(id: 0, message: message)); + await store.handleEvent(eg.messageEvent(message)); } test('smoke', () async { @@ -930,7 +930,7 @@ void main() { ), ]); await prepare(); - await store.handleEvent(MessageEvent(id: 0, message: message)); + await store.handleEvent(eg.messageEvent(message)); check(store.messages[message.id]).isNotNull().poll.isNull(); }); }); diff --git a/test/model/recent_dm_conversations_test.dart b/test/model/recent_dm_conversations_test.dart index f66b38c633..da487304e6 100644 --- a/test/model/recent_dm_conversations_test.dart +++ b/test/model/recent_dm_conversations_test.dart @@ -1,6 +1,5 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; -import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/recent_dm_conversations.dart'; @@ -66,7 +65,7 @@ void main() { final expected = setupView(); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: eg.streamMessage())) + ..handleMessageEvent(eg.messageEvent(eg.streamMessage())) ) ..map.deepEquals(expected.map) ..sorted.deepEquals(expected.sorted) ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); @@ -78,7 +77,7 @@ void main() { final message = eg.dmMessage(id: 300, from: eg.selfUser, to: [eg.user(userId: 2)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([2]): 300, key([1]): 200, @@ -94,7 +93,7 @@ void main() { final message = eg.dmMessage(id: 150, from: eg.selfUser, to: [eg.user(userId: 2)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([1]): 200, key([2]): 150, @@ -111,7 +110,7 @@ void main() { to: [eg.user(userId: 1), eg.user(userId: 2)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([1, 2]): 300, key([1]): 200, @@ -126,7 +125,7 @@ void main() { final message = eg.dmMessage(id: 300, from: eg.selfUser, to: [eg.user(userId: 1)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([1]): 300, key([1, 2]): 100, @@ -143,7 +142,7 @@ void main() { final expected = setupView(); check(setupView() // ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals(expected.map) ..sorted.deepEquals(expected.sorted) ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); @@ -157,7 +156,7 @@ void main() { to: [eg.user(userId: 1), eg.user(userId: 3)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([1, 3]): 300, key([1]): 200, @@ -174,7 +173,7 @@ void main() { to: [eg.user(userId: 1), eg.user(userId: 3)]); check(setupView() ..addListener(() { listenersNotified = true; }) - ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ..handleMessageEvent(eg.messageEvent(message)) ) ..map.deepEquals({ key([1]): 200, key([1, 3]): 150, diff --git a/test/model/test_store.dart b/test/model/test_store.dart index 6f13845cf5..0196611e1d 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -288,7 +288,7 @@ extension PerAccountStoreTestExtension on PerAccountStore { } Future addMessage(Message message) async { - await handleEvent(MessageEvent(id: 1, message: message)); + await handleEvent(eg.messageEvent(message)); } Future addMessages(Iterable messages) async { diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 9a2e899ba7..e8f7a0f850 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -49,7 +49,7 @@ void main() { void fillWithMessages(Iterable messages) { for (final message in messages) { - model.handleMessageEvent(MessageEvent(id: 0, message: message)); + model.handleMessageEvent(eg.messageEvent(message)); } notifiedCount = 0; } @@ -318,7 +318,7 @@ void main() { final message = isStream ? eg.streamMessage(flags: flags) : eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: flags); - model.handleMessageEvent(MessageEvent(id: 0, message: message)); + model.handleMessageEvent(eg.messageEvent(message)); if (isUnread) { checkNotifiedOnce(); } @@ -346,7 +346,7 @@ void main() { prepare(); fillWithMessages([oldMessage]); - model.handleMessageEvent(MessageEvent(id: 0, message: newMessage)); + model.handleMessageEvent(eg.messageEvent(newMessage)); checkNotifiedOnce(); checkMatchesMessages([oldMessage, newMessage]); }); @@ -369,7 +369,7 @@ void main() { final message = eg.dmMessage(from: from, to: to, flags: []); prepare(); - model.handleMessageEvent(MessageEvent(id: 0, message: message)); + model.handleMessageEvent(eg.messageEvent(message)); checkNotifiedOnce(); checkMatchesMessages([message]); }); @@ -387,7 +387,7 @@ void main() { test('existing in $oldDesc narrow; new in ${oldNarrow == newNarrow ? 'same narrow' : 'different narrow ($newDesc)'}', () { prepare(); fillWithMessages([oldMessage]); - model.handleMessageEvent(MessageEvent(id: 0, message: newMessage)); + model.handleMessageEvent(eg.messageEvent(newMessage)); checkNotifiedOnce(); checkMatchesMessages([oldMessage, newMessage]); }); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index b20aaefca9..53ad1334ef 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -1481,7 +1481,7 @@ void main() { // introduce new message final newMessage = eg.streamMessage(flags:[MessageFlag.read]); - await store.handleEvent(MessageEvent(id: 0, message: newMessage)); + await store.handleEvent(eg.messageEvent(newMessage)); await tester.pump(); // process handleEvent check(find.byType(MessageItem).evaluate()).length.equals(2); check(getAnimation(tester, message.id)) From c982795ff2501a208057580bfa85bc4bcc4fad1e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 25 Mar 2025 20:52:07 -0400 Subject: [PATCH 4/5] api: Use RawParameter when passing queue_id and local_id This endpoint in particular expects queue_id and local_id each to be a String just passed as itself, not JSON-encoded. --- lib/api/route/messages.dart | 4 ++-- test/api/route/messages_test.dart | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart index 5af312ce45..dcd7998ee8 100644 --- a/lib/api/route/messages.dart +++ b/lib/api/route/messages.dart @@ -193,8 +193,8 @@ Future sendMessage( 'to': destination.userIds, }}), 'content': RawParameter(content), - if (queueId != null) 'queue_id': queueId, // TODO should this use RawParameter? - if (localId != null) 'local_id': localId, // TODO should this use RawParameter? + if (queueId != null) 'queue_id': RawParameter(queueId), + if (localId != null) 'local_id': RawParameter(localId), if (readBySender != null) 'read_by_sender': readBySender, }, overrideUserAgent: switch ((supportsReadBySender, readBySender)) { diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index a29a87e24b..416fca4f3b 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -362,8 +362,8 @@ void main() { 'to': streamId.toString(), 'topic': topic, 'content': content, - 'queue_id': '"abc:123"', - 'local_id': '"456"', + 'queue_id': 'abc:123', + 'local_id': '456', 'read_by_sender': 'true', }); }); From d224a5ba368787bc3aac1432140c305ab574b64e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 25 Mar 2025 21:27:56 -0400 Subject: [PATCH 5/5] api: Add localMessageId to MessageEvent This will be used for local echoing. local_message_id on message event is not yet fully documented; see CZO discussion: https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/local_id.2C.20queue_id.2Fsender_queue_id/near/2135340 --- lib/api/model/events.dart | 9 ++++++++- lib/api/model/events.g.dart | 7 ++++++- test/api/model/events_checks.dart | 1 + test/example_data.dart | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index 53ea879330..df15295894 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -682,7 +682,14 @@ class MessageEvent extends Event { @JsonKey(readValue: _readMessageValue, includeToJson: false) final Message message; - MessageEvent({required super.id, required this.message}); + // When present, this equals the "local_id" parameter + // from a previous [sendMessage] call by us. + // + // This is not yet fully documented. See CZO discussion for reference: + // https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/local_id.2C.20queue_id.2Fsender_queue_id/near/2135340 + final String? localMessageId; + + MessageEvent({required super.id, required this.message, required this.localMessageId}); static Map _readMessageValue(Map json, String key) => {...json['message'] as Map, 'flags': json['flags']}; diff --git a/lib/api/model/events.g.dart b/lib/api/model/events.g.dart index 1a69c027b4..35206d77b9 100644 --- a/lib/api/model/events.g.dart +++ b/lib/api/model/events.g.dart @@ -433,10 +433,15 @@ MessageEvent _$MessageEventFromJson(Map json) => MessageEvent( message: Message.fromJson( MessageEvent._readMessageValue(json, 'message') as Map, ), + localMessageId: json['local_message_id'] as String?, ); Map _$MessageEventToJson(MessageEvent instance) => - {'id': instance.id, 'type': instance.type}; + { + 'id': instance.id, + 'type': instance.type, + 'local_message_id': instance.localMessageId, + }; UpdateMessageEvent _$UpdateMessageEventFromJson(Map json) => UpdateMessageEvent( diff --git a/test/api/model/events_checks.dart b/test/api/model/events_checks.dart index 12d74479de..59d01d67ac 100644 --- a/test/api/model/events_checks.dart +++ b/test/api/model/events_checks.dart @@ -39,6 +39,7 @@ extension SubscriptionUpdateEventChecks on Subject { extension MessageEventChecks on Subject { Subject get message => has((e) => e.message, 'message'); + Subject get localMessageId => has((e) => e.localMessageId, 'localMessageId'); } extension UpdateMessageEventChecks on Subject { diff --git a/test/example_data.dart b/test/example_data.dart index 89f16c63e7..afa378aa7e 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -623,7 +623,7 @@ UserTopicEvent userTopicEvent( } MessageEvent messageEvent(Message message) => - MessageEvent(id: 0, message: message); + MessageEvent(id: 0, message: message, localMessageId: null); DeleteMessageEvent deleteMessageEvent(List messages) { assert(messages.isNotEmpty);