Skip to content

Commit 3d20ce1

Browse files
sm-sayedism-sayedi
committed
msglist: Exclude DMs with muted users from applicable narrows
Direct messages of those conversations are excluded where all of the other recipients are muted. Currently the applicable narrows are: - CombinedFeedNarrow - MentionsNarrow - StarredMessagesNarrow In the future, this will apply to the ReactionsNarrow from the Web app too, once we have it.
1 parent 8626061 commit 3d20ce1

File tree

6 files changed

+262
-4
lines changed

6 files changed

+262
-4
lines changed

‎lib/model/message.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
489489
poll.handleSubmessageEvent(event);
490490
}
491491

492+
void handleMutedUsersEvent(MutedUsersEvent event) {
493+
for (final view in _messageListViews) {
494+
view.handleMutedUsersEvent(event);
495+
}
496+
}
497+
492498
/// In debug mode, controls whether outbox messages should be created when
493499
/// [sendMessage] is called.
494500
///

‎lib/model/message_list.dart

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'content.dart';
1313
import 'message.dart';
1414
import 'narrow.dart';
1515
import 'store.dart';
16+
import 'user.dart';
1617

1718
/// The number of messages to fetch in each request.
1819
const kMessageListFetchBatchSize = 100; // TODO tune
@@ -461,7 +462,10 @@ class MessageListView with ChangeNotifier, _MessageSequence {
461462
return switch (message.conversation) {
462463
StreamConversation(:final streamId, :final topic) =>
463464
store.isTopicVisible(streamId, topic),
464-
DmConversation() => true,
465+
DmConversation(:final allRecipientIds) => () {
466+
return !store.ignoreConversation(DmNarrow(
467+
allRecipientIds: allRecipientIds, selfUserId: store.selfUserId));
468+
}(),
465469
};
466470

467471
case ChannelNarrow(:final streamId):
@@ -472,8 +476,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
472476

473477
case TopicNarrow():
474478
case DmNarrow():
479+
return true;
480+
475481
case MentionsNarrow():
482+
// case ReactionsNarrow(): // TODO
476483
case StarredMessagesNarrow():
484+
if (message.conversation case DmConversation(:final allRecipientIds)) {
485+
return !store.ignoreConversation(DmNarrow(
486+
allRecipientIds: allRecipientIds, selfUserId: store.selfUserId));
487+
}
477488
return true;
478489
}
479490
}
@@ -485,12 +496,13 @@ class MessageListView with ChangeNotifier, _MessageSequence {
485496
switch (narrow) {
486497
case CombinedFeedNarrow():
487498
case ChannelNarrow():
499+
case MentionsNarrow():
500+
// case ReactionsNarrow(): // TODO
501+
case StarredMessagesNarrow():
488502
return false;
489503

490504
case TopicNarrow():
491505
case DmNarrow():
492-
case MentionsNarrow():
493-
case StarredMessagesNarrow():
494506
return true;
495507
}
496508
}
@@ -514,6 +526,22 @@ class MessageListView with ChangeNotifier, _MessageSequence {
514526
}
515527
}
516528

529+
/// Whether this event could affect the result that [_messageVisible]
530+
/// would ever have returned for any possible message in this message list.
531+
MutenessEffect? _canAffectMuteness(MutedUsersEvent event) {
532+
switch(narrow) {
533+
case CombinedFeedNarrow():
534+
case MentionsNarrow():
535+
case StarredMessagesNarrow():
536+
return store.willChangeIfRecipientMuted(event);
537+
538+
case ChannelNarrow():
539+
case TopicNarrow():
540+
case DmNarrow():
541+
return null;
542+
}
543+
}
544+
517545
/// Fetch messages, starting from scratch.
518546
Future<void> fetchInitial() async {
519547
// TODO(#80): fetch from anchor firstUnread, instead of newest
@@ -706,6 +734,33 @@ class MessageListView with ChangeNotifier, _MessageSequence {
706734
notifyListeners();
707735
}
708736

737+
void handleMutedUsersEvent(MutedUsersEvent event) {
738+
switch(_canAffectMuteness(event)) {
739+
case MutenessEffect.added:
740+
if (_removeMessagesWhere((message) =>
741+
(message is DmMessage
742+
&& store.ignoreConversation(
743+
DmNarrow.ofMessage(message, selfUserId: store.selfUserId),
744+
mutedUsers: {...event.mutedUsers.map((e) => e.id)},
745+
)))) {
746+
notifyListeners();
747+
}
748+
749+
case MutenessEffect.removed:
750+
// TODO get the newly-unmuted messages from the message store
751+
// For now, we simplify the task by just refetching this message list
752+
// from scratch.
753+
if (fetched) {
754+
_reset();
755+
notifyListeners();
756+
fetchInitial();
757+
}
758+
759+
default:
760+
return;
761+
}
762+
}
763+
709764
/// Update data derived from the content of the given message.
710765
///
711766
/// This does not notify listeners.

‎lib/model/store.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
962962

963963
case MutedUsersEvent():
964964
assert(debugLog("server event: muted_users"));
965+
_messages.handleMutedUsersEvent(event);
966+
// Update _users last, so other handlers can compare to the old value.
965967
_users.handleMutedUsersEvent(event);
966968
notifyListeners();
967969

‎lib/model/user.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,25 @@ mixin UserStore on PerAccountStoreBase {
9191
if (narrow.otherRecipientIds.isEmpty) return false;
9292
return !narrow.otherRecipientIds.any((id) => !isUserMuted(id, mutedUsers: mutedUsers));
9393
}
94+
95+
/// Whether the given event will change the result of [allRecipientsMuted]
96+
/// for its mutedUsers, compared to the current state.
97+
MutenessEffect willChangeIfRecipientMuted(MutedUsersEvent event) {
98+
assert(mutedUsers.length != event.mutedUsers.length);
99+
return mutedUsers.length < event.mutedUsers.length
100+
? MutenessEffect.added
101+
: MutenessEffect.removed;
102+
}
103+
}
104+
105+
/// Whether a given [MutedUsersEvent] will affect the results
106+
/// that [UserStore.allRecipientsMuted] would give for some messages.
107+
enum MutenessEffect {
108+
/// A new user is added to the muted users list.
109+
added,
110+
111+
/// A new user is removed from the muted users list.
112+
removed,
94113
}
95114

96115
/// The implementation of [UserStore] that does the work.

‎test/model/message_list_test.dart

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,18 @@ void main() {
6666
void checkNotifiedOnce() => checkNotified(count: 1);
6767

6868
/// Initialize [model] and the rest of the test state.
69-
Future<void> prepare({Narrow narrow = const CombinedFeedNarrow()}) async {
69+
Future<void> prepare({
70+
Narrow narrow = const CombinedFeedNarrow(),
71+
List<User>? users,
72+
List<int>? mutedUserIds,
73+
}) async {
7074
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
7175
subscription = eg.subscription(stream);
7276
store = eg.store();
7377
await store.addStream(stream);
7478
await store.addSubscription(subscription);
79+
await store.addUsers([...?users, eg.selfUser]);
80+
await store.muteUsers(mutedUserIds ?? []);
7581
connection = store.connection as FakeApiConnection;
7682
notifiedCount = 0;
7783
model = MessageListView.init(store: store, narrow: narrow)
@@ -656,6 +662,144 @@ void main() {
656662
});
657663
});
658664

665+
group('MutedUsersEvent', () {
666+
final user1 = eg.user(userId: 1);
667+
final user2 = eg.user(userId: 2);
668+
final user3 = eg.user(userId: 3);
669+
final users = [user1, user2, user3];
670+
671+
test('CombinedFeedNarrow', () async {
672+
await prepare(narrow: CombinedFeedNarrow(), users: users);
673+
await prepareMessages(foundOldest: true, messages: [
674+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1]),
675+
eg.dmMessage(id: 2, from: eg.selfUser, to: [user1, user2]),
676+
eg.dmMessage(id: 3, from: eg.selfUser, to: [user2, user3]),
677+
eg.dmMessage(id: 4, from: eg.selfUser, to: []),
678+
eg.streamMessage(id: 5),
679+
]);
680+
checkHasMessageIds([1, 2, 3, 4, 5]);
681+
682+
await store.muteUser(user1.userId);
683+
checkNotifiedOnce();
684+
checkHasMessageIds([2, 3, 4, 5]);
685+
686+
await store.muteUser(user2.userId);
687+
checkNotifiedOnce();
688+
checkHasMessageIds([3, 4, 5]);
689+
});
690+
691+
test('MentionsNarrow', () async {
692+
await prepare(narrow: MentionsNarrow(), users: users);
693+
await prepareMessages(foundOldest: true, messages: [
694+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1],
695+
flags: [MessageFlag.mentioned]),
696+
eg.dmMessage(id: 2, from: eg.selfUser, to: [user2],
697+
flags: [MessageFlag.mentioned]),
698+
eg.streamMessage(id: 3, flags: [MessageFlag.mentioned]),
699+
]);
700+
checkHasMessageIds([1, 2, 3]);
701+
702+
await store.muteUser(user1.userId);
703+
checkNotifiedOnce();
704+
checkHasMessageIds([2, 3]);
705+
});
706+
707+
test('StarredMessagesNarrow', () async {
708+
await prepare(narrow: StarredMessagesNarrow(), users: users);
709+
await prepareMessages(foundOldest: true, messages: [
710+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1],
711+
flags: [MessageFlag.starred]),
712+
eg.dmMessage(id: 2, from: eg.selfUser, to: [user2],
713+
flags: [MessageFlag.starred]),
714+
eg.streamMessage(id: 3, flags: [MessageFlag.starred]),
715+
]);
716+
checkHasMessageIds([1, 2, 3]);
717+
718+
await store.muteUser(user1.userId);
719+
checkNotifiedOnce();
720+
checkHasMessageIds([2, 3]);
721+
});
722+
723+
test('ChannelNarrow -> do nothing', () async {
724+
await prepare(narrow: ChannelNarrow(eg.defaultStreamMessageStreamId), users: users);
725+
await prepareMessages(foundOldest: true, messages: [
726+
eg.streamMessage(id: 1),
727+
]);
728+
checkHasMessageIds([1]);
729+
730+
await store.muteUser(user1.userId);
731+
checkNotNotified();
732+
checkHasMessageIds([1]);
733+
});
734+
735+
test('TopicNarrow -> do nothing', () async {
736+
await prepare(narrow: TopicNarrow(eg.defaultStreamMessageStreamId,
737+
TopicName('topic')), users: users);
738+
await prepareMessages(foundOldest: true, messages: [
739+
eg.streamMessage(id: 1, topic: 'topic'),
740+
]);
741+
checkHasMessageIds([1]);
742+
743+
await store.muteUser(user1.userId);
744+
checkNotNotified();
745+
checkHasMessageIds([1]);
746+
});
747+
748+
test('DmNarrow -> do nothing', () async {
749+
await prepare(
750+
narrow: DmNarrow.withUser(user1.userId, selfUserId: eg.selfUser.userId),
751+
users: users);
752+
await prepareMessages(foundOldest: true, messages: [
753+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1]),
754+
]);
755+
checkHasMessageIds([1]);
756+
757+
await store.muteUser(user1.userId);
758+
checkNotNotified();
759+
checkHasMessageIds([1]);
760+
});
761+
762+
test('unmute a user -> refetch from scratch', () => awaitFakeAsync((async) async {
763+
await prepare(narrow: CombinedFeedNarrow(), users: users,
764+
mutedUserIds: [user1.userId]);
765+
final messages = <Message>[
766+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1]),
767+
eg.streamMessage(id: 2),
768+
];
769+
await prepareMessages(foundOldest: true, messages: messages);
770+
checkHasMessageIds([2]);
771+
772+
connection.prepare(
773+
json: newestResult(foundOldest: true, messages: messages).toJson());
774+
await store.unmuteUser(user1.userId);
775+
checkNotifiedOnce();
776+
check(model).fetched.isFalse();
777+
checkHasMessageIds([]);
778+
779+
async.elapse(Duration.zero);
780+
checkNotifiedOnce();
781+
checkHasMessageIds([1, 2]);
782+
}));
783+
784+
test('unmute a user before initial fetch completes -> do nothing', () => awaitFakeAsync((async) async {
785+
await prepare(narrow: CombinedFeedNarrow(), users: users,
786+
mutedUserIds: [user1.userId]);
787+
final messages = <Message>[
788+
eg.dmMessage(id: 1, from: eg.selfUser, to: [user1]),
789+
eg.streamMessage(id: 2),
790+
];
791+
connection.prepare(
792+
json: newestResult(foundOldest: true, messages: messages).toJson());
793+
final fetchFuture = model.fetchInitial();
794+
await store.unmuteUser(user1.userId);
795+
checkNotNotified();
796+
797+
await fetchFuture;
798+
checkNotifiedOnce();
799+
checkHasMessageIds([1, 2]);
800+
}));
801+
});
802+
659803
group('notifyListenersIfMessagePresent', () {
660804
test('message present', () async {
661805
await prepare(narrow: const CombinedFeedNarrow());

‎test/model/user_test.dart

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import 'package:checks/checks.dart';
22
import 'package:flutter_test/flutter_test.dart';
33
import 'package:zulip/api/model/events.dart';
44
import 'package:zulip/api/model/model.dart';
5+
import 'package:zulip/model/store.dart';
6+
import 'package:zulip/model/user.dart';
57

68
import '../api/model/model_checks.dart';
79
import '../example_data.dart' as eg;
@@ -53,6 +55,36 @@ void main() {
5355
});
5456
});
5557

58+
group('willChangeIfRecipientMuted', () {
59+
MutedUsersEvent mkEvent(List<int> userIds) =>
60+
eg.mutedUsersEvent(userIds);
61+
62+
void checkChanges(PerAccountStore store,
63+
List<int> userIds,
64+
MutenessEffect expected,
65+
) {
66+
check(store.willChangeIfRecipientMuted(mkEvent(userIds))).equals(expected);
67+
}
68+
69+
testWidgets('one muted user, event comes with two users -> added', (tester) async {
70+
final user1 = eg.user(userId: 1);
71+
final user2 = eg.user(userId: 2);
72+
final store = eg.store();
73+
await store.addUsers([user1, user2]);
74+
await store.muteUser(user1.userId);
75+
checkChanges(store, [user1.userId, user2.userId], MutenessEffect.added);
76+
});
77+
78+
testWidgets('two muted users, event comes with one user -> removed', (tester) async {
79+
final user1 = eg.user(userId: 1);
80+
final user2 = eg.user(userId: 2);
81+
final store = eg.store();
82+
await store.addUsers([user1, user2]);
83+
await store.muteUsers([user1.userId, user2.userId]);
84+
checkChanges(store, [user1.userId], MutenessEffect.removed);
85+
});
86+
});
87+
5688
group('RealmUserUpdateEvent', () {
5789
// TODO write more tests for handling RealmUserUpdateEvent
5890

0 commit comments

Comments
 (0)