Skip to content

Commit

Permalink
narrow: Support MentionsNarrow.
Browse files Browse the repository at this point in the history
Because of how narrow interacts with the entire app, there are test
updates all over the place. The way I checked for the completeness of
this change is by looking at places where CombinedFeedNarrow is
referenced.

Signed-off-by: Zixuan James Li <[email protected]>
  • Loading branch information
PIG208 committed Jul 20, 2024
1 parent 9def0fc commit 73e5374
Show file tree
Hide file tree
Showing 17 changed files with 189 additions and 4 deletions.
4 changes: 4 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@
"@loginErrorMissingUsername": {
"description": "Error message when an empty username was provided."
},
"mentionsPageTitle": "Mentions",
"@mentionsPageTitle": {
"description": "Title for the page of @-mentions."
},
"topicValidationErrorTooLong": "Topic length shouldn't be greater than 60 characters.",
"@topicValidationErrorTooLong": {
"description": "Topic validation error when topic is too long."
Expand Down
1 change: 1 addition & 0 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class MentionAutocompleteView extends ChangeNotifier {
required Narrow narrow,
}) {
assert(narrow is! CombinedFeedNarrow);
assert(narrow is! MentionsNarrow);
return store.users.values.toList()
..sort((userA, userB) => compareByDms(userA, userB, store: store));
}
Expand Down
2 changes: 2 additions & 0 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {

case TopicNarrow():
case DmNarrow():
case MentionsNarrow():
return true;
}
}
Expand All @@ -385,6 +386,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {

case TopicNarrow():
case DmNarrow():
case MentionsNarrow():
return true;
}
}
Expand Down
23 changes: 22 additions & 1 deletion lib/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -286,4 +286,25 @@ class DmNarrow extends Narrow implements SendableNarrow {
int get hashCode => Object.hash('DmNarrow', _key);
}

// TODO other narrow types: starred, mentioned; searches; arbitrary
class MentionsNarrow extends Narrow {
const MentionsNarrow();

@override
bool containsMessage(Message message) {
return message.flags.any((flag) =>
flag == MessageFlag.mentioned || flag == MessageFlag.wildcardMentioned);
}

@override
ApiNarrow apiEncode() => [ApiNarrowIsMentioned()];

@override
bool operator ==(Object other) {
if (other is! MentionsNarrow) return false;
// Conceptually there's only one value of this type.
return true;
}

@override
int get hashCode => 'MentionedNarrow'.hashCode;
}
4 changes: 4 additions & 0 deletions lib/model/unreads.dart
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ class Unreads extends ChangeNotifier {

int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0;

int countInMentionsNarrow() => mentions.length;

int countInNarrow(Narrow narrow) {
switch (narrow) {
case CombinedFeedNarrow():
Expand All @@ -203,6 +205,8 @@ class Unreads extends ChangeNotifier {
return countInTopicNarrow(narrow.streamId, narrow.topic);
case DmNarrow():
return countInDmNarrow(narrow);
case MentionsNarrow():
return countInMentionsNarrow();
}
}

Expand Down
7 changes: 7 additions & 0 deletions lib/widgets/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,12 @@ Future<void> _legacyMarkNarrowAsRead(BuildContext context, Narrow narrow) async
messages: unreadDms,
op: UpdateMessageFlagsOp.add,
flag: MessageFlag.read);
case MentionsNarrow():
final unreadMentions = store.unreads.mentions.toList();
if (unreadMentions.isEmpty) return;
await updateMessageFlags(connection,
messages: unreadMentions,
op: UpdateMessageFlagsOp.add,
flag: MessageFlag.read);
}
}
2 changes: 2 additions & 0 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,7 @@ class ComposeBox extends StatelessWidget {
static bool hasComposeBox(Narrow narrow) {
switch (narrow) {
case CombinedFeedNarrow():
case MentionsNarrow():
return false;

case StreamNarrow():
Expand All @@ -999,6 +1000,7 @@ class ComposeBox extends StatelessWidget {
case DmNarrow():
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
case CombinedFeedNarrow():
case MentionsNarrow():
return const SizedBox.shrink();
}
}
Expand Down
4 changes: 4 additions & 0 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
bool removeAppBarBottomBorder = false;
switch(widget.narrow) {
case CombinedFeedNarrow():
case MentionsNarrow():
appBarBackgroundColor = null; // i.e., inherit

case StreamNarrow(:final streamId):
Expand Down Expand Up @@ -190,6 +191,9 @@ class MessageListAppBarTitle extends StatelessWidget {
final names = otherRecipientIds.map((id) => store.users[id]?.fullName ?? '(unknown user)');
return Text("DMs with ${names.join(", ")}"); // TODO show avatars
}

case MentionsNarrow():
return Text(zulipLocalizations.mentionsPageTitle);
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/api/route/messages_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ void main() {
{'operator': 'stream', 'operand': 12},
{'operator': 'topic', 'operand': 'stuff'},
]));
checkNarrow(const MentionsNarrow().apiEncode(), jsonEncode([
{'operator': 'is', 'operand': 'mentioned'},
]));

checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
{'operator': 'dm', 'operand': [123, 234]},
Expand Down
9 changes: 9 additions & 0 deletions test/model/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,15 @@ void main() {
expected: [0, 1, 2, 3, 4])
).throws();
});

test('MentionsNarrow', () async {
// As we do not expect a compose box in [MentionsNarrow], it should
// not proceed to show any results.
await check(checkResultsIn(
const MentionsNarrow(),
expected: [0, 1, 2, 3, 4])
).throws();
});
});
});
}
9 changes: 7 additions & 2 deletions test/model/compose_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,13 @@ hello
'#narrow/pm-with/1,2-pm/near/12345');
});

// TODO other Narrow subclasses as we add them:
// starred, mentioned; searches; arbitrary
test('MentionsNarrow', () {
final store = eg.store();
check(narrowLink(store, const MentionsNarrow()))
.equals(store.realmUrl.resolve('#narrow/is/mentioned'));
check(narrowLink(store, const MentionsNarrow(), nearMessageId: 1))
.equals(store.realmUrl.resolve('#narrow/is/mentioned/near/1'));
});
});

group('mention', () {
Expand Down
47 changes: 47 additions & 0 deletions test/model/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,52 @@ void main() {
checkNotifiedOnce();
check(model.messages.map((m) => m.id)).deepEquals(expected..add(301));
});

test('in MentionsNarrow', () async {
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
final stream2 = eg.stream(streamId: 2, name: 'stream 2');
await prepare(narrow: const MentionsNarrow());
await store.addStreams([stream1, stream2]);
await store.addSubscription(eg.subscription(stream1));
await store.addUserTopic(stream1, 'B', UserTopicVisibilityPolicy.muted);
await store.addSubscription(eg.subscription(stream2, isMuted: true));
await store.addUserTopic(stream2, 'C', UserTopicVisibilityPolicy.unmuted);

List<Message> getMessages(int startingId) => [
eg.streamMessage(id: startingId,
stream: stream1, topic: "A", flags: [MessageFlag.wildcardMentioned]),
eg.streamMessage(id: startingId + 1,
stream: stream1, topic: "B", flags: [MessageFlag.wildcardMentioned]),
eg.streamMessage(id: startingId + 2,
stream: stream2, topic: "C", flags: [MessageFlag.wildcardMentioned]),
eg.streamMessage(id: startingId + 3,
stream: stream2, topic: "D", flags: [MessageFlag.wildcardMentioned]),
eg.dmMessage(id: startingId + 4,
from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.mentioned]),
];

// Check filtering on fetchInitial…
await prepareMessages(foundOldest: false, messages: getMessages(201));
final expected = <int>[];
check(model.messages.map((m) => m.id))
.deepEquals(expected..addAll([201, 202, 203, 204, 205]));

// … and on fetchOlder…
connection.prepare(json: olderResult(
anchor: 201, foundOldest: true, messages: getMessages(101)).toJson());
await model.fetchOlder();
checkNotified(count: 2);
check(model.messages.map((m) => m.id))
.deepEquals(expected..insertAll(0, [101, 102, 103, 104, 105]));

// … and on MessageEvent.
final messages = getMessages(301);
for (var i = 0; i < 5; i += 1) {
await store.handleEvent(MessageEvent(id: 0, message: messages[i]));
checkNotifiedOnce();
check(model.messages.map((m) => m.id)).deepEquals(expected..add(301 + i));
}
});
});

test('recipient headers are maintained consistently', () async {
Expand Down Expand Up @@ -938,6 +984,7 @@ void checkInvariants(MessageListView model) {
.isTrue();
case TopicNarrow():
case DmNarrow():
case MentionsNarrow():
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/model/narrow_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,17 @@ void main() {
check(narrow123.containsMessage(dm(user3, [user1, user2]))).isTrue();
});
});

group('MentionsNarrow', () {
test('containsMessage', () {
const narrow = MentionsNarrow();

check(narrow.containsMessage(eg.streamMessage(
flags: []))).isFalse();
check(narrow.containsMessage(eg.streamMessage(
flags:[MessageFlag.mentioned]))).isTrue();
check(narrow.containsMessage(eg.streamMessage(
flags: [MessageFlag.wildcardMentioned]))).isTrue();
});
});
}
12 changes: 12 additions & 0 deletions test/model/unreads_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ void main() {
eg.otherUser.userId, selfUserId: eg.selfUser.userId);
check(model.countInDmNarrow(narrow)).equals(5);
});

test('countInMentionsNarrow', () async {
final stream = eg.stream();
prepare();
await streamStore.addStream(stream);
fillWithMessages([
eg.streamMessage(stream: stream, flags: []),
eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]),
eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]),
]);
check(model.countInMentionsNarrow()).equals(2);
});
});

group('handleMessageEvent', () {
Expand Down
6 changes: 6 additions & 0 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,12 @@ void main() {
await setupToMessageActionSheet(tester, message: message, narrow: const CombinedFeedNarrow());
check(findQuoteAndReplyButton(tester)).isNull();
});

testWidgets('not offered in MentionsNarrow (composing to reply is not yet supported)', (WidgetTester tester) async {
final message = eg.streamMessage();
await setupToMessageActionSheet(tester, message: message, narrow: const MentionsNarrow());
check(findQuoteAndReplyButton(tester)).isNull();
});
});

group('CopyMessageTextButton', () {
Expand Down
22 changes: 22 additions & 0 deletions test/widgets/actions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:http/http.dart' as http;
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';
Expand Down Expand Up @@ -267,6 +268,27 @@ void main() {
});
});

testWidgets('MentionsNarrow on legacy server', (WidgetTester tester) async {
const narrow = MentionsNarrow();
final message = eg.streamMessage(flags: [MessageFlag.mentioned]);
final unreadMsgs = eg.unreadMsgs(mentions: [message.id]);
await prepare(tester, unreadMsgs: unreadMsgs);
connection.zulipFeatureLevel = 154;
connection.prepare(json:
UpdateMessageFlagsResult(messages: [message.id]).toJson());
markNarrowAsRead(context, narrow, true); // TODO move legacy-server check inside markNarrowAsRead
await tester.pump(Duration.zero);
check(connection.lastRequest).isA<http.Request>()
..method.equals('POST')
..url.path.equals('/api/v1/messages/flags')
..bodyFields.deepEquals({
'messages': jsonEncode([message.id]),
'op': 'add',
'flag': 'read',
});
await tester.pumpAndSettle();
});

testWidgets('catch-all api errors', (WidgetTester tester) async {
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
const narrow = CombinedFeedNarrow();
Expand Down
25 changes: 24 additions & 1 deletion test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,19 @@ void main() {
final padding = MediaQuery.of(element).padding;
check(padding).equals(EdgeInsets.zero);
});

testWidgets('content in MentionsNarrow not asked to consume insets (including bottom)', (tester) async {
const fakePadding = FakeViewPadding(left: 10, top: 10, right: 10, bottom: 10);
tester.view.viewInsets = fakePadding;
tester.view.padding = fakePadding;

await setupMessageListPage(tester, narrow: const MentionsNarrow(),
messages: [eg.streamMessage(content: ContentExample.codeBlockPlain.html, flags: [MessageFlag.mentioned])]);

final element = tester.element(find.byType(CodeBlock));
final padding = MediaQuery.of(element).padding;
check(padding).equals(EdgeInsets.zero);
});
});

group('fetch older messages on scroll', () {
Expand Down Expand Up @@ -297,7 +310,8 @@ void main() {
group('recipient headers', () {
group('StreamMessageRecipientHeader', () {
final stream = eg.stream(name: 'stream name');
final message = eg.streamMessage(stream: stream, topic: 'topic name');
final message = eg.streamMessage(
stream: stream, topic: 'topic name');

FinderResult<Element> findInMessageList(String text) {
// Stream name shows up in [AppBar] so need to avoid matching that
Expand All @@ -315,6 +329,15 @@ void main() {
check(findInMessageList('topic name')).length.equals(1);
});

testWidgets('show stream name in MentionsNarrow', (tester) async {
await setupMessageListPage(tester,
narrow: const MentionsNarrow(),
messages: [message], subscriptions: [eg.subscription(stream)]);
await tester.pump();
check(findInMessageList('stream name')).length.equals(1);
check(findInMessageList('topic name')).length.equals(1);
});

testWidgets('do not show stream name in StreamNarrow', (tester) async {
await setupMessageListPage(tester,
narrow: StreamNarrow(stream.streamId),
Expand Down

0 comments on commit 73e5374

Please sign in to comment.