Skip to content

Commit

Permalink
unreads: Handle updates when there are moved messages
Browse files Browse the repository at this point in the history 
Signed-off-by: Zixuan James Li <zixuan@zulip.com>
  • Loading branch information
@PIG208
PIG208 committed Feb 26, 2025
1 parent e14008e commit be550c4
Show file tree
Hide file tree
Showing 2 changed files with 246 additions and 5 deletions.
45 changes: 40 additions & 5 deletions lib/model/unreads.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,8 @@ class Unreads extends ChangeNotifier {
(f) => f == MessageFlag.mentioned || f == MessageFlag.wildcardMentioned,
);

// We assume this event can't signal a change in a message's 'read' flag.
// TODO can it actually though, when it's about messages being moved into an
// unsubscribed stream?
// https://chat.zulip.org/#narrow/stream/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/1639957
// We expect the event's 'read' flag to be boring,
// matching the message's local unread state.
final bool isRead = event.flags.contains(MessageFlag.read);
assert(() {
final isUnreadLocally = isUnread(messageId);
Expand All @@ -272,6 +270,17 @@ class Unreads extends ChangeNotifier {
// We were going to check something but can't; shrug.
if (isUnreadLocally == null) return true;

final newChannelId = event.moveData?.newStreamId;
if (newChannelId != null && !channelStore.subscriptions.containsKey(newChannelId)) {
// When unread messages are moved to an unsubscribed channel, the server
// marks them as read without sending a mark-as-read event. Clients are
// asked to special-case this by marking them as read, which we do in
// _handleMessageMove. That contract is clear enough and doesn't involve
// this event's 'read' flag, so don't bother logging about the flag;
// its behavior seems like an implementation detail that could change.
return true;
}

if (isUnreadLocally != isUnreadInEvent) {
// If this happens, then either:
// - the server and client have been out of sync about the message's
Expand All @@ -296,13 +305,39 @@ class Unreads extends ChangeNotifier {
madeAnyUpdate |= mentions.add(messageId);
}

// TODO(#901) handle moved messages
madeAnyUpdate |= _handleMessageMove(event);

if (madeAnyUpdate) {
notifyListeners();
}
}

bool _handleMessageMove(UpdateMessageEvent event) {
if (event.moveData == null) {
// No moved messages.
return false;
}
final UpdateMessageMoveData(
:origStreamId, :newStreamId, :origTopic, :newTopic) = event.moveData!;

final messageToMoveIds = _removeAllInStreamTopic(
event.messageIds.toSet(), origStreamId, origTopic);
if (messageToMoveIds == null || messageToMoveIds.isEmpty) {
// No known unreads affected by move; nothing to do.
return false;
}

if (!channelStore.subscriptions.containsKey(newStreamId)) {
// Unreads moved to an unsubscribed channel; just drop them.
// See also:
// https://chat.zulip.org/#narrow/channel/378-api-design/topic/mark-as-read.20events.20with.20message.20moves.3F/near/2101926
return true;
}

_addAllInStreamTopic(messageToMoveIds..sort(), newStreamId, newTopic);
return true;
}

void handleDeleteMessageEvent(DeleteMessageEvent event) {
mentions.removeAll(event.messageIds);
final messageIdsSet = Set.of(event.messageIds);
Expand Down
206 changes: 206 additions & 0 deletions test/model/unreads_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,212 @@ void main() {
}
}
});

group('moves', () {
final origChannel = eg.stream();
const origTopic = 'origTopic';
const newTopic = 'newTopic';

Future<void> prepareStore() async {
prepare();
await channelStore.addStream(origChannel);
await channelStore.addSubscription(eg.subscription(origChannel));
}

group('move read messages', () {
final readMessages = List<StreamMessage>.generate(10,
(_) => eg.streamMessage(
stream: origChannel, topic: origTopic, flags: [MessageFlag.read]));

test('to new channel', () async {
await prepareStore();
final newChannel = eg.stream();
await channelStore.addStream(newChannel);
await channelStore.addSubscription(eg.subscription(newChannel));
fillWithMessages(readMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: readMessages,
newStreamId: newChannel.streamId));
checkNotNotified();
checkMatchesMessages([]);
});

test('to new topic', () async {
await prepareStore();
fillWithMessages(readMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: readMessages,
newTopicStr: newTopic));
checkNotNotified();
checkMatchesMessages([]);
});

test('from topic with unreads', () async {
await prepareStore();
final unreadMessage = eg.streamMessage(
stream: origChannel, topic: origTopic);
fillWithMessages([...readMessages, unreadMessage]);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: readMessages,
newTopicStr: newTopic));
checkNotNotified();
checkMatchesMessages([unreadMessage]);
});

test('to topic with unreads', () async {
await prepareStore();
final unreadMessage = eg.streamMessage(
stream: origChannel, topic: newTopic);
fillWithMessages([...readMessages, unreadMessage]);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: readMessages,
newTopicStr: newTopic,
));
checkNotNotified();
checkMatchesMessages([unreadMessage]);
});
});

group('move unread messages', () {
final unreadMessages = List<StreamMessage>.generate(10,
(_) => eg.streamMessage(stream: origChannel, topic: origTopic));

test('to another subscribed channel; same topic name', () async {
await prepareStore();
final newChannel = eg.stream();
await channelStore.addStream(newChannel);
await channelStore.addSubscription(eg.subscription(newChannel));
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newStreamId: newChannel.streamId));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(
message.toJson()..['stream_id'] = newChannel.streamId),
]);
});

test('to another subscribed channel; different topic name', () async {
await prepareStore();
final newChannel = eg.stream();
await channelStore.addStream(newChannel);
await channelStore.addSubscription(eg.subscription(newChannel));
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newStreamId: newChannel.streamId,
newTopicStr: newTopic));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(
message.toJson()
..['stream_id'] = newChannel.streamId
..['subject'] = newTopic
),
]);
});

test('to unsubscribed channel', () async {
await prepareStore();
final newChannel = eg.stream();
await channelStore.addStream(newChannel);
assert(!channelStore.subscriptions.containsKey(newChannel.streamId));
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newStreamId: newChannel.streamId));
checkNotifiedOnce();
checkMatchesMessages([]);
});

test('to new topic', () async {
await prepareStore();
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newTopicStr: newTopic));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(message.toJson()..['subject'] = newTopic),
]);
});

test('from topic containing other unreads', () async {
await prepareStore();
final unreadMessage = eg.streamMessage(
stream: origChannel, topic: origTopic);
fillWithMessages([...unreadMessages, unreadMessage]);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newTopicStr: newTopic));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(message.toJson()..['subject'] = newTopic),
unreadMessage,
]);
});

test('to topic containing other unreads', () async {
await prepareStore();
final unreadMessage = eg.streamMessage(
stream: origChannel, topic: newTopic);
fillWithMessages([...unreadMessages, unreadMessage]);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newTopicStr: newTopic));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(message.toJson()..['subject'] = newTopic),
unreadMessage,
]);
});

test('tolerates unsorted messages', () async {
await prepareStore();
final unreadMessages = List.generate(10,
(i) => eg.streamMessage(id: 1000-i, stream: origChannel, topic: origTopic));
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: unreadMessages,
newTopicStr: newTopic));
checkNotifiedOnce();
checkMatchesMessages([
for (final message in unreadMessages)
Message.fromJson(message.toJson()..['subject'] = newTopic)
]);
});

test('tolerates unreads unknown to the model', () async {
await prepareStore();
final unknownUnreadMessage = eg.streamMessage(
stream: eg.stream(), topic: origTopic);
fillWithMessages(unreadMessages);

model.handleUpdateMessageEvent(eg.updateMessageEventMoveFrom(
origMessages: [unknownUnreadMessage],
newTopicStr: newTopic));
checkNotNotified();
checkMatchesMessages(unreadMessages);
});
});
});
});


Expand Down

0 comments on commit be550c4

Please sign in to comment.