Skip to content

Commit 4e86013

Browse files
chrisbobbechrisbobbe
committed
internalLinks: Recognize "channel" as operator in narrow links
Fixes: zulip#5860
1 parent 5fa3ce6 commit 4e86013

File tree

2 files changed

+82
-32
lines changed

2 files changed

+82
-32
lines changed

‎src/utils/__tests__/internalLinks-test.js

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -44,61 +44,71 @@ describe('isNarrowLink', () => {
4444
[true, 'legacy: stream name, no ID', urlOnRealm('#narrow/stream/jest')],
4545
[true, 'legacy: stream name, no ID, topic', urlOnRealm('#narrow/stream/jest/topic/topic1')],
4646

47-
[true, 'with numeric stream ID', urlOnRealm('#narrow/stream/123-jest')],
48-
[true, 'with numeric stream ID and topic', urlOnRealm('#narrow/stream/123-jest/topic/topic1')],
47+
[true, 'with numeric channel ID', urlOnRealm('#narrow/channel/123-jest')],
48+
[true, 'with numeric channel ID (old operator)', urlOnRealm('#narrow/stream/123-jest')],
49+
[
50+
true,
51+
'with numeric channel ID and topic',
52+
urlOnRealm('#narrow/channel/123-jest/topic/topic1'),
53+
],
54+
[
55+
true,
56+
'with numeric channel ID (old operator) and topic',
57+
urlOnRealm('#narrow/stream/123-jest/topic/topic1'),
58+
],
4959

5060
[true, 'with numeric pm user IDs (new operator)', urlOnRealm('#narrow/dm/123-mark')],
5161
[true, 'with numeric pm user IDs (old operator)', urlOnRealm('#narrow/pm-with/123-mark')],
5262

5363
[false, 'wrong fragment', urlOnRealm('#nope')],
54-
[false, 'wrong path', urlOnRealm('/user_uploads/#narrow/stream/jest')],
55-
[false, 'wrong domain', new URL('https://another.com/#narrow/stream/jest')],
64+
[false, 'wrong path', urlOnRealm('/user_uploads/#narrow/channel/jest')],
65+
[false, 'wrong domain', new URL('https://another.com/#narrow/channel/jest')],
5666

57-
[false, '#narrowly', urlOnRealm('#narrowly/stream/jest')],
67+
[false, '#narrowly', urlOnRealm('#narrowly/channel/jest')],
5868

59-
[false, 'double slash', new URL(`${realm.origin}//#narrow/stream/jest`)],
60-
[false, 'triple slash', new URL(`${realm.origin}///#narrow/stream/jest`)],
69+
[false, 'double slash', new URL(`${realm.origin}//#narrow/channel/jest`)],
70+
[false, 'triple slash', new URL(`${realm.origin}///#narrow/channel/jest`)],
6171

6272
[
6373
true,
6474
'with port',
65-
new URL('#narrow/stream/jest', 'https://example.com:444/'),
75+
new URL('#narrow/channel/jest', 'https://example.com:444/'),
6676
new URL('https://example.com:444/'),
6777
],
6878

6979
// This one fails because our polyfilled URL implementation has IDNA stripped out.
7080
// [
7181
// true,
7282
// 'same domain, punycoded host',
73-
// new URL('https://example.xn--h2brj9c/#narrow/stream/jest'),
83+
// new URL('https://example.xn--h2brj9c/#narrow/channel/jest'),
7484
// new URL('https://example.भारत/'),
7585
// ], // FAILS
7686
[
7787
true,
7888
'punycodable host',
79-
new URL('#narrow/stream/jest', 'https://example.भारत/'),
89+
new URL('#narrow/channel/jest', 'https://example.भारत/'),
8090
new URL('https://example.भारत/'),
8191
],
8292

8393
// This one fails because our polyfilled URL implementation has IDNA stripped out.
8494
// [
8595
// true,
8696
// 'same domain, IDNA-mappable',
87-
// new URL('https://ℯⅩªm🄿ₗℰ.ℭᴼⓂ/#narrow/stream/jest'),
97+
// new URL('https://ℯⅩªm🄿ₗℰ.ℭᴼⓂ/#narrow/channel/jest'),
8898
// new URL('https://example.com'),
8999
// ], // FAILS
90100

91101
[
92102
true,
93103
'ipv4 address',
94-
new URL('#narrow/stream/jest', 'http://192.168.0.1/'),
104+
new URL('#narrow/channel/jest', 'http://192.168.0.1/'),
95105
new URL('http://192.168.0.1/'),
96106
],
97107
// This one fails because our polyfilled URL implementation has IDNA stripped out.
98108
// [
99109
// true,
100110
// 'same IPv4 address, IDNA-mappable',
101-
// new URL('http://１𝟗𝟚。①⁶🯸．₀｡𝟭/#narrow/stream/jest'),
111+
// new URL('http://１𝟗𝟚。①⁶🯸．₀｡𝟭/#narrow/channel/jest'),
102112
// new URL('http://192.168.0.1/'),
103113
// ], // FAILS
104114

@@ -111,17 +121,17 @@ describe('isNarrowLink', () => {
111121
// This one, except possibly the fragment, is a 100% realistic link
112122
// for innocent normal use. The buggy old version narrowly avoided
113123
// accepting it... but would accept all the variations below.
114-
new URL(`https://web.archive.org/web/*/${urlOnRealm('#narrow/stream/jest').toString()}`),
124+
new URL(`https://web.archive.org/web/*/${urlOnRealm('#narrow/channel/jest').toString()}`),
115125
],
116126
[
117127
false,
118128
'odd scheme, wrong domain, realm-like path, narrow-like fragment',
119-
new URL(`ftp://web.archive.org/web/*/${urlOnRealm('#narrow/stream/jest').toString()}`),
129+
new URL(`ftp://web.archive.org/web/*/${urlOnRealm('#narrow/channel/jest').toString()}`),
120130
],
121131
[
122132
false,
123133
'same domain, realm-like path, narrow-like fragment',
124-
urlOnRealm(`web/*/${urlOnRealm('#narrow/stream/jest').toString()}`),
134+
urlOnRealm(`web/*/${urlOnRealm('#narrow/channel/jest').toString()}`),
125135
],
126136
];
127137

@@ -138,10 +148,10 @@ describe('isNarrowLink', () => {
138148
describe('getNarrowFromNarrowLink (part 1)', () => {
139149
const mkCheck = (narrowExpectation: (Narrow => boolean) | null) => hash => {
140150
const streams = [
141-
eg.makeStream({ name: 'jest' }),
142-
eg.makeStream({ name: 'stream' }),
143-
eg.makeStream({ name: 'topic' }),
144-
eg.makeStream({ name: 'mobile' }),
151+
eg.makeStream({ stream_id: 1, name: 'jest' }),
152+
eg.makeStream({ stream_id: 2, name: 'stream' }),
153+
eg.makeStream({ stream_id: 3, name: 'topic' }),
154+
eg.makeStream({ stream_id: 4, name: 'mobile' }),
145155
];
146156
const baseState = eg.reduxStatePlus({ streams });
147157
const narrow = getNarrowFromNarrowLink(
@@ -163,6 +173,13 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
163173
}
164174
};
165175

176+
describe('"/#narrow/channel/<…>" is a channel link', () => {
177+
const check = mkCheck(isStreamNarrow);
178+
['/#narrow/channel/1-jest', '/#narrow/channel/2-stream/', '/#narrow/channel/3-topic/'].forEach(
179+
hash => check(hash),
180+
);
181+
});
182+
166183
describe('"/#narrow/stream/<…>" is a stream link', () => {
167184
const check = mkCheck(isStreamNarrow);
168185
['/#narrow/stream/jest', '/#narrow/stream/stream/', '/#narrow/stream/topic/'].forEach(hash =>
@@ -172,6 +189,16 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
172189
// TODO: Test with modern-style stream links that use stream IDs
173190
});
174191

192+
describe('"/#narrow/channel/<…>/topic/<…>" is a topic link', () => {
193+
const check = mkCheck(isTopicNarrow);
194+
[
195+
'/#narrow/channel/1-jest/topic/test',
196+
'/#narrow/channel/4-mobile/subject/topic/near/378333',
197+
'/#narrow/channel/4-mobile/topic/topic/',
198+
'/#narrow/channel/3-stream/topic/topic/near/1',
199+
].forEach(hash => check(hash));
200+
});
201+
175202
describe('"/#narrow/stream/<…>/topic/<…>" is a topic link', () => {
176203
const check = mkCheck(isTopicNarrow);
177204
[
@@ -216,14 +243,14 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
216243
const check = mkCheck(null);
217244
[
218245
// `near` with no operand
219-
'/#narrow/stream/stream/topic/topic/near/',
246+
'/#narrow/channel/stream/topic/topic/near/',
220247

221248
// `is` with invalid operand
222249
'/#narrow/is/men',
223-
'/#narrow/is/men/stream',
250+
'/#narrow/is/men/channel',
224251

225-
// invalid operand `are`; `stream` operator with no operand
226-
'/#narrow/are/men/stream',
252+
// invalid operand `are`; `channel` operator with no operand
253+
'/#narrow/are/men/channel',
227254
].forEach(hash => check(hash));
228255
});
229256
});
@@ -263,8 +290,11 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
263290
eg.selfUser.user_id,
264291
);
265292

266-
describe('on stream links', () => {
293+
describe('on channel links', () => {
267294
const expectStream = (operand, streams, expectedStream: null | Stream) => {
295+
expect(get(`#narrow/channel/${operand}`, streams)).toEqual(
296+
expectedStream === null ? null : streamNarrow(expectedStream.stream_id),
297+
);
268298
expect(get(`#narrow/stream/${operand}`, streams)).toEqual(
269299
expectedStream === null ? null : streamNarrow(expectedStream.stream_id),
270300
);
@@ -278,12 +308,12 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
278308
expectStream(`${streamGeneral.stream_id}-general`, [], streamGeneral);
279309
});
280310

281-
test('on stream link with wrong name: ID wins', () => {
311+
test('on channel link with wrong name: ID wins', () => {
282312
expectStream(`${streamGeneral.stream_id}-nonsense`, [streamGeneral], streamGeneral);
283313
expectStream(`${streamGeneral.stream_id}-`, [streamGeneral], streamGeneral);
284314
});
285315

286-
test('on malformed stream link: reject', () => {
316+
test('on malformed channel link: reject', () => {
287317
expectStream(`-${streamGeneral.stream_id}`, [streamGeneral], null);
288318
expectStream(`${streamGeneral.stream_id}nonsense-general`, [streamGeneral], null);
289319
});
@@ -340,17 +370,21 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
340370
describe('on topic links', () => {
341371
test('basic', () => {
342372
const expectBasic = (operand, expectedTopic) => {
343-
const url = `#narrow/stream/${streamGeneral.stream_id}-general/topic/${operand}`;
373+
const url = `#narrow/channel/${streamGeneral.stream_id}-general/topic/${operand}`;
374+
const legacyUrl = `#narrow/stream/${streamGeneral.stream_id}-general/topic/${operand}`;
344375
expect(get(url, [streamGeneral])).toEqual(
345376
topicNarrow(streamGeneral.stream_id, expectedTopic),
346377
);
378+
expect(get(legacyUrl, [streamGeneral])).toEqual(
379+
topicNarrow(streamGeneral.stream_id, expectedTopic),
380+
);
347381
};
348382

349383
expectBasic('(no.20topic)', '(no topic)');
350384
expectBasic('lunch', 'lunch');
351385
});
352386

353-
test('on old topic link, with dot-encoding', () => {
387+
test('on old topic link (no stream ID), with dot-encoding', () => {
354388
expect(
355389
get(`https://example.com/#narrow/stream/${eg.stream.name}/topic/(no.20topic)`, [eg.stream]),
356390
).toEqual(topicNarrow(eg.stream.stream_id, '(no topic)'));
@@ -376,7 +410,7 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
376410
).toEqual(topicNarrow(eg.stream.stream_id, 'topic'));
377411
});
378412

379-
test('on old topic link, without realm info', () => {
413+
test('on old topic link (no stream ID), without realm info', () => {
380414
expect(get(`/#narrow/stream/${eg.stream.name}/topic/topic`, [eg.stream])).toEqual(
381415
topicNarrow(eg.stream.stream_id, 'topic'),
382416
);
@@ -423,6 +457,13 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
423457
pmNarrowFromUsersUnsafe([userB, userC]),
424458
);
425459

460+
expect(
461+
get(
462+
`https://example.com/#narrow/channel/${eg.stream.stream_id}-${eg.stream.name}/topic/test/near/1`,
463+
[eg.stream],
464+
),
465+
).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
466+
426467
expect(
427468
get(`https://example.com/#narrow/stream/${eg.stream.name}/topic/test/near/1`, [eg.stream]),
428469
).toEqual(topicNarrow(eg.stream.stream_id, 'test'));
@@ -436,6 +477,7 @@ describe('getNarrowFromNarrowLink (part 2)', () => {
436477
describe('getNearOperandFromLink', () => {
437478
test('not message link', () => {
438479
expect(getNearOperandFromLink(new URL('/#narrow/is/private', realm), realm)).toBe(null);
480+
expect(getNearOperandFromLink(new URL('/#narrow/channel/jest', realm), realm)).toBe(null);
439481
expect(getNearOperandFromLink(new URL('/#narrow/stream/jest', realm), realm)).toBe(null);
440482
});
441483

@@ -451,6 +493,9 @@ describe('getNearOperandFromLink', () => {
451493
});
452494

453495
test('when link is a topic link, return anchor message id', () => {
496+
expect(
497+
getNearOperandFromLink(new URL('/#narrow/channel/1-jest/topic/test/near/1', realm), realm),
498+
).toBe(1);
454499
expect(
455500
getNearOperandFromLink(new URL('/#narrow/stream/jest/topic/test/near/1', realm), realm),
456501
).toBe(1);

‎src/utils/internalLinks.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,19 @@ export const getNarrowFromNarrowLink = (
161161

162162
if (
163163
(hashSegments.length === 4 || hashSegments.length === 6)
164-
&& hashSegments[0] === 'stream'
164+
// 'channel' is new in server-9.0; means the same as 'stream'
165+
&& (hashSegments[0] === 'stream' || hashSegments[0] === 'channel')
165166
&& (hashSegments[2] === 'subject' || hashSegments[2] === 'topic')
166167
) {
167168
const streamId = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
168169
return streamId != null ? topicNarrow(streamId, parseTopicOperand(hashSegments[3])) : null;
169170
}
170171

171-
if (hashSegments.length === 2 && hashSegments[0] === 'stream') {
172+
if (
173+
hashSegments.length === 2
174+
// 'channel' is new in server-9.0; means the same as 'stream'
175+
&& (hashSegments[0] === 'stream' || hashSegments[0] === 'channel')
176+
) {
172177
const streamId = parseStreamOperand(hashSegments[1], streamsById, streamsByName);
173178
return streamId != null ? streamNarrow(streamId) : null;
174179
}

0 commit comments

Comments
 (0)