From 9bbab2ae9cfabba9d87c11289f60d4aeaa211924 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 17 Jul 2024 13:34:16 -0700 Subject: [PATCH 1/5] channel: Split up asserts in SubscriptionAddEvent handler I just hit one of these in a new test I was writing, and wanted more information to debug: which of the two conditions failed? I seem to recall hearing a report of this in a non-test context recently, too, but I'm not finding it on a quick search of the tracker or of Zulip. --- lib/model/channel.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/model/channel.dart b/lib/model/channel.dart index 353acde8e4..fab9fac228 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -156,10 +156,10 @@ class ChannelStoreImpl with ChannelStore { switch (event) { case SubscriptionAddEvent(): for (final subscription in event.subscriptions) { - assert(streams.containsKey(subscription.streamId) - && streams[subscription.streamId] is! Subscription); - assert(streamsByName.containsKey(subscription.name) - && streamsByName[subscription.name] is! Subscription); + assert(streams.containsKey(subscription.streamId)); + assert(streams[subscription.streamId] is! Subscription); + assert(streamsByName.containsKey(subscription.name)); + assert(streamsByName[subscription.name] is! Subscription); assert(!subscriptions.containsKey(subscription.streamId)); streams[subscription.streamId] = subscription; streamsByName[subscription.name] = subscription; From 79dc06f6600bd9376f45d7294bf2fb6799e57ddb Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 27 Jun 2024 13:19:29 -0700 Subject: [PATCH 2/5] test [nfc]: Factor out an eg.userTopicEvent --- test/example_data.dart | 11 +++++++++++ test/model/test_store.dart | 9 ++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index bd7d214a88..d438b79853 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -426,6 +426,17 @@ const _unreadMsgs = unreadMsgs; // Events. // +UserTopicEvent userTopicEvent( + int streamId, String topic, UserTopicVisibilityPolicy visibilityPolicy) { + return UserTopicEvent( + id: 1, + streamId: streamId, + topicName: topic, + lastUpdated: 1234567890, + visibilityPolicy: visibilityPolicy, + ); +} + DeleteMessageEvent deleteMessageEvent(List messages) { assert(messages.isNotEmpty); final streamId = messages.first.streamId; diff --git a/test/model/test_store.dart b/test/model/test_store.dart index 32aeed5216..e3a7a5cae9 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -5,6 +5,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/store.dart'; import '../api/fake_api.dart'; +import '../example_data.dart' as eg; /// A [GlobalStore] containing data provided by callers, /// and that causes no database queries or network requests. @@ -146,13 +147,7 @@ extension PerAccountStoreTestExtension on PerAccountStore { } Future addUserTopic(ZulipStream stream, String topic, UserTopicVisibilityPolicy visibilityPolicy) async { - await handleEvent(UserTopicEvent( - id: 1, - streamId: stream.streamId, - topicName: topic, - lastUpdated: 1234567890, - visibilityPolicy: visibilityPolicy, - )); + await handleEvent(eg.userTopicEvent(stream.streamId, topic, visibilityPolicy)); } Future addMessage(Message message) async { From b463115aca885de70564702ac955c4e91fcd3eb3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 17 Jul 2024 13:41:11 -0700 Subject: [PATCH 3/5] test: Use stream ID in default name and description in eg.stream In particular this means that repeated calls `eg.stream()` with no arguments will get distinct names, as well as IDs, so that they can all be added to the same store without collision. Then also simplify away the various places where a test was passing a stream name when the only thing about the name that mattered to the test was being distinct from other names (or when nothing about the name mattered to the test at all). --- test/example_data.dart | 11 +++++++---- test/model/channel_test.dart | 12 ++++++------ test/model/message_list_test.dart | 14 +++++++------- test/model/unreads_test.dart | 6 +++--- test/widgets/message_list_test.dart | 4 ++-- test/widgets/subscription_list_test.dart | 8 ++++---- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index d438b79853..aa4f1931e4 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -183,11 +183,14 @@ ZulipStream stream({ }) { _checkPositive(streamId, 'stream ID'); _checkPositive(firstMessageId, 'message ID'); + var effectiveStreamId = streamId ?? _nextStreamId(); + var effectiveName = name ?? 'stream $effectiveStreamId'; + var effectiveDescription = description ?? 'Description of $effectiveName'; return ZulipStream( - streamId: streamId ?? _nextStreamId(), - name: name ?? 'A stream', // TODO generate example names - description: description ?? 'A description', // TODO generate example descriptions - renderedDescription: renderedDescription ?? '

A description

', // TODO generate random + streamId: effectiveStreamId, + name: effectiveName, + description: effectiveDescription, + renderedDescription: renderedDescription ?? '

$effectiveDescription

', dateCreated: dateCreated ?? 1686774898, firstMessageId: firstMessageId, inviteOnly: inviteOnly ?? false, diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index d8dc3c07f4..7e86d75b16 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -34,8 +34,8 @@ void main() { } test('initial', () { - final stream1 = eg.stream(streamId: 1, name: 'stream 1'); - final stream2 = eg.stream(streamId: 2, name: 'stream 2'); + final stream1 = eg.stream(); + final stream2 = eg.stream(); checkUnified(eg.store(initialSnapshot: eg.initialSnapshot( streams: [stream1, stream2], subscriptions: [eg.subscription(stream1)], @@ -43,8 +43,8 @@ void main() { }); test('added by events', () async { - final stream1 = eg.stream(streamId: 1, name: 'stream 1'); - final stream2 = eg.stream(streamId: 2, name: 'stream 2'); + final stream1 = eg.stream(); + final stream2 = eg.stream(); final store = eg.store(); checkUnified(store); @@ -106,8 +106,8 @@ void main() { }); group('topic visibility', () { - final stream1 = eg.stream(streamId: 1, name: 'stream 1'); - final stream2 = eg.stream(streamId: 2, name: 'stream 2'); + final stream1 = eg.stream(); + final stream2 = eg.stream(); group('getter topicVisibilityPolicy', () { test('with nothing for stream', () { diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index ce1f582b1e..486a04fe7e 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -467,8 +467,8 @@ void main() { }); group('messagesMoved', () { - final stream = eg.stream(streamId: 1, name: 'test stream'); - final otherStream = eg.stream(streamId: 2, name: 'other stream'); + final stream = eg.stream(); + final otherStream = eg.stream(); void checkHasMessages(Iterable messages) { check(model.messages.map((e) => e.id)).deepEquals(messages.map((e) => e.id)); @@ -1094,8 +1094,8 @@ void main() { group('stream/topic muting', () { test('in CombinedFeedNarrow', () async { - final stream1 = eg.stream(streamId: 1, name: 'stream 1'); - final stream2 = eg.stream(streamId: 2, name: 'stream 2'); + final stream1 = eg.stream(); + final stream2 = eg.stream(); await prepare(narrow: const CombinedFeedNarrow()); await store.addStreams([stream1, stream2]); await store.addSubscription(eg.subscription(stream1)); @@ -1157,7 +1157,7 @@ void main() { }); test('in ChannelNarrow', () async { - final stream = eg.stream(streamId: 1, name: 'stream 1'); + final stream = eg.stream(); await prepare(narrow: ChannelNarrow(stream.streamId)); await store.addStream(stream); await store.addSubscription(eg.subscription(stream, isMuted: true)); @@ -1204,7 +1204,7 @@ void main() { }); test('in TopicNarrow', () async { - final stream = eg.stream(streamId: 1, name: 'stream 1'); + final stream = eg.stream(); await prepare(narrow: TopicNarrow(stream.streamId, 'A')); await store.addStream(stream); await store.addSubscription(eg.subscription(stream, isMuted: true)); @@ -1236,7 +1236,7 @@ void main() { }); test('in MentionsNarrow', () async { - final stream = eg.stream(streamId: 1, name: 'muted stream'); + final stream = eg.stream(); const mutedTopic = 'muted'; await prepare(narrow: const MentionsNarrow()); await store.addStream(stream); diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index b35f5c770e..1f66b2ad25 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -153,9 +153,9 @@ void main() { group('count helpers', () { test('countInCombinedFeedNarrow', () async { - final stream1 = eg.stream(streamId: 1, name: 'stream 1'); - final stream2 = eg.stream(streamId: 2, name: 'stream 2'); - final stream3 = eg.stream(streamId: 3, name: 'stream 3'); + final stream1 = eg.stream(); + final stream2 = eg.stream(); + final stream3 = eg.stream(); prepare(); await channelStore.addStreams([stream1, stream2, stream3]); await channelStore.addSubscription(eg.subscription(stream1)); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 23a0be361a..741dbdeec1 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -618,8 +618,8 @@ void main() { group('Update Narrow on message move', () { const topic = 'foo'; - final channel = eg.stream(name: 'move test stream'); - final otherChannel = eg.stream(name: 'other move test stream'); + final channel = eg.stream(); + final otherChannel = eg.stream(); final narrow = TopicNarrow(channel.streamId, topic); void prepareGetMessageResponse(List messages) { diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 9909dc09e9..b1c7b89137 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -284,10 +284,10 @@ void main() { check(wght).equals(expectedWght); } - final unmutedStreamWithUnmutedUnreads = eg.stream(name: 'Unmuted stream with unmuted unreads'); - final unmutedStreamWithNoUnmutedUnreads = eg.stream(name: 'Unmuted stream with no unmuted unreads'); - final mutedStreamWithUnmutedUnreads = eg.stream(name: 'Muted stream with unmuted unreads'); - final mutedStreamWithNoUnmutedUnreads = eg.stream(name: 'Muted stream with no unmuted unreads'); + final unmutedStreamWithUnmutedUnreads = eg.stream(); + final unmutedStreamWithNoUnmutedUnreads = eg.stream(); + final mutedStreamWithUnmutedUnreads = eg.stream(); + final mutedStreamWithNoUnmutedUnreads = eg.stream(); await setupStreamListPage(tester, subscriptions: [ From fde2ee473d78c68b6539b09d1d8ee3df40b9bb46 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 27 Jun 2024 13:03:41 -0700 Subject: [PATCH 4/5] channel: Add willChangeIfTopicVisible/InStream methods --- lib/model/channel.dart | 54 ++++++++++++++++++++- test/model/channel_test.dart | 91 ++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/lib/model/channel.dart b/lib/model/channel.dart index fab9fac228..81dc4123cb 100644 --- a/lib/model/channel.dart +++ b/lib/model/channel.dart @@ -25,7 +25,21 @@ mixin ChannelStore { /// For UI contexts that are not specific to a particular stream, see /// [isTopicVisible]. bool isTopicVisibleInStream(int streamId, String topic) { - switch (topicVisibilityPolicy(streamId, topic)) { + return _isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic)); + } + + /// Whether the given event will change the result of [isTopicVisibleInStream] + /// for its stream and topic, compared to the current state. + VisibilityEffect willChangeIfTopicVisibleInStream(UserTopicEvent event) { + final streamId = event.streamId; + final topic = event.topicName; + return VisibilityEffect._fromBeforeAfter( + _isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic)), + _isTopicVisibleInStream(event.visibilityPolicy)); + } + + static bool _isTopicVisibleInStream(UserTopicVisibilityPolicy policy) { + switch (policy) { case UserTopicVisibilityPolicy.none: return true; case UserTopicVisibilityPolicy.muted: @@ -48,7 +62,21 @@ mixin ChannelStore { /// For UI contexts that are specific to a particular stream, see /// [isTopicVisibleInStream]. bool isTopicVisible(int streamId, String topic) { - switch (topicVisibilityPolicy(streamId, topic)) { + return _isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic)); + } + + /// Whether the given event will change the result of [isTopicVisible] + /// for its stream and topic, compared to the current state. + VisibilityEffect willChangeIfTopicVisible(UserTopicEvent event) { + final streamId = event.streamId; + final topic = event.topicName; + return VisibilityEffect._fromBeforeAfter( + _isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic)), + _isTopicVisible(streamId, event.visibilityPolicy)); + } + + bool _isTopicVisible(int streamId, UserTopicVisibilityPolicy policy) { + switch (policy) { case UserTopicVisibilityPolicy.none: switch (subscriptions[streamId]?.isMuted) { case false: return true; @@ -67,6 +95,28 @@ mixin ChannelStore { } } +/// Whether and how a given [UserTopicEvent] will affect the results +/// that [ChannelStore.isTopicVisible] or [ChannelStore.isTopicVisibleInStream] +/// would give for some messages. +enum VisibilityEffect { + /// The event will have no effect on the visibility results. + none, + + /// The event will change some visibility results from true to false. + muted, + + /// The event will change some visibility results from false to true. + unmuted; + + factory VisibilityEffect._fromBeforeAfter(bool before, bool after) { + return switch ((before, after)) { + (false, true) => VisibilityEffect.unmuted, + (true, false) => VisibilityEffect.muted, + _ => VisibilityEffect.none, + }; + } +} + /// The implementation of [ChannelStore] that does the work. /// /// Generally the only code that should need this class is [PerAccountStore] diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart index 7e86d75b16..219529e2da 100644 --- a/test/model/channel_test.dart +++ b/test/model/channel_test.dart @@ -189,6 +189,97 @@ void main() { }); }); + group('willChangeIfTopicVisible/InStream', () { + UserTopicEvent mkEvent(UserTopicVisibilityPolicy policy) => + eg.userTopicEvent(stream1.streamId, 'topic', policy); + + void checkChanges(PerAccountStore store, + UserTopicVisibilityPolicy newPolicy, + VisibilityEffect expectedInStream, VisibilityEffect expectedOverall) { + final event = mkEvent(newPolicy); + check(store.willChangeIfTopicVisibleInStream(event)).equals(expectedInStream); + check(store.willChangeIfTopicVisible (event)).equals(expectedOverall); + } + + test('stream not muted, policy none -> followed, no change', () async { + final store = eg.store(); + await store.addStream(stream1); + await store.addSubscription(eg.subscription(stream1)); + checkChanges(store, UserTopicVisibilityPolicy.followed, + VisibilityEffect.none, VisibilityEffect.none); + }); + + test('stream not muted, policy none -> muted, means muted', () async { + final store = eg.store(); + await store.addStream(stream1); + await store.addSubscription(eg.subscription(stream1)); + checkChanges(store, UserTopicVisibilityPolicy.muted, + VisibilityEffect.muted, VisibilityEffect.muted); + }); + + test('stream muted, policy none -> followed, means none/unmuted', () async { + final store = eg.store(); + await store.addStream(stream1); + await store.addSubscription(eg.subscription(stream1, isMuted: true)); + checkChanges(store, UserTopicVisibilityPolicy.followed, + VisibilityEffect.none, VisibilityEffect.unmuted); + }); + + test('stream muted, policy none -> muted, means muted/none', () async { + final store = eg.store(); + await store.addStream(stream1); + await store.addSubscription(eg.subscription(stream1, isMuted: true)); + checkChanges(store, UserTopicVisibilityPolicy.muted, + VisibilityEffect.muted, VisibilityEffect.none); + }); + + final policies = [ + UserTopicVisibilityPolicy.muted, + UserTopicVisibilityPolicy.none, + UserTopicVisibilityPolicy.unmuted, + ]; + for (final streamMuted in [null, false, true]) { + for (final oldPolicy in policies) { + for (final newPolicy in policies) { + final streamDesc = switch (streamMuted) { + false => "stream not muted", + true => "stream muted", + null => "stream unsubscribed", + }; + test('$streamDesc, topic ${oldPolicy.name} -> ${newPolicy.name}', () async { + final store = eg.store(); + await store.addStream(stream1); + if (streamMuted != null) { + await store.addSubscription( + eg.subscription(stream1, isMuted: streamMuted)); + } + store.handleEvent(mkEvent(oldPolicy)); + final oldVisibleInStream = store.isTopicVisibleInStream(stream1.streamId, 'topic'); + final oldVisible = store.isTopicVisible(stream1.streamId, 'topic'); + + final event = mkEvent(newPolicy); + final willChangeInStream = store.willChangeIfTopicVisibleInStream(event); + final willChange = store.willChangeIfTopicVisible(event); + + store.handleEvent(event); + final newVisibleInStream = store.isTopicVisibleInStream(stream1.streamId, 'topic'); + final newVisible = store.isTopicVisible(stream1.streamId, 'topic'); + + VisibilityEffect fromOldNew(bool oldVisible, bool newVisible) { + if (newVisible == oldVisible) return VisibilityEffect.none; + if (newVisible) return VisibilityEffect.unmuted; + return VisibilityEffect.muted; + } + check(willChangeInStream) + .equals(fromOldNew(oldVisibleInStream, newVisibleInStream)); + check(willChange) + .equals(fromOldNew(oldVisible, newVisible)); + }); + } + } + } + }); + void compareTopicVisibility(PerAccountStore store, List expected) { final expectedStore = eg.store(initialSnapshot: eg.initialSnapshot( userTopics: expected, From d37f0f71d1b0b6466c6a16ae91a9285aab8cece3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 17 Jul 2024 12:24:49 -0700 Subject: [PATCH 5/5] msglist: Handle UserTopicEvent, hiding/showing messages as needed In particular this will allow us to start offering UI for muting and unmuting topics, and have the message list the user is looking at update appropriately when they do so. Fixes: #421 --- lib/model/message.dart | 6 ++ lib/model/message_list.dart | 76 +++++++++++++ lib/model/store.dart | 2 + test/model/message_list_test.dart | 173 ++++++++++++++++++++++++++++++ 4 files changed, 257 insertions(+) diff --git a/lib/model/message.dart b/lib/model/message.dart index 1e3abbbcdb..a3094941f0 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -82,6 +82,12 @@ class MessageStoreImpl with MessageStore { } } + void handleUserTopicEvent(UserTopicEvent event) { + for (final view in _messageListViews) { + view.handleUserTopicEvent(event); + } + } + void handleMessageEvent(MessageEvent event) { // If the message is one we already know about (from a fetch), // clobber it with the one from the event system. diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index f20960e221..95d660d4f5 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -5,6 +5,7 @@ import '../api/model/events.dart'; import '../api/model/model.dart'; import '../api/route/messages.dart'; import 'algorithms.dart'; +import 'channel.dart'; import 'content.dart'; import 'narrow.dart'; import 'store.dart'; @@ -158,6 +159,38 @@ mixin _MessageSequence { _processMessage(messages.length - 1); } + /// Removes all messages from the list that satisfy [test]. + /// + /// Returns true if any messages were removed, false otherwise. + bool _removeMessagesWhere(bool Function(Message) test) { + // Before we find a message to remove, there's no need to copy elements. + // This is like the loop below, but simplified for `target == candidate`. + int candidate = 0; + while (true) { + if (candidate == messages.length) return false; + if (test(messages[candidate])) break; + candidate++; + } + + int target = candidate; + candidate++; + assert(contents.length == messages.length); + while (candidate < messages.length) { + if (test(messages[candidate])) { + candidate++; + continue; + } + messages[target] = messages[candidate]; + contents[target] = contents[candidate]; + target++; candidate++; + } + messages.length = target; + contents.length = target; + assert(contents.length == messages.length); + _reprocessAll(); + return true; + } + /// Removes the given messages, if present. /// /// Returns true if at least one message was present, false otherwise. @@ -389,6 +422,24 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } + /// Whether this event could affect the result that [_messageVisible] + /// would ever have returned for any possible message in this message list. + VisibilityEffect _canAffectVisibility(UserTopicEvent event) { + switch (narrow) { + case CombinedFeedNarrow(): + return store.willChangeIfTopicVisible(event); + + case ChannelNarrow(:final streamId): + if (event.streamId != streamId) return VisibilityEffect.none; + return store.willChangeIfTopicVisibleInStream(event); + + case TopicNarrow(): + case DmNarrow(): + case MentionsNarrow(): + return VisibilityEffect.none; + } + } + /// Whether [_messageVisible] is true for all possible messages. /// /// This is useful for an optimization. @@ -477,6 +528,31 @@ class MessageListView with ChangeNotifier, _MessageSequence { } } + void handleUserTopicEvent(UserTopicEvent event) { + switch (_canAffectVisibility(event)) { + case VisibilityEffect.none: + return; + + case VisibilityEffect.muted: + if (_removeMessagesWhere((message) => + (message is StreamMessage + && message.streamId == event.streamId + && message.topic == event.topicName))) { + notifyListeners(); + } + + case VisibilityEffect.unmuted: + // TODO get the newly-unmuted messages from the message store + // For now, we simplify the task by just refetching this message list + // from scratch. + if (fetched) { + _reset(); + notifyListeners(); + fetchInitial(); + } + } + } + void handleDeleteMessageEvent(DeleteMessageEvent event) { if (_removeMessagesById(event.messageIds)) { notifyListeners(); diff --git a/lib/model/store.dart b/lib/model/store.dart index 228ff2b37e..bdd6c0d9a5 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -503,6 +503,8 @@ class PerAccountStore extends ChangeNotifier with ChannelStore, MessageStore { case UserTopicEvent(): assert(debugLog("server event: user_topic")); + _messages.handleUserTopicEvent(event); + // Update _channels last, so other handlers can compare to the old value. _channels.handleUserTopicEvent(event); notifyListeners(); diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index 486a04fe7e..ccb006e78a 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -311,6 +311,179 @@ void main() { check(model).fetched.isFalse(); }); + group('UserTopicEvent', () { + // The ChannelStore.willChangeIfTopicVisible/InStream methods have their own + // thorough unit tests. So these tests focus on the rest of the logic. + + final stream = eg.stream(); + const String topic = 'foo'; + + Future setVisibility(UserTopicVisibilityPolicy policy) async { + await store.handleEvent(eg.userTopicEvent(stream.streamId, topic, policy)); + } + + /// (Should run after `prepare`.) + Future prepareMutes([ + bool streamMuted = false, + UserTopicVisibilityPolicy policy = UserTopicVisibilityPolicy.none, + ]) async { + await store.addStream(stream); + await store.addSubscription(eg.subscription(stream, isMuted: streamMuted)); + await setVisibility(policy); + } + + void checkHasMessageIds(Iterable messageIds) { + check(model.messages.map((m) => m.id)).deepEquals(messageIds); + } + + test('mute a visible topic', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMutes(); + final otherStream = eg.stream(); + await store.addStream(otherStream); + await store.addSubscription(eg.subscription(otherStream)); + await prepareMessages(foundOldest: true, messages: [ + eg.streamMessage(id: 1, stream: stream, topic: 'bar'), + eg.streamMessage(id: 2, stream: stream, topic: topic), + eg.streamMessage(id: 3, stream: otherStream, topic: 'elsewhere'), + eg.dmMessage( id: 4, from: eg.otherUser, to: [eg.selfUser]), + ]); + checkHasMessageIds([1, 2, 3, 4]); + + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotifiedOnce(); + checkHasMessageIds([1, 3, 4]); + }); + + test('in CombinedFeedNarrow, use combined-feed visibility', () async { + // Compare the parallel ChannelNarrow test below. + await prepare(narrow: const CombinedFeedNarrow()); + // Mute the stream, so that combined-feed vs. stream visibility differ. + await prepareMutes(true, UserTopicVisibilityPolicy.followed); + await prepareMessages(foundOldest: true, messages: [ + eg.streamMessage(id: 1, stream: stream, topic: topic), + ]); + checkHasMessageIds([1]); + + // Dropping from followed to none hides the message + // (whereas it'd have no effect in a stream narrow). + await setVisibility(UserTopicVisibilityPolicy.none); + checkNotifiedOnce(); + checkHasMessageIds([]); + + // Dropping from none to muted has no further effect + // (whereas it'd hide the message in a stream narrow). + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotNotified(); + checkHasMessageIds([]); + }); + + test('in ChannelNarrow, use stream visibility', () async { + // Compare the parallel CombinedFeedNarrow test above. + await prepare(narrow: ChannelNarrow(stream.streamId)); + // Mute the stream, so that combined-feed vs. stream visibility differ. + await prepareMutes(true, UserTopicVisibilityPolicy.followed); + await prepareMessages(foundOldest: true, messages: [ + eg.streamMessage(id: 1, stream: stream, topic: topic), + ]); + checkHasMessageIds([1]); + + // Dropping from followed to none has no effect + // (whereas it'd hide the message in the combined feed). + await setVisibility(UserTopicVisibilityPolicy.none); + checkNotNotified(); + checkHasMessageIds([1]); + + // Dropping from none to muted hides the message + // (whereas it'd have no effect in a stream narrow). + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotifiedOnce(); + checkHasMessageIds([]); + }); + + test('in TopicNarrow, stay visible', () async { + await prepare(narrow: TopicNarrow(stream.streamId, topic)); + await prepareMutes(); + await prepareMessages(foundOldest: true, messages: [ + eg.streamMessage(id: 1, stream: stream, topic: topic), + ]); + checkHasMessageIds([1]); + + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotNotified(); + checkHasMessageIds([1]); + }); + + test('in DmNarrow, do nothing (smoke test)', () async { + await prepare(narrow: + DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId)); + await prepareMutes(); + await prepareMessages(foundOldest: true, messages: [ + eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]), + ]); + checkHasMessageIds([1]); + + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotNotified(); + checkHasMessageIds([1]); + }); + + test('no affected messages -> no notification', () async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMutes(); + await prepareMessages(foundOldest: true, messages: [ + eg.streamMessage(id: 1, stream: stream, topic: 'bar'), + ]); + checkHasMessageIds([1]); + + await setVisibility(UserTopicVisibilityPolicy.muted); + checkNotNotified(); + checkHasMessageIds([1]); + }); + + test('unmute a topic -> refetch from scratch', () => awaitFakeAsync((async) async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMutes(true); + final messages = [ + eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]), + eg.streamMessage(id: 2, stream: stream, topic: topic), + ]; + await prepareMessages(foundOldest: true, messages: messages); + checkHasMessageIds([1]); + + connection.prepare( + json: newestResult(foundOldest: true, messages: messages).toJson()); + await setVisibility(UserTopicVisibilityPolicy.unmuted); + checkNotifiedOnce(); + check(model).fetched.isFalse(); + checkHasMessageIds([]); + + async.elapse(Duration.zero); + checkNotifiedOnce(); + checkHasMessageIds([1, 2]); + })); + + test('unmute a topic before initial fetch completes -> do nothing', () => awaitFakeAsync((async) async { + await prepare(narrow: const CombinedFeedNarrow()); + await prepareMutes(true); + final messages = [ + eg.streamMessage(id: 1, stream: stream, topic: topic), + ]; + + connection.prepare( + json: newestResult(foundOldest: true, messages: messages).toJson()); + final fetchFuture = model.fetchInitial(); + + await setVisibility(UserTopicVisibilityPolicy.unmuted); + checkNotNotified(); + + // The new policy does get applied when the fetch eventually completes. + await fetchFuture; + checkNotifiedOnce(); + checkHasMessageIds([1]); + })); + }); + group('DeleteMessageEvent', () { final stream = eg.stream(); final messages = List.generate(30, (i) => eg.streamMessage(stream: stream));