Skip to content

Commit 2ef9d7b

Browse files
chrisbobbesm-sayedi
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 e43b93b commit 2ef9d7b

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
@@ -287,6 +287,12 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
287287
}
288288
}
289289

290+
void handleMutedUsersEvent(MutedUsersEvent event) {
291+
for (final view in _messageListViews) {
292+
view.handleMutedUsersEvent(event);
293+
}
294+
}
295+
290296
void handleMessageEvent(MessageEvent event) {
291297
// If the message is one we already know about (from a fetch),
292298
// 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

@@ -622,10 +623,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
622623
bool _messageVisible(MessageBase message) {
623624
switch (narrow) {
624625
case CombinedFeedNarrow():
625-
return switch (message.conversation) {
626+
final conversation = message.conversation;
627+
return switch (conversation) {
626628
StreamConversation(:final streamId, :final topic) =>
627629
store.isTopicVisible(streamId, topic),
628-
DmConversation() => true,
630+
DmConversation() => !store.shouldMuteDmConversation(
631+
DmNarrow.ofConversation(conversation, selfUserId: store.selfUserId)),
629632
};
630633

631634
case ChannelNarrow(:final streamId):
@@ -636,8 +639,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
636639

637640
case TopicNarrow():
638641
case DmNarrow():
642+
return true;
643+
639644
case MentionsNarrow():
640645
case StarredMessagesNarrow():
646+
if (message.conversation case DmConversation(:final allRecipientIds)) {
647+
return !store.shouldMuteDmConversation(DmNarrow(
648+
allRecipientIds: allRecipientIds, selfUserId: store.selfUserId));
649+
}
641650
return true;
642651
}
643652
}
@@ -653,9 +662,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
653662

654663
case TopicNarrow():
655664
case DmNarrow():
665+
return true;
666+
656667
case MentionsNarrow():
657668
case StarredMessagesNarrow():
658-
return true;
669+
return false;
659670
}
660671
}
661672

@@ -678,6 +689,24 @@ class MessageListView with ChangeNotifier, _MessageSequence {
678689
}
679690
}
680691

692+
/// Whether this event could affect the result that [_messageVisible]
693+
/// would ever have returned for any possible message in this message list.
694+
MutedUsersVisibilityEffect _mutedUsersEventCanAffectVisibility(MutedUsersEvent event) {
695+
switch(narrow) {
696+
case CombinedFeedNarrow():
697+
return store.mightChangeShouldMuteDmConversation(event);
698+
699+
case ChannelNarrow():
700+
case TopicNarrow():
701+
case DmNarrow():
702+
return MutedUsersVisibilityEffect.none;
703+
704+
case MentionsNarrow():
705+
case StarredMessagesNarrow():
706+
return store.mightChangeShouldMuteDmConversation(event);
707+
}
708+
}
709+
681710
void _setStatus(FetchingStatus value, {FetchingStatus? was}) {
682711
assert(was == null || _status == was);
683712
_status = value;
@@ -981,6 +1010,34 @@ class MessageListView with ChangeNotifier, _MessageSequence {
9811010
}
9821011
}
9831012

1013+
void handleMutedUsersEvent(MutedUsersEvent event) {
1014+
switch (_mutedUsersEventCanAffectVisibility(event)) {
1015+
case MutedUsersVisibilityEffect.none:
1016+
return;
1017+
1018+
case MutedUsersVisibilityEffect.muted:
1019+
final anyRemoved = _removeMessagesWhere((message) {
1020+
if (message is! DmMessage) return false;
1021+
final narrow = DmNarrow.ofMessage(message, selfUserId: store.selfUserId);
1022+
return store.shouldMuteDmConversation(narrow, event: event);
1023+
});
1024+
if (anyRemoved) {
1025+
notifyListeners();
1026+
}
1027+
1028+
case MutedUsersVisibilityEffect.mixed:
1029+
case MutedUsersVisibilityEffect.unmuted:
1030+
// TODO get the newly-unmuted messages from the message store
1031+
// For now, we simplify the task by just refetching this message list
1032+
// from scratch.
1033+
if (fetched) {
1034+
_reset();
1035+
notifyListeners();
1036+
fetchInitial();
1037+
}
1038+
}
1039+
}
1040+
9841041
void handleDeleteMessageEvent(DeleteMessageEvent event) {
9851042
if (_removeMessagesById(event.messageIds)) {
9861043
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
@@ -649,6 +649,10 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
649649
bool isUserMuted(int userId, {MutedUsersEvent? event}) =>
650650
_users.isUserMuted(userId, event: event);
651651

652+
@override
653+
MutedUsersVisibilityEffect mightChangeShouldMuteDmConversation(MutedUsersEvent event) =>
654+
_users.mightChangeShouldMuteDmConversation(event);
655+
652656
final UserStoreImpl _users;
653657

654658
final TypingStatus typingStatus;
@@ -958,6 +962,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
958962

959963
case MutedUsersEvent():
960964
assert(debugLog("server event: muted_users"));
965+
_messages.handleMutedUsersEvent(event);
966+
// Update _users last, so other handlers can compare to the old value.
961967
_users.handleMutedUsersEvent(event);
962968
notifyListeners();
963969

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)