Skip to content

Commit c4ca1ab

Browse files
committed
message: Handle message deletion event.
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 #120. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 2cbd790 commit c4ca1ab

File tree

5 files changed

+145
-1
lines changed

5 files changed

+145
-1
lines changed

lib/model/message.dart

+6-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,12 @@ class MessageStoreImpl with MessageStore {
175175
}
176176

177177
void handleDeleteMessageEvent(DeleteMessageEvent event) {
178-
// TODO handle DeleteMessageEvent, particularly in MessageListView
178+
for (final messageId in event.messageIds) {
179+
messages.remove(messageId);
180+
}
181+
for (final view in _messageListViews) {
182+
view.handleDeleteMessageEvent(event);
183+
}
179184
}
180185

181186
void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {

lib/model/message_list.dart

+30
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,30 @@ mixin _MessageSequence {
155155
_processMessage(messages.length - 1);
156156
}
157157

158+
/// Removes the given messages, if present.
159+
///
160+
/// Returns true if at least one message was present, false otherwise.
161+
/// If none of [messageIds] are found, this is a no-op.
162+
bool _removeMessagesById(Iterable<int> messageIds) {
163+
final messagesToRemoveById = <int>{};
164+
final contentToRemove = Set<ZulipContent>.identity();
165+
for (final messageId in messageIds) {
166+
final index = _findMessageWithId(messageId);
167+
if (index == -1) continue;
168+
messagesToRemoveById.add(messageId);
169+
contentToRemove.add(contents[index]);
170+
}
171+
if (messagesToRemoveById.isEmpty) return false;
172+
173+
assert(contents.length == messages.length);
174+
messages.removeWhere((message) => messagesToRemoveById.contains(message.id));
175+
contents.removeWhere((content) => contentToRemove.contains(content));
176+
assert(contents.length == messages.length);
177+
_reprocessAll();
178+
179+
return true;
180+
}
181+
158182
void _insertAllMessages(int index, Iterable<Message> toInsert) {
159183
// TODO parse/process messages in smaller batches, to not drop frames.
160184
// On a Pixel 5, a batch of 100 messages takes ~15-20ms in _insertAllMessages.
@@ -429,6 +453,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
429453
}
430454
}
431455

456+
void handleDeleteMessageEvent(DeleteMessageEvent event) {
457+
if (_removeMessagesById(event.messageIds)) {
458+
notifyListeners();
459+
}
460+
}
461+
432462
/// Add [MessageEvent.message] to this view, if it belongs here.
433463
void handleMessageEvent(MessageEvent event) {
434464
final message = event.message;

test/example_data.dart

+15
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,21 @@ const _unreadMsgs = unreadMsgs;
388388
// Events.
389389
//
390390

391+
DeleteMessageEvent deleteMessageEvent(List<StreamMessage> messages) {
392+
assert(messages.isNotEmpty);
393+
final streamId = messages.first.streamId;
394+
final topic = messages.first.topic;
395+
assert(messages.every((m) => m.streamId == streamId));
396+
assert(messages.every((m) => m.topic == topic));
397+
return DeleteMessageEvent(
398+
id: 0,
399+
messageIds: messages.map((message) => message.id).toList(),
400+
messageType: MessageType.stream,
401+
streamId: messages[0].streamId,
402+
topic: messages[0].topic,
403+
);
404+
}
405+
391406
UpdateMessageEvent updateMessageEditEvent(
392407
Message origMessage, {
393408
int? userId = -1, // null means null; default is [selfUser.userId]

test/model/message_list_test.dart

+61
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,67 @@ void main() {
269269
check(model).fetched.isFalse();
270270
});
271271

272+
group('DeleteMessageEvent', () {
273+
final stream = eg.stream();
274+
final messages = List.generate(30, (i) => eg.streamMessage(stream: stream));
275+
276+
test('in narrow', () async {
277+
await prepare(narrow: StreamNarrow(stream.streamId));
278+
await prepareMessages(foundOldest: true, messages: messages);
279+
280+
check(model).messages.length.equals(30);
281+
await store.handleEvent(eg.deleteMessageEvent(messages.sublist(0, 10)));
282+
checkNotifiedOnce();
283+
check(model).messages.length.equals(20);
284+
});
285+
286+
test('not all in narrow', () async {
287+
await prepare(narrow: StreamNarrow(stream.streamId));
288+
await prepareMessages(foundOldest: true, messages: messages.sublist(5));
289+
290+
check(model).messages.length.equals(25);
291+
await store.handleEvent(eg.deleteMessageEvent(messages.sublist(0, 10)));
292+
checkNotifiedOnce();
293+
check(model).messages.length.equals(20);
294+
});
295+
296+
test('not in narrow', () async {
297+
await prepare(narrow: StreamNarrow(stream.streamId));
298+
await prepareMessages(foundOldest: true, messages: messages.sublist(5));
299+
300+
check(model).messages.length.equals(25);
301+
await store.handleEvent(eg.deleteMessageEvent(messages.sublist(0, 5)));
302+
checkNotNotified();
303+
check(model).messages.length.equals(25);
304+
});
305+
306+
test('complete message deletion', () async {
307+
await prepare(narrow: StreamNarrow(stream.streamId));
308+
await prepareMessages(foundOldest: true, messages: messages.sublist(0, 25));
309+
310+
check(model).messages.length.equals(25);
311+
await store.handleEvent(eg.deleteMessageEvent(messages));
312+
checkNotifiedOnce();
313+
check(model).messages.length.equals(0);
314+
});
315+
316+
test('non-consecutive message deletion', () async {
317+
await prepare(narrow: StreamNarrow(stream.streamId));
318+
await prepareMessages(foundOldest: true, messages: messages);
319+
final messagesToDelete = messages.sublist(2, 5) + messages.sublist(10, 15);
320+
321+
check(model).messages.length.equals(30);
322+
await store.handleEvent(eg.deleteMessageEvent(messagesToDelete));
323+
checkNotifiedOnce();
324+
final expected = [
325+
...messages.sublist(0, 2),
326+
...messages.sublist(5, 10),
327+
...messages.sublist(15)].map((message) => message.id);
328+
check(model).messages.length.equals(expected.length);
329+
check(model.messages.map((message) => message.id)).deepEquals(expected);
330+
});
331+
});
332+
272333
group('notifyListenersIfMessagePresent', () {
273334
test('message present', () async {
274335
await prepare(narrow: const CombinedFeedNarrow());

test/model/message_test.dart

+33
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,39 @@ void main() {
375375
});
376376
});
377377

378+
group('handleDeleteMessageEvent', () {
379+
test('delete an unknown message', () async {
380+
final message1 = eg.streamMessage();
381+
final message2 = eg.streamMessage();
382+
await prepare();
383+
await prepareMessages([message1]);
384+
await store.handleEvent(eg.deleteMessageEvent([message2]));
385+
checkNotNotified();
386+
check(store).messages.values.single.id.equals(message1.id);
387+
});
388+
389+
test('delete messages', () async {
390+
final message1 = eg.streamMessage();
391+
final message2 = eg.streamMessage();
392+
await prepare();
393+
await prepareMessages([message1, message2]);
394+
await store.handleEvent(eg.deleteMessageEvent([message1, message2]));
395+
checkNotifiedOnce();
396+
check(store).messages.isEmpty();
397+
});
398+
399+
test('delete an unknown message with a known message', () async {
400+
final message1 = eg.streamMessage();
401+
final message2 = eg.streamMessage();
402+
final message3 = eg.streamMessage();
403+
await prepare();
404+
await prepareMessages([message1, message2]);
405+
await store.handleEvent(eg.deleteMessageEvent([message2, message3]));
406+
checkNotifiedOnce();
407+
check(store).messages.values.single.id.equals(message1.id);
408+
});
409+
});
410+
378411
group('handleReactionEvent', () {
379412
test('add reaction', () async {
380413
final originalMessage = eg.streamMessage(reactions: []);

0 commit comments

Comments
 (0)