Skip to content

Commit

Permalink
message: Handle message deletion event.
Browse files Browse the repository at this point in the history
When handling a message deletion event, we just simply remove the
messages from the per account store and wipe them from the message list
view if they are present.

Because message deletion can affect what items are to be seen (date
separator, recipient header, etc.), if the view has any message deleted, a
`_reprocessAll` call is triggered. It would be ideal to just removed the
affected items for better performance.

Fixes zulip#120.

Signed-off-by: Zixuan James Li <[email protected]>
  • Loading branch information
PIG208 committed Jun 26, 2024
1 parent 0399af6 commit 9b86848
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/model/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,13 @@ class MessageStoreImpl with MessageStore {
}

void handleDeleteMessageEvent(DeleteMessageEvent event) {
// TODO handle DeleteMessageEvent, particularly in MessageListView
for (final messageId in event.messageIds) {
if (messages.containsKey(messageId)) messages.remove(messageId);
}
for (final view in _messageListViews) {
if (view.messages.isEmpty) continue;
view.messagesRemoved(event.messageIds);
}
}

void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {
Expand Down
27 changes: 27 additions & 0 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,27 @@ mixin _MessageSequence {
_processMessage(messages.length - 1);
}

/// Given message ids, remove the corresponding messages from the view, and
/// return true if at least one message has been removed, or false otherwise.
///
/// If none of the message ids provided can be found, this is essentially
/// a no-op.
bool _removeMessagesById(Iterable<int> messageIds) {
bool hasRemovedMessage = false;
for (final messageId in messageIds) {
final messageIndex = _findMessageWithId(messageId);
if (messageIndex == -1) continue;

assert(contents.length == messages.length);
messages.removeAt(messageIndex);
contents.removeAt(messageIndex);
assert(contents.length == messages.length);
hasRemovedMessage = true;
}
if (hasRemovedMessage) _reprocessAll();
return hasRemovedMessage;
}

void _insertAllMessages(int index, Iterable<Message> toInsert) {
// TODO parse/process messages in smaller batches, to not drop frames.
// On a Pixel 5, a batch of 100 messages takes ~15-20ms in _insertAllMessages.
Expand Down Expand Up @@ -478,6 +499,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
}
}

void messagesRemoved(List<int> messageIds) {
if (_removeMessagesById(messageIds)) {
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
Expand Down
10 changes: 10 additions & 0 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,16 @@ const _unreadMsgs = unreadMsgs;
// Events.
//

DeleteMessageEvent deleteMessageEvent(List<StreamMessage> messages) {
return DeleteMessageEvent(
id: 0,
messageIds: messages.map((message) => message.id).toList(),
messageType: MessageType.stream,
streamId: messages[0].streamId,
topic: messages[0].topic,
);
}

UpdateMessageEvent updateMessageEditEvent(
Message origMessage, {
int? userId = -1, // null means null; default is [selfUser.userId]
Expand Down
34 changes: 34 additions & 0 deletions test/model/message_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,40 @@ void main() {
});
});

group('handleDeleteMessageEvent', () {

test('delete unknown message', () async {
final message1 = eg.streamMessage();
final message2 = eg.streamMessage();
await prepare();
await prepareMessages([message1]);
await store.handleEvent(eg.deleteMessageEvent([message2]));
checkNotNotified();
check(store).messages.values.single.id.equals(message1.id);
});

test('delete messages', () async {
final message1 = eg.streamMessage();
final message2 = eg.streamMessage();
await prepare();
await prepareMessages([message1, message2]);
await store.handleEvent(eg.deleteMessageEvent([message1, message2]));
checkNotifiedOnce();
check(store).messages.isEmpty();
});

test('delete an unknown messages with a known messages', () async {
final message1 = eg.streamMessage();
final message2 = eg.streamMessage();
final message3 = eg.streamMessage();
await prepare();
await prepareMessages([message1, message2]);
await store.handleEvent(eg.deleteMessageEvent([message2, message3]));
checkNotifiedOnce();
check(store).messages.values.single.id.equals(message1.id);
});
});

group('handleReactionEvent', () {
test('add reaction', () async {
final originalMessage = eg.streamMessage(reactions: []);
Expand Down

0 comments on commit 9b86848

Please sign in to comment.