Skip to content

Commit 1bc9a51

Browse files
chrisbobbesm-sayedi
authored andcommitted
msglist: In combined/mentions/starred, exclude DMs if all recipients muted
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 f351eae commit 1bc9a51

File tree

7 files changed

+359
-51
lines changed

7 files changed

+359
-51
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: 60 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

@@ -658,10 +659,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
658659
bool _messageVisible(MessageBase message) {
659660
switch (narrow) {
660661
case CombinedFeedNarrow():
661-
return switch (message.conversation) {
662+
final conversation = message.conversation;
663+
return switch (conversation) {
662664
StreamConversation(:final streamId, :final topic) =>
663665
store.isTopicVisible(streamId, topic),
664-
DmConversation() => true,
666+
DmConversation() => !store.shouldMuteDmConversation(
667+
DmNarrow.ofConversation(conversation, selfUserId: store.selfUserId)),
665668
};
666669

667670
case ChannelNarrow(:final streamId):
@@ -672,8 +675,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
672675

673676
case TopicNarrow():
674677
case DmNarrow():
678+
return true;
679+
675680
case MentionsNarrow():
676681
case StarredMessagesNarrow():
682+
if (message.conversation case DmConversation(:final allRecipientIds)) {
683+
return !store.shouldMuteDmConversation(DmNarrow(
684+
allRecipientIds: allRecipientIds, selfUserId: store.selfUserId));
685+
}
677686
return true;
678687
}
679688
}
@@ -689,9 +698,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
689698

690699
case TopicNarrow():
691700
case DmNarrow():
701+
return true;
702+
692703
case MentionsNarrow():
693704
case StarredMessagesNarrow():
694-
return true;
705+
return false;
695706
}
696707
}
697708

@@ -714,6 +725,24 @@ class MessageListView with ChangeNotifier, _MessageSequence {
714725
}
715726
}
716727

728+
/// Whether this event could affect the result that [_messageVisible]
729+
/// would ever have returned for any possible message in this message list.
730+
MutedUsersVisibilityEffect _mutedUsersEventCanAffectVisibility(MutedUsersEvent event) {
731+
switch(narrow) {
732+
case CombinedFeedNarrow():
733+
return store.mightChangeShouldMuteDmConversation(event);
734+
735+
case ChannelNarrow():
736+
case TopicNarrow():
737+
case DmNarrow():
738+
return MutedUsersVisibilityEffect.none;
739+
740+
case MentionsNarrow():
741+
case StarredMessagesNarrow():
742+
return store.mightChangeShouldMuteDmConversation(event);
743+
}
744+
}
745+
717746
void _setStatus(FetchingStatus value, {FetchingStatus? was}) {
718747
assert(was == null || _status == was);
719748
_status = value;
@@ -1017,6 +1046,34 @@ class MessageListView with ChangeNotifier, _MessageSequence {
10171046
}
10181047
}
10191048

1049+
void handleMutedUsersEvent(MutedUsersEvent event) {
1050+
switch (_mutedUsersEventCanAffectVisibility(event)) {
1051+
case MutedUsersVisibilityEffect.none:
1052+
return;
1053+
1054+
case MutedUsersVisibilityEffect.muted:
1055+
final anyRemoved = _removeMessagesWhere((message) {
1056+
if (message is! DmMessage) return false;
1057+
final narrow = DmNarrow.ofMessage(message, selfUserId: store.selfUserId);
1058+
return store.shouldMuteDmConversation(narrow, event: event);
1059+
});
1060+
if (anyRemoved) {
1061+
notifyListeners();
1062+
}
1063+
1064+
case MutedUsersVisibilityEffect.mixed:
1065+
case MutedUsersVisibilityEffect.unmuted:
1066+
// TODO get the newly-unmuted messages from the message store
1067+
// For now, we simplify the task by just refetching this message list
1068+
// from scratch.
1069+
if (fetched) {
1070+
_reset();
1071+
notifyListeners();
1072+
fetchInitial();
1073+
}
1074+
}
1075+
}
1076+
10201077
void handleDeleteMessageEvent(DeleteMessageEvent event) {
10211078
if (_removeMessagesById(event.messageIds)) {
10221079
notifyListeners();

lib/model/narrow.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,21 @@ class DmNarrow extends Narrow implements SendableNarrow {
200200
required int selfUserId,
201201
}) {
202202
return DmNarrow(
203+
// TODO should this really be making a copy of `allRecipientIds`?
203204
allRecipientIds: List.unmodifiable(message.conversation.allRecipientIds),
204205
selfUserId: selfUserId,
205206
);
206207
}
207208

209+
factory DmNarrow.ofConversation(DmConversation conversation, {
210+
required int selfUserId,
211+
}) {
212+
return DmNarrow(
213+
allRecipientIds: conversation.allRecipientIds,
214+
selfUserId: selfUserId,
215+
);
216+
}
217+
208218
/// A [DmNarrow] from an item in [InitialSnapshot.recentPrivateConversations].
209219
factory DmNarrow.ofRecentDmConversation(RecentDmConversation conversation, {required int selfUserId}) {
210220
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)