diff --git a/src/message/__tests__/messageActions-test.js b/src/message/__tests__/messageActions-test.js index 984d0e32178..b513743bf0a 100644 --- a/src/message/__tests__/messageActions-test.js +++ b/src/message/__tests__/messageActions-test.js @@ -68,6 +68,21 @@ describe('messageActions', () => { pmNarrowFromUsersUnsafe([user1, user2]), ); }); + + test('handles /with/ links', async () => { + const stream = eg.makeStream({ stream_id: 1, name: 'test' }); + const user1 = eg.makeUser({ user_id: 1, full_name: 'user 1' }); + const user2 = eg.makeUser({ user_id: 2, full_name: 'user 2' }); + const { store } = prepare({ streams: [stream], users: [user1, user2] }); + + await checkLink(store, '#narrow/stream/1-test/topic/hello/with/1', topicNarrow(1, 'hello')); + await checkLink(store, '#narrow/dm/1-user-1/with/1', pmNarrowFromUsersUnsafe([user1])); + await checkLink( + store, + '#narrow/dm/1,2-group/with/1', + pmNarrowFromUsersUnsafe([user1, user2]), + ); + }); }); describe('doNarrow', () => { diff --git a/src/utils/__tests__/internalLinks-test.js b/src/utils/__tests__/internalLinks-test.js index 1be1336a073..27417524455 100644 --- a/src/utils/__tests__/internalLinks-test.js +++ b/src/utils/__tests__/internalLinks-test.js @@ -194,8 +194,10 @@ describe('getNarrowFromNarrowLink (part 1)', () => { [ '/#narrow/channel/1-jest/topic/test', '/#narrow/channel/4-mobile/subject/topic/near/378333', + '/#narrow/channel/4-mobile/subject/topic/with/378333', '/#narrow/channel/4-mobile/topic/topic/', '/#narrow/channel/3-stream/topic/topic/near/1', + '/#narrow/channel/3-stream/topic/topic/with/1', ].forEach(hash => check(hash)); }); @@ -204,9 +206,12 @@ describe('getNarrowFromNarrowLink (part 1)', () => { [ '/#narrow/stream/jest/topic/test', '/#narrow/stream/mobile/subject/topic/near/378333', + '/#narrow/stream/mobile/subject/topic/with/378333', '/#narrow/stream/mobile/topic/topic/', '/#narrow/stream/stream/topic/topic/near/1', + '/#narrow/stream/stream/topic/topic/with/1', '/#narrow/stream/stream/subject/topic/near/1', + '/#narrow/stream/stream/subject/topic/with/1', '/#narrow/stream/stream/subject/topic', ].forEach(hash => check(hash)); }); @@ -216,7 +221,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => { [ '/#narrow/dm/1,2-group', '/#narrow/dm/1,2-group/near/1', + '/#narrow/dm/1,2-group/with/1', '/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', + '/#narrow/dm/a.40b.2Ecom.2Ec.2Ed.2Ecom/with/3', ].forEach(hash => check(hash)); }); @@ -225,7 +232,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => { [ '/#narrow/pm-with/1,2-group', '/#narrow/pm-with/1,2-group/near/1', + '/#narrow/pm-with/1,2-group/with/1', '/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/near/3', + '/#narrow/pm-with/a.40b.2Ecom.2Ec.2Ed.2Ecom/with/3', ].forEach(hash => check(hash)); }); @@ -245,6 +254,9 @@ describe('getNarrowFromNarrowLink (part 1)', () => { // `near` with no operand '/#narrow/channel/stream/topic/topic/near/', + // `with` with no operand + '/#narrow/channel/stream/topic/topic/with/', + // `is` with invalid operand '/#narrow/is/men', '/#narrow/is/men/channel', @@ -472,6 +484,31 @@ describe('getNarrowFromNarrowLink (part 2)', () => { get(`https://example.com/#narrow/stream/${eg.stream.name}/subject/test/near/1`, [eg.stream]), ).toEqual(topicNarrow(eg.stream.stream_id, 'test')); }); + + test('on a /with/ link', () => { + const ids = `${userB.user_id},${userC.user_id}`; + expect(get(`https://example.com/#narrow/dm/${ids}-group/with/2`, [])).toEqual( + pmNarrowFromUsersUnsafe([userB, userC]), + ); + expect(get(`https://example.com/#narrow/pm-with/${ids}-group/with/2`, [])).toEqual( + pmNarrowFromUsersUnsafe([userB, userC]), + ); + + expect( + get( + `https://example.com/#narrow/channel/${eg.stream.stream_id}-${eg.stream.name}/topic/test/with/1`, + [eg.stream], + ), + ).toEqual(topicNarrow(eg.stream.stream_id, 'test')); + + expect( + get(`https://example.com/#narrow/stream/${eg.stream.name}/topic/test/with/1`, [eg.stream]), + ).toEqual(topicNarrow(eg.stream.stream_id, 'test')); + + expect( + get(`https://example.com/#narrow/stream/${eg.stream.name}/subject/test/with/1`, [eg.stream]), + ).toEqual(topicNarrow(eg.stream.stream_id, 'test')); + }); }); describe('getNearOperandFromLink', () => { @@ -485,6 +522,10 @@ describe('getNearOperandFromLink', () => { expect(getNearOperandFromLink(new URL('/#narrow/near/1', realm), realm)).toBe(1); }); + test('`with` is the only operator', () => { + expect(getNearOperandFromLink(new URL('/#narrow/with/1', realm), realm)).toBe(1); + }); + test('when link is a group link, return anchor message id', () => { expect(getNearOperandFromLink(new URL('/#narrow/dm/1,3-group/near/1/', realm), realm)).toBe(1); expect( @@ -492,6 +533,13 @@ describe('getNearOperandFromLink', () => { ).toBe(1); }); + test('when link is a group link with /with/, return anchor message id', () => { + expect(getNearOperandFromLink(new URL('/#narrow/dm/1,3-group/with/1/', realm), realm)).toBe(1); + expect( + getNearOperandFromLink(new URL('/#narrow/pm-with/1,3-group/with/1/', realm), realm), + ).toBe(1); + }); + test('when link is a topic link, return anchor message id', () => { expect( getNearOperandFromLink(new URL('/#narrow/channel/1-jest/topic/test/near/1', realm), realm), @@ -500,4 +548,13 @@ describe('getNearOperandFromLink', () => { getNearOperandFromLink(new URL('/#narrow/stream/jest/topic/test/near/1', realm), realm), ).toBe(1); }); + + test('when link is a topic link with /with/, return anchor message id', () => { + expect( + getNearOperandFromLink(new URL('/#narrow/channel/1-jest/topic/test/with/1', realm), realm), + ).toBe(1); + expect( + getNearOperandFromLink(new URL('/#narrow/stream/jest/topic/test/with/1', realm), realm), + ).toBe(1); + }); }); diff --git a/src/utils/internalLinks.js b/src/utils/internalLinks.js index e153ef4d776..10da8b8d450 100644 --- a/src/utils/internalLinks.js +++ b/src/utils/internalLinks.js @@ -148,7 +148,7 @@ export const getNarrowFromNarrowLink = ( (hashSegments.length === 2 && (hashSegments[0] === 'pm-with' || hashSegments[0] === 'dm')) || (hashSegments.length === 4 && (hashSegments[0] === 'pm-with' || hashSegments[0] === 'dm') - && hashSegments[2] === 'near') + && (hashSegments[2] === 'near' || hashSegments[2] === 'with')) ) { // TODO: This case is pretty useless in practice, due to basically a // bug in the webapp: the URL that appears in the location bar for a @@ -167,7 +167,7 @@ export const getNarrowFromNarrowLink = ( || (hashSegments.length === 6 && (hashSegments[0] === 'stream' || hashSegments[0] === 'channel') && (hashSegments[2] === 'subject' || hashSegments[2] === 'topic') - && hashSegments[4] === 'near') + && (hashSegments[4] === 'near' || hashSegments[4] === 'with')) ) { const streamId = parseStreamOperand(hashSegments[1], streamsById, streamsByName); return streamId != null ? topicNarrow(streamId, parseTopicOperand(hashSegments[3])) : null; @@ -219,9 +219,18 @@ export const getNearOperandFromLink = (url: URL, realm: URL): number | null => { (str, i) => // This is a segment where we expect an operator to be specified. i % 2 === 0 - // The operator is 'near' and its meaning is not negated (`str` does - // not start with "-"). - && str === 'near', + // The operator is 'near' or 'with' and its meaning is not negated + // (`str` does not start with "-"). + // + // Quoting Greg from #5866: + // > Currently the app doesn't interpret the "anchor" meaning of + // > `/near/` links at all (that's #3604) — we already effectively + // > give `/near/` links exactly the meaning that the upcoming + // > `/with/` links will have. So to handle the new links, we just + // > need to parse them and then handle them the way we already handle + // > `/near/` links. + // Which makes sense for this legacy codebase. + && (str === 'near' || str === 'with'), ); if (nearOperatorIndex < 0) { return null;