Skip to content

Commit

Permalink
unread: Apply stream and topic muting to unread counts
Browse files Browse the repository at this point in the history
  • Loading branch information
gnprice committed Dec 20, 2023
1 parent 331d4ba commit 0d03c8e
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 15 deletions.
46 changes: 38 additions & 8 deletions lib/model/unreads.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion lib/widgets/subscription_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
27 changes: 23 additions & 4 deletions test/model/unreads_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
});

Expand Down
5 changes: 3 additions & 2 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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),
]);
Expand All @@ -323,7 +324,7 @@ void main() {
});
await setupMessageListPage(tester,
narrow: const AllMessagesNarrow(),
streams: [streamAfter],
subscriptions: [eg.subscription(streamAfter)],
messages: [
eg.streamMessage(stream: streamBefore),
]);
Expand Down
23 changes: 23 additions & 0 deletions test/widgets/subscription_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -15,12 +16,14 @@ void main() {

Future<void> setupStreamListPage(WidgetTester tester, {
required List<Subscription> subscriptions,
List<UserTopicItem> 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);
Expand Down Expand Up @@ -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<Text>(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: []);
Expand Down

0 comments on commit 0d03c8e

Please sign in to comment.