Skip to content

Commit 3d37653

Browse files
committed
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
1 parent 8e0a840 commit 3d37653

File tree

4 files changed

+257
-0
lines changed

4 files changed

+257
-0
lines changed

lib/model/message.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ class MessageStoreImpl with MessageStore {
8282
}
8383
}
8484

85+
void handleUserTopicEvent(UserTopicEvent event) {
86+
for (final view in _messageListViews) {
87+
view.handleUserTopicEvent(event);
88+
}
89+
}
90+
8591
void handleMessageEvent(MessageEvent event) {
8692
// If the message is one we already know about (from a fetch),
8793
// clobber it with the one from the event system.

lib/model/message_list.dart

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import '../api/model/events.dart';
55
import '../api/model/model.dart';
66
import '../api/route/messages.dart';
77
import 'algorithms.dart';
8+
import 'channel.dart';
89
import 'content.dart';
910
import 'narrow.dart';
1011
import 'store.dart';
@@ -158,6 +159,38 @@ mixin _MessageSequence {
158159
_processMessage(messages.length - 1);
159160
}
160161

162+
/// Removes all messages from the list that satisfy [test].
163+
///
164+
/// Returns true if any messages were removed, false otherwise.
165+
bool _removeMessagesWhere(bool Function(Message) test) {
166+
// Before we find a message to remove, there's no need to copy elements.
167+
// This is like the loop below, but simplified for `target == candidate`.
168+
int candidate = 0;
169+
while (true) {
170+
if (candidate == messages.length) return false;
171+
if (test(messages[candidate])) break;
172+
candidate++;
173+
}
174+
175+
int target = candidate;
176+
candidate++;
177+
assert(contents.length == messages.length);
178+
while (candidate < messages.length) {
179+
if (test(messages[candidate])) {
180+
candidate++;
181+
continue;
182+
}
183+
messages[target] = messages[candidate];
184+
contents[target] = contents[candidate];
185+
target++; candidate++;
186+
}
187+
messages.length = target;
188+
contents.length = target;
189+
assert(contents.length == messages.length);
190+
_reprocessAll();
191+
return true;
192+
}
193+
161194
/// Removes the given messages, if present.
162195
///
163196
/// Returns true if at least one message was present, false otherwise.
@@ -389,6 +422,24 @@ class MessageListView with ChangeNotifier, _MessageSequence {
389422
}
390423
}
391424

425+
/// Whether this event could affect the result that [_messageVisible]
426+
/// would ever have returned for any possible message in this message list.
427+
VisibilityEffect _canAffectVisibility(UserTopicEvent event) {
428+
switch (narrow) {
429+
case CombinedFeedNarrow():
430+
return store.willChangeIfTopicVisible(event);
431+
432+
case ChannelNarrow(:final streamId):
433+
if (event.streamId != streamId) return VisibilityEffect.none;
434+
return store.willChangeIfTopicVisibleInStream(event);
435+
436+
case TopicNarrow():
437+
case DmNarrow():
438+
case MentionsNarrow():
439+
return VisibilityEffect.none;
440+
}
441+
}
442+
392443
/// Whether [_messageVisible] is true for all possible messages.
393444
///
394445
/// This is useful for an optimization.
@@ -477,6 +528,31 @@ class MessageListView with ChangeNotifier, _MessageSequence {
477528
}
478529
}
479530

531+
void handleUserTopicEvent(UserTopicEvent event) {
532+
switch (_canAffectVisibility(event)) {
533+
case VisibilityEffect.none:
534+
return;
535+
536+
case VisibilityEffect.muted:
537+
if (_removeMessagesWhere((message) =>
538+
(message is StreamMessage
539+
&& message.streamId == event.streamId
540+
&& message.topic == event.topicName))) {
541+
notifyListeners();
542+
}
543+
544+
case VisibilityEffect.unmuted:
545+
// TODO get the newly-unmuted messages from the message store
546+
// For now, we simplify the task by just refetching this message list
547+
// from scratch.
548+
if (fetched) {
549+
_reset();
550+
notifyListeners();
551+
fetchInitial();
552+
}
553+
}
554+
}
555+
480556
void handleDeleteMessageEvent(DeleteMessageEvent event) {
481557
if (_removeMessagesById(event.messageIds)) {
482558
notifyListeners();

lib/model/store.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,8 @@ class PerAccountStore extends ChangeNotifier with ChannelStore, MessageStore {
494494

495495
case UserTopicEvent():
496496
assert(debugLog("server event: user_topic"));
497+
_messages.handleUserTopicEvent(event);
498+
// Update _channels last, so other handlers can compare to the old value.
497499
_channels.handleUserTopicEvent(event);
498500
notifyListeners();
499501

test/model/message_list_test.dart

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,179 @@ void main() {
311311
check(model).fetched.isFalse();
312312
});
313313

314+
group('UserTopicEvent', () {
315+
// The ChannelStore.willChangeIfTopicVisible/InStream methods have their own
316+
// thorough unit tests. So these tests focus on the rest of the logic.
317+
318+
final stream = eg.stream();
319+
const String topic = 'foo';
320+
321+
Future<void> setVisibility(UserTopicVisibilityPolicy policy) async {
322+
await store.handleEvent(eg.userTopicEvent(stream.streamId, topic, policy));
323+
}
324+
325+
/// (Should run after `prepare`.)
326+
Future<void> prepareMutes([
327+
bool streamMuted = false,
328+
UserTopicVisibilityPolicy policy = UserTopicVisibilityPolicy.none,
329+
]) async {
330+
await store.addStream(stream);
331+
await store.addSubscription(eg.subscription(stream, isMuted: streamMuted));
332+
await setVisibility(policy);
333+
}
334+
335+
void checkHasMessageIds(Iterable<int> messageIds) {
336+
check(model.messages.map((m) => m.id)).deepEquals(messageIds);
337+
}
338+
339+
test('mute a visible topic', () async {
340+
await prepare(narrow: const CombinedFeedNarrow());
341+
await prepareMutes();
342+
final otherStream = eg.stream();
343+
await store.addStream(otherStream);
344+
await store.addSubscription(eg.subscription(otherStream));
345+
await prepareMessages(foundOldest: true, messages: [
346+
eg.streamMessage(id: 1, stream: stream, topic: 'bar'),
347+
eg.streamMessage(id: 2, stream: stream, topic: topic),
348+
eg.streamMessage(id: 3, stream: otherStream, topic: 'elsewhere'),
349+
eg.dmMessage( id: 4, from: eg.otherUser, to: [eg.selfUser]),
350+
]);
351+
checkHasMessageIds([1, 2, 3, 4]);
352+
353+
await setVisibility(UserTopicVisibilityPolicy.muted);
354+
checkNotifiedOnce();
355+
checkHasMessageIds([1, 3, 4]);
356+
});
357+
358+
test('in CombinedFeedNarrow, use combined-feed visibility', () async {
359+
// Compare the parallel ChannelNarrow test below.
360+
await prepare(narrow: const CombinedFeedNarrow());
361+
// Mute the stream, so that combined-feed vs. stream visibility differ.
362+
await prepareMutes(true, UserTopicVisibilityPolicy.followed);
363+
await prepareMessages(foundOldest: true, messages: [
364+
eg.streamMessage(id: 1, stream: stream, topic: topic),
365+
]);
366+
checkHasMessageIds([1]);
367+
368+
// Dropping from followed to none hides the message
369+
// (whereas it'd have no effect in a stream narrow).
370+
await setVisibility(UserTopicVisibilityPolicy.none);
371+
checkNotifiedOnce();
372+
checkHasMessageIds([]);
373+
374+
// Dropping from none to muted has no further effect
375+
// (whereas it'd hide the message in a stream narrow).
376+
await setVisibility(UserTopicVisibilityPolicy.muted);
377+
checkNotNotified();
378+
checkHasMessageIds([]);
379+
});
380+
381+
test('in ChannelNarrow, use stream visibility', () async {
382+
// Compare the parallel CombinedFeedNarrow test above.
383+
await prepare(narrow: ChannelNarrow(stream.streamId));
384+
// Mute the stream, so that combined-feed vs. stream visibility differ.
385+
await prepareMutes(true, UserTopicVisibilityPolicy.followed);
386+
await prepareMessages(foundOldest: true, messages: [
387+
eg.streamMessage(id: 1, stream: stream, topic: topic),
388+
]);
389+
checkHasMessageIds([1]);
390+
391+
// Dropping from followed to none has no effect
392+
// (whereas it'd hide the message in the combined feed).
393+
await setVisibility(UserTopicVisibilityPolicy.none);
394+
checkNotNotified();
395+
checkHasMessageIds([1]);
396+
397+
// Dropping from none to muted hides the message
398+
// (whereas it'd have no effect in a stream narrow).
399+
await setVisibility(UserTopicVisibilityPolicy.muted);
400+
checkNotifiedOnce();
401+
checkHasMessageIds([]);
402+
});
403+
404+
test('in TopicNarrow, stay visible', () async {
405+
await prepare(narrow: TopicNarrow(stream.streamId, topic));
406+
await prepareMutes();
407+
await prepareMessages(foundOldest: true, messages: [
408+
eg.streamMessage(id: 1, stream: stream, topic: topic),
409+
]);
410+
checkHasMessageIds([1]);
411+
412+
await setVisibility(UserTopicVisibilityPolicy.muted);
413+
checkNotNotified();
414+
checkHasMessageIds([1]);
415+
});
416+
417+
test('in DmNarrow, do nothing (smoke test)', () async {
418+
await prepare(narrow:
419+
DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId));
420+
await prepareMutes();
421+
await prepareMessages(foundOldest: true, messages: [
422+
eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]),
423+
]);
424+
checkHasMessageIds([1]);
425+
426+
await setVisibility(UserTopicVisibilityPolicy.muted);
427+
checkNotNotified();
428+
checkHasMessageIds([1]);
429+
});
430+
431+
test('no affected messages -> no notification', () async {
432+
await prepare(narrow: const CombinedFeedNarrow());
433+
await prepareMutes();
434+
await prepareMessages(foundOldest: true, messages: [
435+
eg.streamMessage(id: 1, stream: stream, topic: 'bar'),
436+
]);
437+
checkHasMessageIds([1]);
438+
439+
await setVisibility(UserTopicVisibilityPolicy.muted);
440+
checkNotNotified();
441+
checkHasMessageIds([1]);
442+
});
443+
444+
test('unmute a topic -> refetch from scratch', () => awaitFakeAsync((async) async {
445+
await prepare(narrow: const CombinedFeedNarrow());
446+
await prepareMutes(true);
447+
final messages = [
448+
eg.dmMessage(id: 1, from: eg.otherUser, to: [eg.selfUser]),
449+
eg.streamMessage(id: 2, stream: stream, topic: topic),
450+
];
451+
await prepareMessages(foundOldest: true, messages: messages);
452+
checkHasMessageIds([1]);
453+
454+
connection.prepare(
455+
json: newestResult(foundOldest: true, messages: messages).toJson());
456+
await setVisibility(UserTopicVisibilityPolicy.unmuted);
457+
checkNotifiedOnce();
458+
check(model).fetched.isFalse();
459+
checkHasMessageIds([]);
460+
461+
async.elapse(Duration.zero);
462+
checkNotifiedOnce();
463+
checkHasMessageIds([1, 2]);
464+
}));
465+
466+
test('unmute a topic before initial fetch completes -> do nothing', () => awaitFakeAsync((async) async {
467+
await prepare(narrow: const CombinedFeedNarrow());
468+
await prepareMutes(true);
469+
final messages = [
470+
eg.streamMessage(id: 1, stream: stream, topic: topic),
471+
];
472+
473+
connection.prepare(
474+
json: newestResult(foundOldest: true, messages: messages).toJson());
475+
final fetchFuture = model.fetchInitial();
476+
477+
await setVisibility(UserTopicVisibilityPolicy.unmuted);
478+
checkNotNotified();
479+
480+
// The new policy does get applied when the fetch eventually completes.
481+
await fetchFuture;
482+
checkNotifiedOnce();
483+
checkHasMessageIds([1]);
484+
}));
485+
});
486+
314487
group('DeleteMessageEvent', () {
315488
final stream = eg.stream();
316489
final messages = List.generate(30, (i) => eg.streamMessage(stream: stream));

0 commit comments

Comments
 (0)