Skip to content

Commit ce08cfe

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 cd90a72 commit ce08cfe

File tree

4 files changed

+256
-0
lines changed

4 files changed

+256
-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: 75 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.
@@ -388,6 +421,23 @@ class MessageListView with ChangeNotifier, _MessageSequence {
388421
}
389422
}
390423

424+
/// Whether this event could affect the result that [_messageVisible]
425+
/// would ever have returned for any possible message in this message list.
426+
VisibilityEffect _canAffectVisibility(UserTopicEvent event) {
427+
switch (narrow) {
428+
case CombinedFeedNarrow():
429+
return store.willChangeIfTopicVisible(event);
430+
431+
case StreamNarrow(:final streamId):
432+
if (event.streamId != streamId) return VisibilityEffect.none;
433+
return store.willChangeIfTopicVisibleInStream(event);
434+
435+
case TopicNarrow():
436+
case DmNarrow():
437+
return VisibilityEffect.none;
438+
}
439+
}
440+
391441
/// Whether [_messageVisible] is true for all possible messages.
392442
///
393443
/// This is useful for an optimization.
@@ -475,6 +525,31 @@ class MessageListView with ChangeNotifier, _MessageSequence {
475525
}
476526
}
477527

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

lib/model/store.dart

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

490490
case UserTopicEvent():
491491
assert(debugLog("server event: user_topic"));
492+
_messages.handleUserTopicEvent(event);
493+
// Update _channels last, so other handlers can compare to the old value.
492494
_channels.handleUserTopicEvent(event);
493495
notifyListeners();
494496

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: 'foo'),
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 StreamNarrow 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: 'foo'),
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 StreamNarrow, use stream visibility', () async {
382+
// Compare the parallel CombinedFeedNarrow test above.
383+
await prepare(narrow: StreamNarrow(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: 'foo'),
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, 'foo'));
406+
await prepareMutes();
407+
await prepareMessages(foundOldest: true, messages: [
408+
eg.streamMessage(id: 1, stream: stream, topic: 'foo'),
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: 'foo'),
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: 'foo'),
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)