From 0d03c8ec7be0a5b7825a985a13c7e108354a2932 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 21 Nov 2023 22:24:43 -0800 Subject: [PATCH] unread: Apply stream and topic muting to unread counts --- lib/model/unreads.dart | 46 +++++++++++++++++++----- lib/widgets/subscription_list.dart | 3 +- test/model/unreads_test.dart | 27 +++++++++++--- test/widgets/message_list_test.dart | 5 +-- test/widgets/subscription_list_test.dart | 23 ++++++++++++ 5 files changed, 89 insertions(+), 15 deletions(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index cd68acc668..d8485c3e0b 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -132,34 +132,64 @@ class Unreads extends ChangeNotifier { final int selfUserId; - // TODO(#346): account for muted topics and streams // TODO(#370): maintain this count incrementally, rather than recomputing from scratch int countInAllMessagesNarrow() { int c = 0; for (final messageIds in dms.values) { c = c + messageIds.length; } - for (final topics in streams.values) { - for (final messageIds in topics.values) { - c = c + messageIds.length; + for (final MapEntry(key: streamId, value: topics) in streams.entries) { + for (final MapEntry(key: topic, value: messageIds) in topics.entries) { + if (streamStore.isTopicVisible(streamId, topic)) { + c = c + messageIds.length; + } + } + } + return c; + } + + /// The "strict" unread count for this stream, + /// using [StreamStore.isTopicVisible]. + /// + /// If the stream is muted, this will count only topics that are + /// actively unmuted. + /// + /// For a count that's appropriate in UI contexts that are focused + /// specifically on this stream, see [countInStreamNarrow]. + // TODO(#370): maintain this count incrementally, rather than recomputing from scratch + int countInStream(int streamId) { + final topics = streams[streamId]; + if (topics == null) return 0; + int c = 0; + for (final entry in topics.entries) { + if (streamStore.isTopicVisible(streamId, entry.key)) { + c = c + entry.value.length; } } return c; } - // TODO(#346): account for muted topics and streams + /// The "broad" unread count for this stream, + /// using [StreamStore.isTopicVisibleInStream]. + /// + /// This includes topics that have no visibility policy of their own, + /// even if the stream itself is muted. + /// + /// For a count that's appropriate in UI contexts that are not already + /// focused on this stream, see [countInStream]. // TODO(#370): maintain this count incrementally, rather than recomputing from scratch int countInStreamNarrow(int streamId) { final topics = streams[streamId]; if (topics == null) return 0; int c = 0; - for (final messageIds in topics.values) { - c = c + messageIds.length; + for (final entry in topics.entries) { + if (streamStore.isTopicVisibleInStream(streamId, entry.key)) { + c = c + entry.value.length; + } } return c; } - // TODO(#346): account for muted topics and streams int countInTopicNarrow(int streamId, String topic) { final topics = streams[streamId]; return topics?[topic]?.length ?? 0; diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index e055c31f12..935da57280 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -176,7 +176,8 @@ class _SubscriptionList extends StatelessWidget { itemCount: subscriptions.length, itemBuilder: (BuildContext context, int index) { final subscription = subscriptions[index]; - final unreadCount = unreadsModel!.countInStreamNarrow(subscription.streamId); + final unreadCount = unreadsModel!.countInStream(subscription.streamId); + // TODO(#346): if stream muted, show a dot for unreads return SubscriptionItem(subscription: subscription, unreadCount: unreadCount); }); } diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 555235185e..2725af0649 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -9,6 +9,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/model/unreads.dart'; import '../example_data.dart' as eg; +import 'test_store.dart'; import 'unreads_checks.dart'; void main() { @@ -152,30 +153,48 @@ void main() { group('count helpers', () { test('countInAllMessagesNarrow', () { - final stream1 = eg.stream(); - final stream2 = eg.stream(); + final stream1 = eg.stream(streamId: 1, name: 'stream 1'); + final stream2 = eg.stream(streamId: 2, name: 'stream 2'); + final stream3 = eg.stream(streamId: 3, name: 'stream 3'); prepare(); + streamStore.addStreams([stream1, stream2, stream3]); + streamStore.addSubscription(eg.subscription(stream1)); + streamStore.addSubscription(eg.subscription(stream2)); + streamStore.addSubscription(eg.subscription(stream3, isMuted: true)); + streamStore.addUserTopic(stream1, 'a', UserTopicVisibilityPolicy.muted); fillWithMessages([ eg.streamMessage(stream: stream1, topic: 'a', flags: []), eg.streamMessage(stream: stream1, topic: 'b', flags: []), eg.streamMessage(stream: stream1, topic: 'b', flags: []), eg.streamMessage(stream: stream2, topic: 'c', flags: []), + eg.streamMessage(stream: stream3, topic: 'd', flags: []), eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []), eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []), ]); - check(model.countInAllMessagesNarrow()).equals(6); + check(model.countInAllMessagesNarrow()).equals(5); }); - test('countInStreamNarrow', () { + test('countInStream/Narrow', () { final stream = eg.stream(); prepare(); + streamStore.addStream(stream); + streamStore.addSubscription(eg.subscription(stream)); + streamStore.addUserTopic(stream, 'a', UserTopicVisibilityPolicy.unmuted); + streamStore.addUserTopic(stream, 'c', UserTopicVisibilityPolicy.muted); fillWithMessages([ eg.streamMessage(stream: stream, topic: 'a', flags: []), eg.streamMessage(stream: stream, topic: 'a', flags: []), eg.streamMessage(stream: stream, topic: 'b', flags: []), eg.streamMessage(stream: stream, topic: 'b', flags: []), eg.streamMessage(stream: stream, topic: 'b', flags: []), + eg.streamMessage(stream: stream, topic: 'c', flags: []), ]); + check(model.countInStream (stream.streamId)).equals(5); + check(model.countInStreamNarrow(stream.streamId)).equals(5); + + streamStore.handleEvent(SubscriptionUpdateEvent(id: 1, streamId: stream.streamId, + property: SubscriptionProperty.isMuted, value: true)); + check(model.countInStream (stream.streamId)).equals(2); check(model.countInStreamNarrow(stream.streamId)).equals(5); }); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 40403f5daf..8a30707a4a 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -47,7 +47,7 @@ void main() { UnreadMessagesSnapshot? unreadMsgs, }) async { addTearDown(testBinding.reset); - streams ??= subscriptions; + streams ??= subscriptions ??= [eg.subscription(eg.stream())]; await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot( streams: streams, subscriptions: subscriptions, unreadMsgs: unreadMsgs)); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); @@ -307,6 +307,7 @@ void main() { final stream = eg.stream(name: 'stream name'); await setupMessageListPage(tester, narrow: const AllMessagesNarrow(), + subscriptions: [], messages: [ eg.streamMessage(stream: stream), ]); @@ -323,7 +324,7 @@ void main() { }); await setupMessageListPage(tester, narrow: const AllMessagesNarrow(), - streams: [streamAfter], + subscriptions: [eg.subscription(streamAfter)], messages: [ eg.streamMessage(stream: streamBefore), ]); diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 1d596a0497..1259bca4a4 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -7,6 +7,7 @@ import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/subscription_list.dart'; import 'package:zulip/widgets/unread_count_badge.dart'; +import '../flutter_checks.dart'; import '../model/binding.dart'; import '../example_data.dart' as eg; @@ -15,12 +16,14 @@ void main() { Future setupStreamListPage(WidgetTester tester, { required List subscriptions, + List userTopics = const [], UnreadMessagesSnapshot? unreadMsgs, }) async { addTearDown(testBinding.reset); final initialSnapshot = eg.initialSnapshot( subscriptions: subscriptions, streams: subscriptions.toList(), + userTopics: userTopics, unreadMsgs: unreadMsgs, ); await testBinding.globalStore.add(eg.selfAccount, initialSnapshot); @@ -111,6 +114,26 @@ void main() { check(find.byType(UnreadCountBadge).evaluate()).length.equals(1); }); + testWidgets('unread badge counts unmuted only', (tester) async { + final stream = eg.stream(); + final unreadMsgs = eg.unreadMsgs(streams: [ + UnreadStreamSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), + UnreadStreamSnapshot(streamId: stream.streamId, topic: 'b', unreadMessageIds: [3]), + ]); + await setupStreamListPage(tester, + subscriptions: [eg.subscription(stream, isMuted: true)], + userTopics: [UserTopicItem( + streamId: stream.streamId, + topicName: 'b', + lastUpdated: 1234567890, + visibilityPolicy: UserTopicVisibilityPolicy.unmuted, + )], + unreadMsgs: unreadMsgs); + check(tester.widget(find.descendant( + of: find.byType(UnreadCountBadge), matching: find.byType(Text)))) + .data.equals('1'); + }); + testWidgets('unread badge does not show with no unreads', (tester) async { final stream = eg.stream(); final unreadMsgs = eg.unreadMsgs(streams: []);