From c3b9b6ca32d9781babf158524af7697846a3e7aa Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 14 Jun 2023 14:47:27 -0700 Subject: [PATCH 1/4] test: Bump recentZulip{Version,FeatureLevel} in example data --- test/example_data.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index 4a9d79d485..f3aab3a523 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -6,8 +6,8 @@ import 'api/fake_api.dart'; final Uri realmUrl = Uri.parse('https://chat.example/'); -const String recentZulipVersion = '6.1'; -const int recentZulipFeatureLevel = 164; +const String recentZulipVersion = '8.0'; +const int recentZulipFeatureLevel = 185; const int futureZulipFeatureLevel = 9999; User user({int? userId, String? email, String? fullName}) { From aab6cadb8b9fa03d5f455ca86ed2e4fa4322a597 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 14 Jun 2023 14:49:08 -0700 Subject: [PATCH 2/4] test: Add `stream` helper in example data --- test/example_data.dart | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/example_data.dart b/test/example_data.dart index f3aab3a523..8146678078 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -57,6 +57,36 @@ final Account otherAccount = Account( final User thirdUser = user(fullName: 'Third User', email: 'third@example', userId: 345); +ZulipStream stream({ + int? streamId, + String? name, + String? description, + String? renderedDescription, + int? dateCreated, + int? firstMessageId, + bool? inviteOnly, + bool? isWebPublic, + bool? historyPublicToSubscribers, + int? messageRetentionDays, + int? streamPostPolicy, + int? canRemoveSubscribersGroupId, +}) { + return ZulipStream( + streamId: streamId ?? 123, // TODO generate example IDs + name: name ?? 'A stream', // TODO generate example names + description: description ?? 'A description', // TODO generate example descriptions + renderedDescription: renderedDescription ?? '

A description

', // TODO generate random + dateCreated: dateCreated ?? 1686774898, + firstMessageId: firstMessageId, + inviteOnly: inviteOnly ?? false, + isWebPublic: isWebPublic ?? false, + historyPublicToSubscribers: historyPublicToSubscribers ?? true, + messageRetentionDays: messageRetentionDays, + streamPostPolicy: streamPostPolicy ?? 1, + canRemoveSubscribersGroupId: canRemoveSubscribersGroupId ?? 123, + ); +} + final _messagePropertiesBase = { 'is_me_message': false, 'last_edit_timestamp': null, From 4ee034193818aff08e61e4fbd9520153e60b0769 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 13 Jun 2023 18:11:57 -0700 Subject: [PATCH 3/4] compose: Add `narrowLink` helper, to write links to Narrows We'll use this soon, for quote-and-reply #116. With the recent commit 4e11ea708, PerAccountStoreTestExtension now has `addStream`, which we use in the tests. --- lib/model/compose.dart | 60 ++++++++++++++++++++++++++++++ test/model/compose_test.dart | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/lib/model/compose.dart b/lib/model/compose.dart index c76f38c0cf..a26dc071e3 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -1,5 +1,9 @@ import 'dart:math'; +import '../api/model/narrow.dart'; +import 'narrow.dart'; +import 'store.dart'; + // // Put functions for nontrivial message-content generation in this file. // @@ -96,5 +100,61 @@ String wrapWithBacktickFence({required String content, String? infoString}) { return resultBuffer.toString(); } +const _hashReplacements = { + "%": ".", + "(": ".28", + ")": ".29", + ".": ".2E", +}; + +final _encodeHashComponentRegex = RegExp(r'[%().]'); + +// Corresponds to encodeHashComponent in Zulip web; +// see web/shared/src/internal_url.ts. +String _encodeHashComponent(String str) { + return Uri.encodeComponent(str) + .replaceAllMapped(_encodeHashComponentRegex, (Match m) => _hashReplacements[m[0]!]!); +} + +/// A URL to the given [Narrow], on `store`'s realm. +Uri narrowLink(PerAccountStore store, Narrow narrow) { + final apiNarrow = narrow.apiEncode(); + final fragment = StringBuffer('narrow'); + for (ApiNarrowElement element in apiNarrow) { + fragment.write('/'); + if (element.negated) { + fragment.write('-'); + } + + if (element is ApiNarrowDm) { + final supportsOperatorDm = store.connection.zulipFeatureLevel! >= 177; // TODO(server-7) + element = element.resolve(legacy: !supportsOperatorDm); + } + + fragment.write('${element.operator}/'); + + switch (element) { + case ApiNarrowStream(): + final streamId = element.operand; + final name = store.streams[streamId]?.name ?? 'unknown'; + final slugifiedName = _encodeHashComponent(name.replaceAll(' ', '-')); + fragment.write('$streamId-$slugifiedName'); + case ApiNarrowTopic(): + fragment.write(_encodeHashComponent(element.operand)); + case ApiNarrowDmModern(): + final suffix = element.operand.length >= 3 ? 'group' : 'dm'; + fragment.write('${element.operand.join(',')}-$suffix'); + case ApiNarrowPmWith(): + final suffix = element.operand.length >= 3 ? 'group' : 'pm'; + fragment.write('${element.operand.join(',')}-$suffix'); + case ApiNarrowDm(): + assert(false, 'ApiNarrowDm should have been resolved'); + case ApiNarrowMessageId(): + fragment.write(element.operand.toString()); + } + } + return store.account.realmUrl.replace(fragment: fragment.toString()); +} + // TODO more, like /near links to messages in conversations // (also to be used in quote-and-reply) diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index fac3eb0ffd..a924b79c73 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -1,6 +1,10 @@ import 'package:checks/checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/model/compose.dart'; +import 'package:zulip/model/narrow.dart'; + +import '../example_data.dart' as eg; +import 'test_store.dart'; void main() { group('wrapWithBacktickFence', () { @@ -215,4 +219,71 @@ hello '''); }); }); + + group('narrowLink', () { + test('AllMessagesNarrow', () { + final store = eg.store(); + check(narrowLink(store, const AllMessagesNarrow())).equals(store.account.realmUrl.resolve('#narrow')); + }); + + test('StreamNarrow / TopicNarrow', () { + void checkNarrow(String expectedFragment, { + required int streamId, + required String name, + String? topic, + }) { + assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment'); + final store = eg.store(); + store.addStream(eg.stream(streamId: streamId, name: name)); + final narrow = topic == null + ? StreamNarrow(streamId) + : TopicNarrow(streamId, topic); + check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment)); + } + + checkNarrow(streamId: 1, name: 'announce', '#narrow/stream/1-announce'); + checkNarrow(streamId: 378, name: 'api design', '#narrow/stream/378-api-design'); + checkNarrow(streamId: 391, name: 'Outreachy', '#narrow/stream/391-Outreachy'); + checkNarrow(streamId: 415, name: 'chat.zulip.org', '#narrow/stream/415-chat.2Ezulip.2Eorg'); + checkNarrow(streamId: 419, name: 'français', '#narrow/stream/419-fran.C3.A7ais'); + checkNarrow(streamId: 403, name: 'Hshs[™~}(.', '#narrow/stream/403-Hshs.5B.E2.84.A2~.7D.28.2E'); + + checkNarrow(streamId: 48, name: 'mobile', topic: 'Welcome screen UI', + '#narrow/stream/48-mobile/topic/Welcome.20screen.20UI'); + checkNarrow(streamId: 243, name: 'mobile-team', topic: 'Podfile.lock clash #F92', + '#narrow/stream/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92'); + checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"', + '#narrow/stream/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22'); + }); + + test('DmNarrow', () { + void checkNarrow(String expectedFragment, String legacyExpectedFragment, { + required List allRecipientIds, + required int selfUserId, + }) { + assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment'); + final store = eg.store(); + final narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: selfUserId); + check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment)); + store.connection.zulipFeatureLevel = 176; + check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(legacyExpectedFragment)); + } + + checkNarrow(allRecipientIds: [1], selfUserId: 1, + '#narrow/dm/1-dm', + '#narrow/pm-with/1-pm'); + checkNarrow(allRecipientIds: [1, 2], selfUserId: 1, + '#narrow/dm/1,2-dm', + '#narrow/pm-with/1,2-pm'); + checkNarrow(allRecipientIds: [1, 2, 3], selfUserId: 1, + '#narrow/dm/1,2,3-group', + '#narrow/pm-with/1,2,3-group'); + checkNarrow(allRecipientIds: [1, 2, 3, 4], selfUserId: 4, + '#narrow/dm/1,2,3,4-group', + '#narrow/pm-with/1,2,3,4-group'); + }); + + // TODO other Narrow subclasses as we add them: + // starred, mentioned; searches; arbitrary + }); } From 26b693a2ff66d2ae3eb8b24fcc81844bd3e41b9c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 14 Jun 2023 15:39:33 -0700 Subject: [PATCH 4/4] compose: Have `narrowLink` take an optional `nearMessageId` --- lib/model/compose.dart | 25 +++++++++++++++++++++---- test/model/compose_test.dart | 22 ++++++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/lib/model/compose.dart b/lib/model/compose.dart index a26dc071e3..9e9691fd1c 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -117,7 +117,22 @@ String _encodeHashComponent(String str) { } /// A URL to the given [Narrow], on `store`'s realm. -Uri narrowLink(PerAccountStore store, Narrow narrow) { +/// +/// To include /near/{messageId} in the link, pass a non-null [nearMessageId]. +// Why take [nearMessageId] in a param, instead of looking for it in [narrow]? +// +// A reasonable question: after all, the "near" part of a near link (e.g., for +// quote-and-reply) does take the same form as other operator/operand pairs +// that we represent with [ApiNarrowElement]s, like "/stream/48-mobile". +// +// But unlike those other elements, we choose not to give the "near" element +// an [ApiNarrowElement] representation, because it doesn't have quite that role: +// it says where to look in a list of messages, but it doesn't filter the list down. +// In fact, from a brief look at server code, it seems to be *ignored* +// if you include it in the `narrow` param in get-messages requests. +// When you want to point the server to a location in a message list, you +// you do so by passing the `anchor` param. +Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) { final apiNarrow = narrow.apiEncode(); final fragment = StringBuffer('narrow'); for (ApiNarrowElement element in apiNarrow) { @@ -153,8 +168,10 @@ Uri narrowLink(PerAccountStore store, Narrow narrow) { fragment.write(element.operand.toString()); } } + + if (nearMessageId != null) { + fragment.write('/near/$nearMessageId'); + } + return store.account.realmUrl.replace(fragment: fragment.toString()); } - -// TODO more, like /near links to messages in conversations -// (also to be used in quote-and-reply) diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index a924b79c73..77472ab564 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -223,7 +223,10 @@ hello group('narrowLink', () { test('AllMessagesNarrow', () { final store = eg.store(); - check(narrowLink(store, const AllMessagesNarrow())).equals(store.account.realmUrl.resolve('#narrow')); + check(narrowLink(store, const AllMessagesNarrow())) + .equals(store.account.realmUrl.resolve('#narrow')); + check(narrowLink(store, const AllMessagesNarrow(), nearMessageId: 1)) + .equals(store.account.realmUrl.resolve('#narrow/near/1')); }); test('StreamNarrow / TopicNarrow', () { @@ -231,6 +234,7 @@ hello required int streamId, required String name, String? topic, + int? nearMessageId, }) { assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment'); final store = eg.store(); @@ -238,7 +242,8 @@ hello final narrow = topic == null ? StreamNarrow(streamId) : TopicNarrow(streamId, topic); - check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment)); + check(narrowLink(store, narrow, nearMessageId: nearMessageId)) + .equals(store.account.realmUrl.resolve(expectedFragment)); } checkNarrow(streamId: 1, name: 'announce', '#narrow/stream/1-announce'); @@ -247,6 +252,7 @@ hello checkNarrow(streamId: 415, name: 'chat.zulip.org', '#narrow/stream/415-chat.2Ezulip.2Eorg'); checkNarrow(streamId: 419, name: 'français', '#narrow/stream/419-fran.C3.A7ais'); checkNarrow(streamId: 403, name: 'Hshs[™~}(.', '#narrow/stream/403-Hshs.5B.E2.84.A2~.7D.28.2E'); + checkNarrow(streamId: 60, name: 'twitter', nearMessageId: 1570686, '#narrow/stream/60-twitter/near/1570686'); checkNarrow(streamId: 48, name: 'mobile', topic: 'Welcome screen UI', '#narrow/stream/48-mobile/topic/Welcome.20screen.20UI'); @@ -254,19 +260,24 @@ hello '#narrow/stream/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92'); checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"', '#narrow/stream/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22'); + checkNarrow(streamId: 42, name: 'Outreachy 2016-2017', topic: '2017-18 Stream?', nearMessageId: 302690, + '#narrow/stream/42-Outreachy-2016-2017/topic/2017-18.20Stream.3F/near/302690'); }); test('DmNarrow', () { void checkNarrow(String expectedFragment, String legacyExpectedFragment, { required List allRecipientIds, required int selfUserId, + int? nearMessageId, }) { assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment'); final store = eg.store(); final narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: selfUserId); - check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(expectedFragment)); + check(narrowLink(store, narrow, nearMessageId: nearMessageId)) + .equals(store.account.realmUrl.resolve(expectedFragment)); store.connection.zulipFeatureLevel = 176; - check(narrowLink(store, narrow)).equals(store.account.realmUrl.resolve(legacyExpectedFragment)); + check(narrowLink(store, narrow, nearMessageId: nearMessageId)) + .equals(store.account.realmUrl.resolve(legacyExpectedFragment)); } checkNarrow(allRecipientIds: [1], selfUserId: 1, @@ -281,6 +292,9 @@ hello checkNarrow(allRecipientIds: [1, 2, 3, 4], selfUserId: 4, '#narrow/dm/1,2,3,4-group', '#narrow/pm-with/1,2,3,4-group'); + checkNarrow(allRecipientIds: [1, 2], selfUserId: 1, nearMessageId: 12345, + '#narrow/dm/1,2-dm/near/12345', + '#narrow/pm-with/1,2-pm/near/12345'); }); // TODO other Narrow subclasses as we add them: