diff --git a/lib/api/model/events.dart b/lib/api/model/events.dart index d49d2418a3..47aa24256b 100644 --- a/lib/api/model/events.dart +++ b/lib/api/model/events.dart @@ -55,7 +55,7 @@ class UnexpectedEvent extends Event { Map toJson() => json; } -/// A Zulip event of type `alert_words`. +/// A Zulip event of type `alert_words`: https://zulip.com/api/get-events#alert_words @JsonSerializable(fieldRename: FieldRename.snake) class AlertWordsEvent extends Event { @override @@ -74,6 +74,9 @@ class AlertWordsEvent extends Event { } /// A Zulip event of type `realm_user`. +/// +/// The corresponding API docs are in several places for +/// different values of `op`; see subclasses. abstract class RealmUserEvent extends Event { @override @JsonKey(includeToJson: true) @@ -122,6 +125,7 @@ class RealmUserRemoveEvent extends RealmUserEvent { } } +/// As in [RealmUserUpdateEvent.customProfileField]. @JsonSerializable(fieldRename: FieldRename.snake) class RealmUserUpdateCustomProfileField { final int id; @@ -184,6 +188,9 @@ class RealmUserUpdateEvent extends RealmUserEvent { } /// A Zulip event of type `stream`. +/// +/// The corresponding API docs are in several places for +/// different values of `op`; see subclasses. abstract class StreamEvent extends Event { @override @JsonKey(includeToJson: true) @@ -231,7 +238,7 @@ class StreamDeleteEvent extends StreamEvent { // TODO(#182) StreamUpdateEvent, for a [StreamEvent] with op `update`: // https://zulip.com/api/get-events#stream-update -/// A Zulip event of type `message`. +/// 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]. class MessageEvent extends Event { @@ -267,7 +274,7 @@ class MessageEvent extends Event { } } -/// A Zulip event of type `update_message`. +/// A Zulip event of type `update_message`: https://zulip.com/api/get-events#update_message @JsonSerializable(fieldRename: FieldRename.snake) class UpdateMessageEvent extends Event { @override @@ -329,7 +336,7 @@ enum PropagateMode { changeAll; } -/// A Zulip event of type `delete_message`. +/// A Zulip event of type `delete_message`: https://zulip.com/api/get-events#delete_message @JsonSerializable(fieldRename: FieldRename.snake) class DeleteMessageEvent extends Event { @override @@ -363,6 +370,7 @@ enum MessageType { private; } +/// A Zulip event of type `heartbeat`: https://zulip.com/api/get-events#heartbeat @JsonSerializable(fieldRename: FieldRename.snake) class HeartbeatEvent extends Event { @override diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 6dbdec8d13..e7e8149501 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -250,13 +250,13 @@ class Subscription { sealed class Message { final String? avatarUrl; final String client; - final String content; + String content; final String contentType; // final List editHistory; // TODO handle final int id; - final bool isMeMessage; - final int? lastEditTimestamp; + bool isMeMessage; + int? lastEditTimestamp; // final List reactions; // TODO handle final int recipientId; @@ -271,7 +271,7 @@ sealed class Message { // final List topicLinks; // TODO handle // final string type; // handled by runtime type of object - final List flags; // TODO enum + List flags; // TODO enum final String? matchContent; final String? matchSubject; diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 952b00ebbd..26d01017c2 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -1,5 +1,6 @@ import 'package:flutter/foundation.dart'; +import '../api/model/events.dart'; import '../api/model/model.dart'; import '../api/route/messages.dart'; import 'content.dart'; @@ -86,6 +87,70 @@ class MessageListView extends ChangeNotifier { notifyListeners(); } + _applyChangesToMessage(UpdateMessageEvent event, Message message) { + // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114 + final isRenderingOnly = event.renderingOnly ?? (event.userId == null); + if (event.editTimestamp != null && !isRenderingOnly) { + // A rendering-only update gets omitted from the message edit history, + // and [Message.lastEditTimestamp] is the last timestamp of that history. + // So on a rendering-only update, the timestamp doesn't get updated. + message.lastEditTimestamp = event.editTimestamp; + } + + message.flags = event.flags; + + if (event.renderedContent != null) { + assert(message.contentType == 'text/html', + "Message contentType was ${message.contentType}; expected text/html."); + message.content = event.renderedContent!; + } + + if (event.isMeMessage != null) { + message.isMeMessage = event.isMeMessage!; + } + } + + // This is almost directly copied from package:collection/src/algorithms.dart + // The way that package was set up doesn't allow us to search + // for a message ID among a bunch of message objects - this is a quick + // modification of that method to work here for us. + @visibleForTesting + int findMessageWithId(int messageId) { + var min = 0; + var max = messages.length; + while (min < max) { + var mid = min + ((max - min) >> 1); + final message = messages[mid]; + var comp = message.id.compareTo(messageId); + if (comp == 0) return mid; + if (comp < 0) { + min = mid + 1; + } else { + max = mid; + } + } + return -1; + } + + /// Update the message the given event applies to, if present in this view. + /// + /// This method only handles the case where the message's contents + /// were changed, and ignores any changes to its stream or topic. + /// + /// TODO(#150): Handle message moves. + void maybeUpdateMessage(UpdateMessageEvent event) { + final idx = findMessageWithId(event.messageId); + if (idx == -1) { + return; + } + + final message = messages[idx]; + _applyChangesToMessage(event, message); + + contents[idx] = parseContent(message.content); + notifyListeners(); + } + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// This will redo from scratch any computations we can, such as parsing diff --git a/lib/model/store.dart b/lib/model/store.dart index d9de720020..f5121f3da8 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -272,7 +272,9 @@ class PerAccountStore extends ChangeNotifier { } } else if (event is UpdateMessageEvent) { assert(debugLog("server event: update_message ${event.messageId}")); - // TODO handle + for (final view in _messageListViews) { + view.maybeUpdateMessage(event); + } } else if (event is DeleteMessageEvent) { assert(debugLog("server event: delete_message ${event.messageIds}")); // TODO handle diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index 1b41954791..a049df975a 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -8,6 +8,9 @@ extension MessageChecks on Subject { toJson.deepEquals(expected.toJson()); } + Subject get content => has((e) => e.content, 'content'); + Subject get isMeMessage => has((e) => e.isMeMessage, 'isMeMessage'); + Subject get lastEditTimestamp => has((e) => e.lastEditTimestamp, 'lastEditTimestamp'); Subject> get flags => has((e) => e.flags, 'flags'); // TODO accessors for other fields diff --git a/test/example_data.dart b/test/example_data.dart index da24aa274d..0e0e87171e 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -127,6 +127,7 @@ StreamMessage streamMessage({ String? topic, String? content, String? contentMarkdown, + List? flags, }) { final effectiveStream = stream ?? _stream(); // The use of JSON here is convenient in order to delegate parts of the data @@ -140,7 +141,7 @@ StreamMessage streamMessage({ ..._messagePropertiesFromContent(content, contentMarkdown), 'display_recipient': effectiveStream.name, 'stream_id': effectiveStream.streamId, - 'flags': [], + 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'subject': topic ?? 'example topic', 'timestamp': 1678139636, @@ -158,6 +159,7 @@ DmMessage dmMessage({ required List to, String? content, String? contentMarkdown, + List? flags, }) { assert(!to.any((user) => user.userId == from.userId)); return DmMessage.fromJson({ @@ -168,7 +170,7 @@ DmMessage dmMessage({ .map((u) => {'id': u.userId, 'email': u.email, 'full_name': u.fullName}) .toList(growable: false), - 'flags': [], + 'flags': flags ?? [], 'id': id ?? 1234567, // TODO generate example IDs 'subject': '', 'timestamp': 1678139636, diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart new file mode 100644 index 0000000000..beebc09c86 --- /dev/null +++ b/test/model/message_list_test.dart @@ -0,0 +1,225 @@ +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/events.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/model/message_list.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/store.dart'; + +import '../api/fake_api.dart'; +import '../api/model/model_checks.dart'; +import '../model/binding.dart'; +import '../model/test_store.dart'; +import '../example_data.dart' as eg; + +const int userId = 1; + +Future setupStore(ZulipStream stream) async { + addTearDown(TestZulipBinding.instance.reset); + + await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + + final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id); + store.addUser(eg.user(userId: userId)); + store.addStream(stream); + + return store; +} + +Future messageListViewWithMessages(List messages, PerAccountStore store, Narrow narrow) async { + final messageList = MessageListView.init(store: store, narrow: narrow); + + final connection = store.connection as FakeApiConnection; + + connection.prepare(json: GetMessagesResult( + anchor: messages.first.id, + foundNewest: true, + foundOldest: true, + foundAnchor: true, + historyLimited: false, + messages: messages, + ).toJson()); + + await messageList.fetch(); + + check(messageList.messages.length).equals(messages.length); + + return messageList; +} + +void main() async { + TestZulipBinding.ensureInitialized(); + + final stream = eg.stream(); + final narrow = StreamNarrow(stream.streamId); + + group('update message tests', () { + + test('find message in message list returns index of message', () async { + final store = await setupStore(stream); + + final m1 = eg.streamMessage(id: 2, stream: stream); + final m2 = eg.streamMessage(id: 4, stream: stream); + final m3 = eg.streamMessage(id: 6, stream: stream); + + final messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow); + // The implementation of this uses a binary search, so let's test it + // a bit more exhaustively. + + check(messageList.findMessageWithId(1)).equals(-1); + check(messageList.findMessageWithId(2)).equals(0); + check(messageList.findMessageWithId(3)).equals(-1); + check(messageList.findMessageWithId(4)).equals(1); + check(messageList.findMessageWithId(5)).equals(-1); + check(messageList.findMessageWithId(6)).equals(2); + check(messageList.findMessageWithId(7)).equals(-1); + + // Invalid IDs + check(messageList.findMessageWithId(-8409)).equals(-1); + check(messageList.findMessageWithId(0)).equals(-1); + }); + + test('update events are correctly applied to message when it is in the stream', () async { + final store = await setupStore(stream); + + const oldContent = "

Hello, world

"; + const newContent = "

Hello, edited

"; + const newTimestamp = 99999; + + List oldFlags = []; + List newFlags = ["starred"]; + + final originalMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent, flags: oldFlags); + final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + + final updateEvent = UpdateMessageEvent( + id: 1, + messageId: originalMessage.id, + messageIds: [originalMessage.id], + flags: newFlags, + renderedContent: newContent, + editTimestamp: newTimestamp, + isMeMessage: true, + userId: userId, + renderingOnly: false, + ); + + final message = messageList.messages.single; + check(message) + ..content.equals(oldContent) + ..flags.deepEquals(oldFlags) + ..isMeMessage.isFalse(); + + var listenersNotified = false; + + messageList.addListener(() { listenersNotified = true; }); + messageList.maybeUpdateMessage(updateEvent); + + check(listenersNotified).isTrue(); + + check(message) + ..identicalTo(messageList.messages.single) + ..content.equals(newContent) + ..lastEditTimestamp.equals(newTimestamp) + ..flags.equals(newFlags) + ..isMeMessage.isTrue(); + }); + + test('update event is ignored when message is not in the message list', () async { + final store = await setupStore(stream); + + const oldContent = "

Hello, world

"; + const newContent = "

Hello, edited

"; + const newTimestamp = 99999; + + final originalMessage = eg.streamMessage(id: 243, stream: stream, content: oldContent); + final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + + final updateEvent = UpdateMessageEvent( + id: 1, + messageId: originalMessage.id + 1, + messageIds: [originalMessage.id + 1], + flags: originalMessage.flags, + renderedContent: newContent, + editTimestamp: newTimestamp, + userId: userId, + renderingOnly: false, + ); + + final message = messageList.messages.single; + check(message).content.equals(oldContent); + + var listenersNotified = false; + + messageList.addListener(() { listenersNotified = true; }); + messageList.maybeUpdateMessage(updateEvent); + + check(listenersNotified).isFalse(); + check(message).content.equals(oldContent); + }); + + test('rendering-only update does not change timestamp', () async { + final store = await setupStore(stream); + + const oldContent = "

Hello, world

"; + const oldTimestamp = 78492; + const newContent = "

Hello, world

Some link preview
"; + const newTimestamp = 99999; + + final originalMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent); + originalMessage.lastEditTimestamp = oldTimestamp; + + final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + + final updateEvent = UpdateMessageEvent( + id: 1, + messageId: originalMessage.id, + messageIds: [originalMessage.id], + flags: originalMessage.flags, + renderedContent: newContent, + editTimestamp: newTimestamp, + renderingOnly: true, + userId: null, + ); + + final message = messageList.messages[0]; + messageList.maybeUpdateMessage(updateEvent); + check(message) + ..content.equals(newContent) + ..lastEditTimestamp.equals(oldTimestamp); + }); + + // TODO(server-5): Cut this test; rely on renderingOnly from FL 114 + test('rendering-only update does not change timestamp (for old server versions)', () async { + final store = await setupStore(stream); + + const oldContent = "

Hello, world

"; + const oldTimestamp = 78492; + const newContent = "

Hello, world

Some link preview
"; + const newTimestamp = 99999; + + final originalMessage = eg.streamMessage(id: 972, stream: stream, content: oldContent); + originalMessage.lastEditTimestamp = oldTimestamp; + + final messageList = await messageListViewWithMessages([originalMessage], store, narrow); + + final updateEvent = UpdateMessageEvent( + id: 1, + messageId: originalMessage.id, + messageIds: [originalMessage.id], + flags: originalMessage.flags, + renderedContent: newContent, + editTimestamp: newTimestamp, + renderingOnly: null, + userId: null, + ); + + final message = messageList.messages.single; + messageList.maybeUpdateMessage(updateEvent); + check(message) + ..content.equals(newContent) + ..lastEditTimestamp.equals(oldTimestamp); + }); + }); +}