Skip to content

Commit 6f645fc

Browse files
sm-sayedism-sayedi
committed
internal_link: Generate new narrow links with "channel", not "stream"
Fixes: #633
1 parent 7f7aa98 commit 6f645fc

File tree

12 files changed

+154
-59
lines changed

12 files changed

+154
-59
lines changed

‎lib/api/model/narrow.dart

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,29 @@ typedef ApiNarrow = List<ApiNarrowElement>;
1414
/// reasonably be omitted will be omitted.
1515
ApiNarrow resolveApiNarrowForServer(ApiNarrow narrow, int zulipFeatureLevel) {
1616
final supportsOperatorDm = zulipFeatureLevel >= 177; // TODO(server-7)
17+
final supportsOperatorChannel = zulipFeatureLevel >= 250; // TODO(server-9)
1718
final supportsOperatorWith = zulipFeatureLevel >= 271; // TODO(server-9)
1819

1920
bool hasDmElement = false;
21+
bool hasChannelElement = false;
2022
bool hasWithElement = false;
2123
for (final element in narrow) {
2224
switch (element) {
23-
case ApiNarrowDm(): hasDmElement = true;
24-
case ApiNarrowWith(): hasWithElement = true;
25+
case ApiNarrowChannel(): hasChannelElement = true;
26+
case ApiNarrowDm(): hasDmElement = true;
27+
case ApiNarrowWith(): hasWithElement = true;
2528
default:
2629
}
2730
}
28-
if (!(hasDmElement || (hasWithElement && !supportsOperatorWith))) {
31+
if (!(hasChannelElement || hasDmElement || (hasWithElement && !supportsOperatorWith))) {
2932
return narrow;
3033
}
3134

3235
final result = <ApiNarrowElement>[];
3336
for (final element in narrow) {
3437
switch (element) {
38+
case ApiNarrowChannel():
39+
result.add(element.resolve(legacy: !supportsOperatorChannel));
3540
case ApiNarrowDm():
3641
result.add(element.resolve(legacy: !supportsOperatorDm));
3742
case ApiNarrowWith() when !supportsOperatorWith:
@@ -93,17 +98,52 @@ sealed class ApiNarrowElement {
9398
};
9499
}
95100

96-
class ApiNarrowStream extends ApiNarrowElement {
97-
@override String get operator => 'stream';
101+
class ApiNarrowChannel extends ApiNarrowElement {
102+
@override String get operator {
103+
assert(false,
104+
"The [operator] getter was called on a plain [ApiNarrowChannel]. "
105+
"Before passing to [jsonEncode] or otherwise getting [operator], "
106+
"the [ApiNarrowChannel] must be replaced by the result of [ApiNarrowChannel.resolve]."
107+
);
108+
return "channel";
109+
}
98110

99111
@override final int operand;
100112

101-
ApiNarrowStream(this.operand, {super.negated});
113+
ApiNarrowChannel(this.operand, {super.negated});
102114

103-
factory ApiNarrowStream.fromJson(Map<String, dynamic> json) => ApiNarrowStream(
104-
json['operand'] as int,
105-
negated: json['negated'] as bool? ?? false,
106-
);
115+
factory ApiNarrowChannel.fromJson(Map<String, dynamic> json) {
116+
var operand = (json['operand'] as int);
117+
var negated = json['negated'] as bool? ?? false;
118+
return json['operator'] == 'stream'
119+
? ApiNarrowStream._(operand, negated: negated)
120+
: ApiNarrowChannelModern._(operand, negated: negated);
121+
}
122+
123+
/// This element resolved, as either an [ApiNarrowChannelModern] or an [ApiNarrowStream].
124+
ApiNarrowChannel resolve({required bool legacy}) {
125+
return legacy ? ApiNarrowStream._(operand, negated: negated)
126+
: ApiNarrowChannelModern._(operand, negated: negated);
127+
}
128+
}
129+
130+
131+
/// An [ApiNarrowElement] with the 'channel' operator (and not the legacy 'stream').
132+
///
133+
/// To construct one of these, use [ApiNarrowChannel.resolve].
134+
class ApiNarrowChannelModern extends ApiNarrowChannel {
135+
@override String get operator => 'channel';
136+
137+
ApiNarrowChannelModern._(super.operand, {super.negated});
138+
}
139+
140+
/// An [ApiNarrowElement] with the legacy 'stream' operator.
141+
///
142+
/// To construct one of these, use [ApiNarrowChannel.resolve].
143+
class ApiNarrowStream extends ApiNarrowChannel {
144+
@override String get operator => 'stream';
145+
146+
ApiNarrowStream._(super.operand, {super.negated});
107147
}
108148

109149
class ApiNarrowTopic extends ApiNarrowElement {

‎lib/api/route/messages.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ Future<GetMessagesResult> getMessages(ApiConnection connection, {
100100
}) {
101101
assert(allowEmptyTopicName, '`allowEmptyTopicName` should only be true');
102102
return connection.get('getMessages', GetMessagesResult.fromJson, 'messages', {
103-
'narrow': resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!),
103+
'narrow': resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!),
104104
'anchor': RawParameter(anchor.toJson()),
105105
if (includeAnchor != null) 'include_anchor': includeAnchor,
106106
'num_before': numBefore,

‎lib/generated/l10n/zulip_localizations.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ abstract class ZulipLocalizations {
275275
/// **'To upload files, please grant Zulip additional permissions in Settings.'**
276276
String get permissionsDeniedReadExternalStorage;
277277

278-
/// Label in the channel context menu for subscribing to the channel.
278+
/// Label in the channel action sheet for subscribing to the channel.
279279
///
280280
/// In en, this message translates to:
281281
/// **'Subscribe'**
@@ -305,7 +305,7 @@ abstract class ZulipLocalizations {
305305
/// **'List of topics'**
306306
String get actionSheetOptionListOfTopics;
307307

308-
/// Label in the channel context menu for unsubscribing from the channel.
308+
/// Label in the channel action sheet for unsubscribing from the channel.
309309
///
310310
/// In en, this message translates to:
311311
/// **'Unsubscribe'**

‎lib/model/internal_link.dart

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,14 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) {
7171
fragment.write('${element.operator}/');
7272

7373
switch (element) {
74+
case ApiNarrowChannelModern():
7475
case ApiNarrowStream():
7576
final streamId = element.operand;
7677
final name = store.streams[streamId]?.name ?? 'unknown';
7778
final slugifiedName = _encodeHashComponent(name.replaceAll(' ', '-'));
7879
fragment.write('$streamId-$slugifiedName');
80+
case ApiNarrowChannel():
81+
assert(false, 'ApiNarrowChannel should have been resolved');
7982
case ApiNarrowTopic():
8083
fragment.write(_encodeHashComponent(element.operand.apiName));
8184
case ApiNarrowDmModern():
@@ -182,7 +185,7 @@ NarrowLink? _interpretNarrowSegments(List<String> segments, PerAccountStore stor
182185
assert(segments.isNotEmpty);
183186
assert(segments.length.isEven);
184187

185-
ApiNarrowStream? streamElement;
188+
ApiNarrowChannel? channelElement;
186189
ApiNarrowTopic? topicElement;
187190
ApiNarrowDm? dmElement;
188191
ApiNarrowWith? withElement;
@@ -196,10 +199,10 @@ NarrowLink? _interpretNarrowSegments(List<String> segments, PerAccountStore stor
196199
switch (operator) {
197200
case _NarrowOperator.stream:
198201
case _NarrowOperator.channel:
199-
if (streamElement != null) return null;
202+
if (channelElement != null) return null;
200203
final streamId = _parseStreamOperand(operand, store);
201204
if (streamId == null) return null;
202-
streamElement = ApiNarrowStream(streamId, negated: negated);
205+
channelElement = ApiNarrowChannel(streamId, negated: negated);
203206

204207
case _NarrowOperator.topic:
205208
case _NarrowOperator.subject:
@@ -238,7 +241,7 @@ NarrowLink? _interpretNarrowSegments(List<String> segments, PerAccountStore stor
238241

239242
final Narrow? narrow;
240243
if (isElementOperands.isNotEmpty) {
241-
if (streamElement != null || topicElement != null || dmElement != null || withElement != null) {
244+
if (channelElement != null || topicElement != null || dmElement != null || withElement != null) {
242245
return null;
243246
}
244247
if (isElementOperands.length > 1) return null;
@@ -257,10 +260,10 @@ NarrowLink? _interpretNarrowSegments(List<String> segments, PerAccountStore stor
257260
return null;
258261
}
259262
} else if (dmElement != null) {
260-
if (streamElement != null || topicElement != null || withElement != null) return null;
263+
if (channelElement != null || topicElement != null || withElement != null) return null;
261264
narrow = DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId);
262-
} else if (streamElement != null) {
263-
final streamId = streamElement.operand;
265+
} else if (channelElement != null) {
266+
final streamId = channelElement.operand;
264267
if (topicElement != null) {
265268
narrow = TopicNarrow(streamId, topicElement.operand, with_: withElement?.operand);
266269
} else {

‎lib/model/narrow.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class ChannelNarrow extends Narrow {
8080
}
8181

8282
@override
83-
ApiNarrow apiEncode() => [ApiNarrowStream(streamId)];
83+
ApiNarrow apiEncode() => [ApiNarrowChannel(streamId)];
8484

8585
@override
8686
String toString() => 'ChannelNarrow($streamId)';
@@ -117,7 +117,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {
117117

118118
@override
119119
ApiNarrow apiEncode() => [
120-
ApiNarrowStream(streamId),
120+
ApiNarrowChannel(streamId),
121121
ApiNarrowTopic(topic),
122122
if (with_ != null) ApiNarrowWith(with_!),
123123
];

‎test/api/route/messages_test.dart

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,14 @@ void main() {
214214

215215
checkNarrow(const CombinedFeedNarrow().apiEncode(), jsonEncode([]));
216216
checkNarrow(const ChannelNarrow(12).apiEncode(), jsonEncode([
217-
{'operator': 'stream', 'operand': 12},
217+
{'operator': 'channel', 'operand': 12},
218218
]));
219219
checkNarrow(eg.topicNarrow(12, 'stuff').apiEncode(), jsonEncode([
220-
{'operator': 'stream', 'operand': 12},
220+
{'operator': 'channel', 'operand': 12},
221221
{'operator': 'topic', 'operand': 'stuff'},
222222
]));
223223
checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
224-
{'operator': 'stream', 'operand': 12},
224+
{'operator': 'channel', 'operand': 12},
225225
{'operator': 'topic', 'operand': 'stuff'},
226226
{'operator': 'with', 'operand': 1},
227227
]));
@@ -235,7 +235,7 @@ void main() {
235235

236236
connection.zulipFeatureLevel = 270;
237237
checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
238-
{'operator': 'stream', 'operand': 12},
238+
{'operator': 'channel', 'operand': 12},
239239
{'operator': 'topic', 'operand': 'stuff'},
240240
]));
241241
checkNarrow([ApiNarrowDm([123, 234])], jsonEncode([
@@ -245,6 +245,19 @@ void main() {
245245
{'operator': 'dm', 'operand': [123, 234]},
246246
]));
247247

248+
connection.zulipFeatureLevel = 249;
249+
checkNarrow(const ChannelNarrow(12).apiEncode(), jsonEncode([
250+
{'operator': 'stream', 'operand': 12},
251+
]));
252+
checkNarrow(eg.topicNarrow(12, 'stuff').apiEncode(), jsonEncode([
253+
{'operator': 'stream', 'operand': 12},
254+
{'operator': 'topic', 'operand': 'stuff'},
255+
]));
256+
checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
257+
{'operator': 'stream', 'operand': 12},
258+
{'operator': 'topic', 'operand': 'stuff'},
259+
]));
260+
248261
connection.zulipFeatureLevel = 176;
249262
checkNarrow(eg.topicNarrow(12, 'stuff', with_: 1).apiEncode(), jsonEncode([
250263
{'operator': 'stream', 'operand': 12},

‎test/model/compose_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,11 @@ hello
351351

352352
check(quoteAndReplyPlaceholder(
353353
GlobalLocalizations.zulipLocalizations, store, message: message)).equals('''
354-
@_**Full Name|123** [said](${eg.selfAccount.realmUrl}#narrow/stream/1-test-here/topic/some.20topic/near/${message.id}): *(loading message ${message.id})*
354+
@_**Full Name|123** [said](${eg.selfAccount.realmUrl}#narrow/channel/1-test-here/topic/some.20topic/near/${message.id}): *(loading message ${message.id})*
355355
''');
356356

357357
check(quoteAndReply(store, message: message, rawContent: 'Hello world!')).equals('''
358-
@_**Full Name|123** [said](${eg.selfAccount.realmUrl}#narrow/stream/1-test-here/topic/some.20topic/near/${message.id}):
358+
@_**Full Name|123** [said](${eg.selfAccount.realmUrl}#narrow/channel/1-test-here/topic/some.20topic/near/${message.id}):
359359
```quote
360360
Hello world!
361361
```

‎test/model/internal_link_test.dart

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,16 @@ void main() {
6060
.equals(store.realmUrl.resolve('#narrow/is/starred/near/1'));
6161
});
6262

63-
test('ChannelNarrow / TopicNarrow', () {
63+
group('ChannelNarrow / TopicNarrow', () {
6464
void checkNarrow(String expectedFragment, {
6565
required int streamId,
6666
required String name,
6767
String? topic,
6868
int? nearMessageId,
69+
int? zulipFeatureLevel = eg.futureZulipFeatureLevel,
6970
}) async {
7071
assert(expectedFragment.startsWith('#'), 'wrong-looking expectedFragment');
71-
final store = eg.store();
72+
final store = eg.store()..connection.zulipFeatureLevel = zulipFeatureLevel;
7273
await store.addStream(eg.stream(streamId: streamId, name: name));
7374
final narrow = topic == null
7475
? ChannelNarrow(streamId)
@@ -77,22 +78,54 @@ void main() {
7778
.equals(store.realmUrl.resolve(expectedFragment));
7879
}
7980

80-
checkNarrow(streamId: 1, name: 'announce', '#narrow/stream/1-announce');
81-
checkNarrow(streamId: 378, name: 'api design', '#narrow/stream/378-api-design');
82-
checkNarrow(streamId: 391, name: 'Outreachy', '#narrow/stream/391-Outreachy');
83-
checkNarrow(streamId: 415, name: 'chat.zulip.org', '#narrow/stream/415-chat.2Ezulip.2Eorg');
84-
checkNarrow(streamId: 419, name: 'français', '#narrow/stream/419-fran.C3.A7ais');
85-
checkNarrow(streamId: 403, name: 'Hshs[™~}(.', '#narrow/stream/403-Hshs.5B.E2.84.A2~.7D.28.2E');
86-
checkNarrow(streamId: 60, name: 'twitter', nearMessageId: 1570686, '#narrow/stream/60-twitter/near/1570686');
87-
88-
checkNarrow(streamId: 48, name: 'mobile', topic: 'Welcome screen UI',
89-
'#narrow/stream/48-mobile/topic/Welcome.20screen.20UI');
90-
checkNarrow(streamId: 243, name: 'mobile-team', topic: 'Podfile.lock clash #F92',
91-
'#narrow/stream/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92');
92-
checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"',
93-
'#narrow/stream/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22');
94-
checkNarrow(streamId: 42, name: 'Outreachy 2016-2017', topic: '2017-18 Stream?', nearMessageId: 302690,
95-
'#narrow/stream/42-Outreachy-2016-2017/topic/2017-18.20Stream.3F/near/302690');
81+
test('modern including "channel" operator', () {
82+
checkNarrow(streamId: 1, name: 'announce', '#narrow/channel/1-announce');
83+
checkNarrow(streamId: 378, name: 'api design', '#narrow/channel/378-api-design');
84+
checkNarrow(streamId: 391, name: 'Outreachy', '#narrow/channel/391-Outreachy');
85+
checkNarrow(streamId: 415, name: 'chat.zulip.org', '#narrow/channel/415-chat.2Ezulip.2Eorg');
86+
checkNarrow(streamId: 419, name: 'français', '#narrow/channel/419-fran.C3.A7ais');
87+
checkNarrow(streamId: 403, name: 'Hshs[™~}(.', '#narrow/channel/403-Hshs.5B.E2.84.A2~.7D.28.2E');
88+
checkNarrow(streamId: 60, name: 'twitter', nearMessageId: 1570686, '#narrow/channel/60-twitter/near/1570686');
89+
90+
checkNarrow(streamId: 48, name: 'mobile', topic: 'Welcome screen UI',
91+
'#narrow/channel/48-mobile/topic/Welcome.20screen.20UI');
92+
checkNarrow(streamId: 243, name: 'mobile-team', topic: 'Podfile.lock clash #F92',
93+
'#narrow/channel/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92');
94+
checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"',
95+
'#narrow/channel/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22');
96+
checkNarrow(streamId: 42, name: 'Outreachy 2016-2017', topic: '2017-18 Stream?', nearMessageId: 302690,
97+
'#narrow/channel/42-Outreachy-2016-2017/topic/2017-18.20Stream.3F/near/302690');
98+
});
99+
100+
test('legacy including "stream" operator', () {
101+
checkNarrow(streamId: 1, name: 'announce', zulipFeatureLevel: 249,
102+
'#narrow/stream/1-announce');
103+
checkNarrow(streamId: 378, name: 'api design', zulipFeatureLevel: 249,
104+
'#narrow/stream/378-api-design');
105+
checkNarrow(streamId: 391, name: 'Outreachy', zulipFeatureLevel: 249,
106+
'#narrow/stream/391-Outreachy');
107+
checkNarrow(streamId: 415, name: 'chat.zulip.org', zulipFeatureLevel: 249,
108+
'#narrow/stream/415-chat.2Ezulip.2Eorg');
109+
checkNarrow(streamId: 419, name: 'français', zulipFeatureLevel: 249,
110+
'#narrow/stream/419-fran.C3.A7ais');
111+
checkNarrow(streamId: 403, name: 'Hshs[™~}(.', zulipFeatureLevel: 249,
112+
'#narrow/stream/403-Hshs.5B.E2.84.A2~.7D.28.2E');
113+
checkNarrow(streamId: 60, name: 'twitter', nearMessageId: 1570686, zulipFeatureLevel: 249,
114+
'#narrow/stream/60-twitter/near/1570686');
115+
116+
checkNarrow(streamId: 48, name: 'mobile', topic: 'Welcome screen UI',
117+
zulipFeatureLevel: 249,
118+
'#narrow/stream/48-mobile/topic/Welcome.20screen.20UI');
119+
checkNarrow(streamId: 243, name: 'mobile-team', topic: 'Podfile.lock clash #F92',
120+
zulipFeatureLevel: 249,
121+
'#narrow/stream/243-mobile-team/topic/Podfile.2Elock.20clash.20.23F92');
122+
checkNarrow(streamId: 377, name: 'translation/zh_tw', topic: '翻譯 "stream"',
123+
zulipFeatureLevel: 249,
124+
'#narrow/stream/377-translation.2Fzh_tw/topic/.E7.BF.BB.E8.AD.AF.20.22stream.22');
125+
checkNarrow(streamId: 42, name: 'Outreachy 2016-2017', topic: '2017-18 Stream?', nearMessageId: 302690,
126+
zulipFeatureLevel: 249,
127+
'#narrow/stream/42-Outreachy-2016-2017/topic/2017-18.20Stream.3F/near/302690');
128+
});
96129
});
97130

98131
test('DmNarrow', () {

‎test/model/message_list_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void main() {
156156
..method.equals('GET')
157157
..url.path.equals('/api/v1/messages')
158158
..url.queryParameters.deepEquals({
159-
'narrow': jsonEncode(narrow),
159+
'narrow': jsonEncode(resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!)),
160160
'anchor': anchor,
161161
if (includeAnchor != null) 'include_anchor': includeAnchor.toString(),
162162
'num_before': numBefore.toString(),

‎test/widgets/action_sheet_test.dart

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ void main() {
347347
'num_before': '0',
348348
'num_after': '1000',
349349
'narrow': jsonEncode([
350-
{'operator': 'stream', 'operand': channelId},
350+
{'operator': 'channel', 'operand': channelId},
351351
{'operator': 'is', 'operand': 'unread'},
352352
]),
353353
'op': 'add',
@@ -1011,7 +1011,9 @@ void main() {
10111011
check(connection.lastRequest).isA<http.Request>()
10121012
..url.path.equals('/api/v1/messages/flags/narrow')
10131013
..bodyFields['narrow'].equals(jsonEncode([
1014-
...eg.topicNarrow(someChannel.streamId, someTopic).apiEncode(),
1014+
...resolveApiNarrowForServer(
1015+
eg.topicNarrow(someChannel.streamId, someTopic).apiEncode(),
1016+
connection.zulipFeatureLevel!),
10151017
ApiNarrowIs(IsOperand.unread),
10161018
]))
10171019
..bodyFields['op'].equals('add')
@@ -1607,7 +1609,9 @@ void main() {
16071609
'include_anchor': 'true',
16081610
'num_before': '0',
16091611
'num_after': '1000',
1610-
'narrow': jsonEncode(TopicNarrow.ofMessage(message).apiEncode()),
1612+
'narrow': jsonEncode(resolveApiNarrowForServer(
1613+
TopicNarrow.ofMessage(message).apiEncode(),
1614+
connection.zulipFeatureLevel!)),
16111615
'op': 'remove',
16121616
'flag': 'read',
16131617
});
@@ -1652,7 +1656,9 @@ void main() {
16521656
..method.equals('POST')
16531657
..url.path.equals('/api/v1/messages/flags/narrow')
16541658
..bodyFields['narrow'].equals(
1655-
jsonEncode(eg.topicNarrow(newStream.streamId, newTopic).apiEncode()));
1659+
jsonEncode(resolveApiNarrowForServer(
1660+
eg.topicNarrow(newStream.streamId, newTopic).apiEncode(),
1661+
connection.zulipFeatureLevel!)));
16561662
});
16571663

16581664
testWidgets('shows error when fails', (tester) async {

0 commit comments

Comments
 (0)