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
gnprice committed Dec 19, 2023
1 parent 7be6130 commit cb4e0e1
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 32 deletions.
46 changes: 38 additions & 8 deletions lib/model/unreads.dart
View file
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.isTopicVisibleAsUnread(streamId, topic)) {
c = c + messageIds.length;
}
}
}
return c;
}

/// The "strict" unread count for this stream,
/// using [StreamStore.isTopicVisibleAsUnread].
///
/// 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.isTopicVisibleAsUnread(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
View file
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
26 changes: 22 additions & 4 deletions test/model/unreads_test.dart
View file
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,47 @@ 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(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(4);
});

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
50 changes: 31 additions & 19 deletions test/widgets/message_list_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:http/http.dart' as http;
import 'package:zulip/api/model/events.dart';
import 'package:zulip/api/model/initial_snapshot.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/model/narrow.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/narrow.dart';
Expand Down Expand Up @@ -432,7 +433,8 @@ void main() {
}

testWidgets('from read to unread', (WidgetTester tester) async {
final message = eg.streamMessage(flags: [MessageFlag.read]);
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser],
flags: [MessageFlag.read]);
await setupMessageListPage(tester, messages: [message]);
check(isMarkAsReadButtonVisible(tester)).isFalse();

Expand All @@ -443,9 +445,10 @@ void main() {
});

testWidgets('from unread to read', (WidgetTester tester) async {
final message = eg.streamMessage(flags: []);
final unreadMsgs = eg.unreadMsgs(streams:[
UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId, unreadMessageIds: [message.id])
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
final unreadMsgs = eg.unreadMsgs(dms: [
UnreadDmSnapshot(otherUserId: eg.otherUser.userId,
unreadMessageIds: [message.id]),
]);
await setupMessageListPage(tester, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -461,14 +464,14 @@ void main() {
});

group('onPressed behavior', () {
final message = eg.streamMessage(flags: []);
final unreadMsgs = eg.unreadMsgs(streams: [
UnreadStreamSnapshot(streamId: message.streamId, topic: message.subject,
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
final unreadMsgs = eg.unreadMsgs(dms: [
UnreadDmSnapshot(otherUserId: eg.otherUser.userId,
unreadMessageIds: [message.id]),
]);

testWidgets('smoke test on modern server', (WidgetTester tester) async {
final narrow = TopicNarrow.ofMessage(message);
final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -487,7 +490,8 @@ void main() {
'include_anchor': 'false',
'num_before': '0',
'num_after': '1000',
'narrow': jsonEncode(narrow.apiEncode()),
'narrow': jsonEncode(resolveDmElements(narrow.apiEncode(),
eg.recentZulipFeatureLevel)),
'op': 'add',
'flag': 'read',
});
Expand Down Expand Up @@ -526,7 +530,7 @@ void main() {
testWidgets('markNarrowAsRead pagination', (WidgetTester tester) async {
// Check that `lastProcessedId` returned from an initial
// response is used as `anchorId` for the subsequent request.
final narrow = TopicNarrow.ofMessage(message);
final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -545,7 +549,8 @@ void main() {
'include_anchor': 'false',
'num_before': '0',
'num_after': '1000',
'narrow': jsonEncode(narrow.apiEncode()),
'narrow': jsonEncode(resolveDmElements(narrow.apiEncode(),
eg.recentZulipFeatureLevel)),
'op': 'add',
'flag': 'read',
});
Expand All @@ -564,15 +569,16 @@ void main() {
'include_anchor': 'false',
'num_before': '0',
'num_after': '1000',
'narrow': jsonEncode(narrow.apiEncode()),
'narrow': jsonEncode(resolveDmElements(narrow.apiEncode(),
eg.recentZulipFeatureLevel)),
'op': 'add',
'flag': 'read',
});
});

testWidgets('markNarrowAsRead on invalid response', (WidgetTester tester) async {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
final narrow = TopicNarrow.ofMessage(message);
final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -591,7 +597,8 @@ void main() {
'include_anchor': 'false',
'num_before': '0',
'num_after': '1000',
'narrow': jsonEncode(narrow.apiEncode()),
'narrow': jsonEncode(resolveDmElements(narrow.apiEncode(),
eg.recentZulipFeatureLevel)),
'op': 'add',
'flag': 'read',
});
Expand Down Expand Up @@ -621,7 +628,12 @@ void main() {
});

testWidgets('StreamNarrow on legacy server', (WidgetTester tester) async {
final message = eg.streamMessage();
final narrow = StreamNarrow(message.streamId);
final unreadMsgs = eg.unreadMsgs(streams: [
UnreadStreamSnapshot(streamId: message.streamId, topic: message.subject,
unreadMessageIds: [message.id]),
]);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -641,7 +653,12 @@ void main() {
});

testWidgets('TopicNarrow on legacy server', (WidgetTester tester) async {
final message = eg.streamMessage();
final narrow = TopicNarrow.ofMessage(message);
final unreadMsgs = eg.unreadMsgs(streams: [
UnreadStreamSnapshot(streamId: message.streamId, topic: message.subject,
unreadMessageIds: [message.id]),
]);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand All @@ -662,12 +679,7 @@ void main() {
});

testWidgets('DmNarrow on legacy server', (WidgetTester tester) async {
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
final narrow = DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId);
final unreadMsgs = eg.unreadMsgs(dms: [
UnreadDmSnapshot(otherUserId: eg.otherUser.userId,
unreadMessageIds: [message.id]),
]);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();
Expand Down
23 changes: 23 additions & 0 deletions test/widgets/subscription_list_test.dart
View file
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 cb4e0e1

Please sign in to comment.