From 56292344f24bcaa574fb1217f96ea7fe6629eb8e Mon Sep 17 00:00:00 2001 From: Amy Dyer Date: Mon, 24 Jul 2023 11:18:10 -0400 Subject: [PATCH] msglist: Handle updated events in MessageListView (#118). Processes an UpdateMessageEvent and hands it off to the MessageListView to update, if the message is visible in the MessageListView. This completes the changes required for issue #118. --- lib/api/model/model.dart | 8 +- lib/model/message_list.dart | 63 +++++++++++++ lib/model/store.dart | 4 +- test/example_data.dart | 3 +- test/model/message_list_test.dart | 143 ++++++++++++++++++++++++++++++ 5 files changed, 215 insertions(+), 6 deletions(-) create mode 100644 test/model/message_list_test.dart diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 6dbdec8d13f..e7e81495016 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 952b00ebbd4..b90da25b953 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -1,5 +1,7 @@ +import 'package:collection/collection.dart'; 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 +88,67 @@ class MessageListView extends ChangeNotifier { notifyListeners(); } + applyChangesToMessage(UpdateMessageEvent event, Message message) { + if (event.renderingOnly != null && event.renderingOnly == true) { + //TODO update inline preview + return; + } + + if (!event.flags.equals(message.flags)) { + message.flags = event.flags; + } + + if (event.renderedContent != null) { + assert(message.contentType == 'text/html', "Expected message to have html contentType. Instead, got ${message.contentType}"); + message.content = event.renderedContent!; + } + + if (event.editTimestamp != null) { + message.lastEditTimestamp = event.editTimestamp; + } + + if (event.isMeMessage != null) { + message.isMeMessage = event.isMeMessage!; + } + + } + + ///This is almost directly copied from package:collection/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. + int findMessageWithId(int messageId) { + var min = 0; + var max = messages.length; + + while (min < max) { + var mid = min + ((max - min) >> 1); + Message 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; + } + + void maybeUpdateMessage(UpdateMessageEvent event) { + int idx = findMessageWithId(event.messageId); + + if (idx == -1) { + return; + } + + Message 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 d9de7200207..f5121f3da82 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/example_data.dart b/test/example_data.dart index da24aa274d2..14d3ef6a598 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, diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart new file mode 100644 index 00000000000..66ee78521b1 --- /dev/null +++ b/test/model/message_list_test.dart @@ -0,0 +1,143 @@ +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 '../model/binding.dart'; +import '../model/test_store.dart'; +import '../example_data.dart' as eg; + +const int userId = 1; +const int streamId = 2; + +Future setupStore(ZulipStream stream) async { + await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + PerAccountStore 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 { + MessageListView 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(); + const narrow = StreamNarrow(streamId); + + final ZulipStream stream = eg.stream(streamId: streamId); + PerAccountStore store = await setupStore(stream); + + group('update message tests', () { + + test('find message in message list returns index of message', () async { + Message m1 = eg.streamMessage(id: 792, stream: stream); + Message m2 = eg.streamMessage(id: 793, stream: stream); + Message m3 = eg.streamMessage(id: 794, stream: stream); + + MessageListView messageList = await messageListViewWithMessages([m1, m2, m3], store, narrow); + + int idx = messageList.findMessageWithId(793); + check(idx).equals(1); + + idx = messageList.findMessageWithId(999); + check(idx).equals(-1); + }); + + test('update events are correctly applied to message when it is in the stream', () async { + String oldContent = "

Hello, world

"; + String newContent = "

Hello, edited

"; + int newTimestamp = 99999; + + List oldFlags = []; + List newFlags = ["starred"]; + + Message m = eg.streamMessage(id: 243, stream: stream, content: oldContent, flags: oldFlags); + MessageListView messageList = await messageListViewWithMessages([m], store, narrow); + + UpdateMessageEvent updateEvent = UpdateMessageEvent( + id: 1, + messageId: m.id, + messageIds: [m.id], + flags: newFlags, + renderedContent: newContent, + editTimestamp: newTimestamp, + isMeMessage: true + ); + + Message oldMessage = messageList.messages[0]; + check(oldMessage.content).equals(oldContent); + check(oldMessage.flags).deepEquals(oldFlags); + check(oldMessage.isMeMessage).equals(false); + + bool listenersNotified = false; + + messageList.addListener(() { listenersNotified = true; }); + messageList.maybeUpdateMessage(updateEvent); + + Message updatedMessage = messageList.messages[0]; + + check(listenersNotified).equals(true); + check(updatedMessage.content).equals(newContent); + check(updatedMessage.lastEditTimestamp).equals(newTimestamp); + check(updatedMessage.flags).deepEquals(newFlags); + check(updatedMessage.isMeMessage).equals(true); + + }); + + test('update event is ignored when message is not in the message list', () async { + String oldContent = "

Hello, world

"; + String newContent = "

Hello, edited

"; + int newTimestamp = 99999; + + Message m = eg.streamMessage(id: 243, stream: stream, content: oldContent); + MessageListView messageList = await messageListViewWithMessages([m], store, narrow); + + UpdateMessageEvent updateEvent = UpdateMessageEvent( + id: 1, + messageId: 972, + messageIds: [972], + flags: m.flags, + renderedContent: newContent, + editTimestamp: newTimestamp, + ); + + Message oldMessage = messageList.messages[0]; + check(oldMessage.content).equals(oldContent); + + bool listenersNotified = false; + + messageList.addListener(() { listenersNotified = true; }); + messageList.maybeUpdateMessage(updateEvent); + + Message updatedMessage = messageList.messages[0]; + + check(listenersNotified).equals(false); + check(updatedMessage.content).equals(oldContent); + + }); + + }); +} \ No newline at end of file