Skip to content

Commit f7aff19

Browse files
chrisbobbesm-sayedi
andcommitted
msglist: Exclude DMs if recipients muted (combined/mentions/starred/search)
In the future, this should apply to the ReactionsNarrow from the Web app too, once we have it. Co-authored-by: Sayed Mahmood Sayedi <[email protected]>
1 parent ca03731 commit f7aff19

File tree

7 files changed

+362
-52
lines changed

7 files changed

+362
-52
lines changed

lib/model/message.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
351351
}
352352
}
353353

354+
void handleMutedUsersEvent(MutedUsersEvent event) {
355+
for (final view in _messageListViews) {
356+
view.handleMutedUsersEvent(event);
357+
}
358+
}
359+
354360
void handleMessageEvent(MessageEvent event) {
355361
// If the message is one we already know about (from a fetch),
356362
// clobber it with the one from the event system.

lib/model/message_list.dart

Lines changed: 61 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
export '../api/route/messages.dart' show Anchor, AnchorCode, NumericAnchor;
1819

@@ -668,10 +669,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
668669
bool _messageVisible(MessageBase message) {
669670
switch (narrow) {
670671
case CombinedFeedNarrow():
671-
return switch (message.conversation) {
672+
final conversation = message.conversation;
673+
return switch (conversation) {
672674
StreamConversation(:final streamId, :final topic) =>
673675
store.isTopicVisible(streamId, topic),
674-
DmConversation() => true,
676+
DmConversation() => !store.shouldMuteDmConversation(
677+
DmNarrow.ofConversation(conversation, selfUserId: store.selfUserId)),
675678
};
676679

677680
case ChannelNarrow(:final streamId):
@@ -682,9 +685,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
682685

683686
case TopicNarrow():
684687
case DmNarrow():
688+
return true;
689+
685690
case MentionsNarrow():
686691
case StarredMessagesNarrow():
687692
case KeywordSearchNarrow():
693+
if (message.conversation case DmConversation(:final allRecipientIds)) {
694+
return !store.shouldMuteDmConversation(DmNarrow(
695+
allRecipientIds: allRecipientIds, selfUserId: store.selfUserId));
696+
}
688697
return true;
689698
}
690699
}
@@ -700,10 +709,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
700709

701710
case TopicNarrow():
702711
case DmNarrow():
712+
return true;
713+
703714
case MentionsNarrow():
704715
case StarredMessagesNarrow():
705716
case KeywordSearchNarrow():
706-
return true;
717+
return false;
707718
}
708719
}
709720

@@ -727,6 +738,25 @@ class MessageListView with ChangeNotifier, _MessageSequence {
727738
}
728739
}
729740

741+
/// Whether this event could affect the result that [_messageVisible]
742+
/// would ever have returned for any possible message in this message list.
743+
MutedUsersVisibilityEffect _mutedUsersEventCanAffectVisibility(MutedUsersEvent event) {
744+
switch(narrow) {
745+
case CombinedFeedNarrow():
746+
return store.mightChangeShouldMuteDmConversation(event);
747+
748+
case ChannelNarrow():
749+
case TopicNarrow():
750+
case DmNarrow():
751+
return MutedUsersVisibilityEffect.none;
752+
753+
case MentionsNarrow():
754+
case StarredMessagesNarrow():
755+
case KeywordSearchNarrow():
756+
return store.mightChangeShouldMuteDmConversation(event);
757+
}
758+
}
759+
730760
void _setStatus(FetchingStatus value, {FetchingStatus? was}) {
731761
assert(was == null || _status == was);
732762
_status = value;
@@ -1041,6 +1071,34 @@ class MessageListView with ChangeNotifier, _MessageSequence {
10411071
}
10421072
}
10431073

1074+
void handleMutedUsersEvent(MutedUsersEvent event) {
1075+
switch (_mutedUsersEventCanAffectVisibility(event)) {
1076+
case MutedUsersVisibilityEffect.none:
1077+
return;
1078+
1079+
case MutedUsersVisibilityEffect.muted:
1080+
final anyRemoved = _removeMessagesWhere((message) {
1081+
if (message is! DmMessage) return false;
1082+
final narrow = DmNarrow.ofMessage(message, selfUserId: store.selfUserId);
1083+
return store.shouldMuteDmConversation(narrow, event: event);
1084+
});
1085+
if (anyRemoved) {
1086+
notifyListeners();
1087+
}
1088+
1089+
case MutedUsersVisibilityEffect.mixed:
1090+
case MutedUsersVisibilityEffect.unmuted:
1091+
// TODO get the newly-unmuted messages from the message store
1092+
// For now, we simplify the task by just refetching this message list
1093+
// from scratch.
1094+
if (fetched) {
1095+
_reset();
1096+
notifyListeners();
1097+
fetchInitial();
1098+
}
1099+
}
1100+
}
1101+
10441102
void handleDeleteMessageEvent(DeleteMessageEvent event) {
10451103
if (_removeMessagesById(event.messageIds)) {
10461104
notifyListeners();

lib/model/narrow.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,21 @@ class DmNarrow extends Narrow implements SendableNarrow {
203203
required int selfUserId,
204204
}) {
205205
return DmNarrow(
206+
// TODO should this really be making a copy of `allRecipientIds`?
206207
allRecipientIds: List.unmodifiable(message.conversation.allRecipientIds),
207208
selfUserId: selfUserId,
208209
);
209210
}
210211

212+
factory DmNarrow.ofConversation(DmConversation conversation, {
213+
required int selfUserId,
214+
}) {
215+
return DmNarrow(
216+
allRecipientIds: conversation.allRecipientIds,
217+
selfUserId: selfUserId,
218+
);
219+
}
220+
211221
/// A [DmNarrow] from an item in [InitialSnapshot.recentPrivateConversations].
212222
factory DmNarrow.ofRecentDmConversation(RecentDmConversation conversation, {required int selfUserId}) {
213223
return DmNarrow.withOtherUsers(conversation.userIds, selfUserId: selfUserId);

lib/model/store.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,10 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
665665
bool isUserMuted(int userId, {MutedUsersEvent? event}) =>
666666
_users.isUserMuted(userId, event: event);
667667

668+
@override
669+
MutedUsersVisibilityEffect mightChangeShouldMuteDmConversation(MutedUsersEvent event) =>
670+
_users.mightChangeShouldMuteDmConversation(event);
671+
668672
final UserStoreImpl _users;
669673

670674
final TypingStatus typingStatus;
@@ -983,6 +987,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
983987

984988
case MutedUsersEvent():
985989
assert(debugLog("server event: muted_users"));
990+
_messages.handleMutedUsersEvent(event);
991+
// Update _users last, so other handlers can compare to the old value.
986992
_users.handleMutedUsersEvent(event);
987993
notifyListeners();
988994

lib/model/user.dart

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import '../api/model/events.dart';
22
import '../api/model/initial_snapshot.dart';
33
import '../api/model/model.dart';
4+
import 'algorithms.dart';
45
import 'localizations.dart';
56
import 'narrow.dart';
67
import 'store.dart';
@@ -97,6 +98,27 @@ mixin UserStore on PerAccountStoreBase {
9798
return narrow.otherRecipientIds.every(
9899
(userId) => isUserMuted(userId, event: event));
99100
}
101+
102+
/// Whether the given event might change the result of [shouldMuteDmConversation]
103+
/// for its list of muted users, compared to the current state.
104+
MutedUsersVisibilityEffect mightChangeShouldMuteDmConversation(MutedUsersEvent event);
105+
}
106+
107+
/// Whether and how a given [MutedUsersEvent] may affect the results
108+
/// that [UserStore.shouldMuteDmConversation] would give for some messages.
109+
enum MutedUsersVisibilityEffect {
110+
/// The event will have no effect on the visibility results.
111+
none,
112+
113+
/// The event may change some visibility results from true to false.
114+
muted,
115+
116+
/// The event may change some visibility results from false to true.
117+
unmuted,
118+
119+
/// The event may change some visibility results from false to true,
120+
/// and some from true to false.
121+
mixed;
100122
}
101123

102124
/// The implementation of [UserStore] that does the work.
@@ -130,6 +152,29 @@ class UserStoreImpl extends PerAccountStoreBase with UserStore {
130152
return (event?.mutedUsers.map((item) => item.id) ?? _mutedUsers).contains(userId);
131153
}
132154

155+
@override
156+
MutedUsersVisibilityEffect mightChangeShouldMuteDmConversation(MutedUsersEvent event) {
157+
final sortedOld = _mutedUsers.toList()..sort();
158+
final sortedNew = event.mutedUsers.map((u) => u.id).toList()..sort();
159+
assert(isSortedWithoutDuplicates(sortedOld));
160+
assert(isSortedWithoutDuplicates(sortedNew));
161+
final union = setUnion(sortedOld, sortedNew);
162+
163+
final willMuteSome = sortedOld.length < union.length;
164+
final willUnmuteSome = sortedNew.length < union.length;
165+
166+
switch ((willUnmuteSome, willMuteSome)) {
167+
case (true, false):
168+
return MutedUsersVisibilityEffect.unmuted;
169+
case (false, true):
170+
return MutedUsersVisibilityEffect.muted;
171+
case (true, true):
172+
return MutedUsersVisibilityEffect.mixed;
173+
case (false, false): // TODO(log)?
174+
return MutedUsersVisibilityEffect.none;
175+
}
176+
}
177+
133178
void handleRealmUserEvent(RealmUserEvent event) {
134179
switch (event) {
135180
case RealmUserAddEvent():

0 commit comments

Comments
 (0)