Skip to content

Commit 5369c36

Browse files
PIG208PIG208
committed
unreads: Handle updates when there are moved messages
Signed-off-by: Zixuan James Li <[email protected]>
1 parent fd42760 commit 5369c36

File tree

2 files changed

+210
-4
lines changed

2 files changed

+210
-4
lines changed

‎lib/model/unreads.dart

+27-4
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,6 @@ class Unreads extends ChangeNotifier {
260260
);
261261

262262
// We assume this event can't signal a change in a message's 'read' flag.
263-
// TODO can it actually though, when it's about messages being moved into an
264-
// unsubscribed stream?
265-
// https://chat.zulip.org/#narrow/stream/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/1639957
266263
final bool isRead = event.flags.contains(MessageFlag.read);
267264
assert(() {
268265
final isUnreadLocally = isUnread(messageId);
@@ -296,13 +293,39 @@ class Unreads extends ChangeNotifier {
296293
madeAnyUpdate |= mentions.add(messageId);
297294
}
298295

299-
// TODO(#901) handle moved messages
296+
madeAnyUpdate |= _handleMessageMove(event);
300297

301298
if (madeAnyUpdate) {
302299
notifyListeners();
303300
}
304301
}
305302

303+
bool _handleMessageMove(UpdateMessageEvent event) {
304+
if (event.moveData == null) {
305+
// No moved messages.
306+
return false;
307+
}
308+
final UpdateMessageMoveData(
309+
:origStreamId, :newStreamId, :origTopic, :newTopic) = event.moveData!;
310+
311+
final messageToMoveIds = _removeAllInStreamTopic(
312+
event.messageIds.toSet(), origStreamId, origTopic);
313+
if (messageToMoveIds == null || messageToMoveIds.isEmpty) {
314+
// No known unreads affected by move; nothing to do.
315+
return false;
316+
}
317+
318+
if (!channelStore.subscriptions.containsKey(newStreamId)) {
319+
// Unreads moved to an unsubscribed channel; just drop them.
320+
// See also:
321+
// https://chat.zulip.org/#narrow/channel/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/2101926
322+
return true;
323+
}
324+
325+
_addAllInStreamTopic(messageToMoveIds..sort(), newStreamId, newTopic);
326+
return true;
327+
}
328+
306329
void handleDeleteMessageEvent(DeleteMessageEvent event) {
307330
mentions.removeAll(event.messageIds);
308331
final messageIdsSet = Set.of(event.messageIds);

‎test/model/unreads_test.dart

+183
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,189 @@ void main() {
469469
}
470470
}
471471
});
472+
473+
group('moves', () {
474+
final origChannel = eg.stream();
475+
const origTopic = 'origTopic';
476+
const newTopic = 'newTopic';
477+
478+
Future<void> prepareStore() async {
479+
prepare();
480+
await channelStore.addStream(origChannel);
481+
await channelStore.addSubscription(eg.subscription(origChannel));
482+
}
483+
484+
group('move read messages', () {
485+
final readMessages = List<StreamMessage>.generate(10,
486+
(_) => eg.streamMessage(
487+
stream: origChannel, topic: origTopic, flags: [MessageFlag.read]));
488+
489+
test('to new channel', () async {
490+
await prepareStore();
491+
final newChannel = eg.stream();
492+
await channelStore.addStream(newChannel);
493+
await channelStore.addSubscription(eg.subscription(newChannel));
494+
fillWithMessages(readMessages);
495+
496+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
497+
origMessages: readMessages,
498+
newStreamId: newChannel.streamId));
499+
checkNotNotified();
500+
checkMatchesMessages([]);
501+
});
502+
503+
test('to new topic', () async {
504+
await prepareStore();
505+
fillWithMessages(readMessages);
506+
507+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
508+
origMessages: readMessages,
509+
newTopicStr: newTopic));
510+
checkNotNotified();
511+
checkMatchesMessages([]);
512+
});
513+
514+
test('from topic with unreads', () async {
515+
await prepareStore();
516+
final unreadMessage = eg.streamMessage(
517+
stream: origChannel, topic: origTopic);
518+
fillWithMessages([...readMessages, unreadMessage]);
519+
520+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
521+
origMessages: readMessages,
522+
newTopicStr: newTopic));
523+
checkNotNotified();
524+
checkMatchesMessages([unreadMessage]);
525+
});
526+
527+
test('to topic with unreads', () async {
528+
await prepareStore();
529+
final unreadMessage = eg.streamMessage(
530+
stream: origChannel, topic: newTopic);
531+
fillWithMessages([...readMessages, unreadMessage]);
532+
533+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
534+
origMessages: readMessages,
535+
newTopicStr: newTopic,
536+
));
537+
checkNotNotified();
538+
checkMatchesMessages([unreadMessage]);
539+
});
540+
});
541+
542+
group('move unread messages', () {
543+
final unreadMessages = List<StreamMessage>.generate(10,
544+
(_) => eg.streamMessage(stream: origChannel, topic: origTopic));
545+
546+
test('to another subscribed channel', () async {
547+
await prepareStore();
548+
final newChannel = eg.stream();
549+
await channelStore.addStream(newChannel);
550+
await channelStore.addSubscription(eg.subscription(newChannel));
551+
fillWithMessages(unreadMessages);
552+
553+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
554+
origMessages: unreadMessages,
555+
newStreamId: newChannel.streamId));
556+
checkNotifiedOnce();
557+
checkMatchesMessages([
558+
for (final message in unreadMessages)
559+
Message.fromJson(
560+
message.toJson()..['stream_id'] = newChannel.streamId),
561+
]);
562+
});
563+
564+
test('to unsubscribed channel', () async {
565+
await prepareStore();
566+
final newChannel = eg.stream();
567+
await channelStore.addStream(newChannel);
568+
fillWithMessages(unreadMessages);
569+
570+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
571+
origMessages: unreadMessages,
572+
newStreamId: newChannel.streamId));
573+
checkNotifiedOnce();
574+
checkMatchesMessages([]);
575+
});
576+
577+
test('to new topic', () async {
578+
await prepareStore();
579+
fillWithMessages(unreadMessages);
580+
581+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
582+
origMessages: unreadMessages,
583+
newTopicStr: newTopic));
584+
checkNotifiedOnce();
585+
checkMatchesMessages([
586+
for (final message in unreadMessages)
587+
Message.fromJson(message.toJson()..['subject'] = newTopic),
588+
]);
589+
});
590+
591+
test('from topic containing other unreads', () async {
592+
await prepareStore();
593+
final unreadMessage = eg.streamMessage(
594+
stream: origChannel, topic: origTopic);
595+
fillWithMessages([...unreadMessages, unreadMessage]);
596+
597+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
598+
origMessages: unreadMessages,
599+
newTopicStr: newTopic));
600+
checkNotifiedOnce();
601+
checkMatchesMessages([
602+
for (final message in unreadMessages)
603+
Message.fromJson(message.toJson()..['subject'] = newTopic),
604+
unreadMessage,
605+
]);
606+
});
607+
608+
test('to topic containing other unreads', () async {
609+
await prepareStore();
610+
final unreadMessage = eg.streamMessage(
611+
stream: origChannel, topic: newTopic);
612+
fillWithMessages([...unreadMessages, unreadMessage]);
613+
614+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
615+
origMessages: unreadMessages,
616+
newTopicStr: newTopic));
617+
checkNotifiedOnce();
618+
checkMatchesMessages([
619+
for (final message in unreadMessages)
620+
Message.fromJson(message.toJson()..['subject'] = newTopic),
621+
unreadMessage,
622+
]);
623+
});
624+
625+
test('tolerate unsorted messages', () async {
626+
await prepareStore();
627+
final unreadMessages = List.generate(10,
628+
(i) => eg.streamMessage(id: 1000-i, stream: origChannel, topic: origTopic));
629+
fillWithMessages(unreadMessages);
630+
631+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
632+
origMessages: unreadMessages,
633+
newTopicStr: newTopic));
634+
checkNotifiedOnce();
635+
checkMatchesMessages([
636+
for (final message in unreadMessages)
637+
Message.fromJson(message.toJson()..['subject'] = newTopic)
638+
]);
639+
});
640+
641+
test('tolerate unreads unknown to the model', () async {
642+
await prepareStore();
643+
final unknownUnreadMessage = eg.streamMessage(
644+
stream: eg.stream(), topic: origTopic);
645+
fillWithMessages(unreadMessages);
646+
647+
model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
648+
origMessages: [unknownUnreadMessage],
649+
newTopicStr: newTopic));
650+
checkNotNotified();
651+
checkMatchesMessages(unreadMessages);
652+
});
653+
});
654+
});
472655
});
473656

474657

0 commit comments

Comments
 (0)