Skip to content

internalLinks: Recognize "channel" as operator in narrow links #5862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

chrisbobbe
Copy link
Contributor

Fixes: #5860

@chrisbobbe chrisbobbe added P1 high-priority webapp parity Features that exist in the webapp that we need to port to mobile. We aren't aiming for full parity. server release goal Things we should try to coordinate with a major Zulip Server release. labels Apr 24, 2024
@chrisbobbe chrisbobbe requested a review from gnprice April 24, 2024 01:08
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chrisbobbe! Looks good; merging.

Comment on lines -141 to +154
eg.makeStream({ name: 'jest' }),
eg.makeStream({ name: 'stream' }),
eg.makeStream({ name: 'topic' }),
eg.makeStream({ name: 'mobile' }),
eg.makeStream({ stream_id: 1, name: 'jest' }),
eg.makeStream({ stream_id: 2, name: 'stream' }),
eg.makeStream({ stream_id: 3, name: 'topic' }),
eg.makeStream({ stream_id: 4, name: 'mobile' }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm interesting — why do these become newly needed?

@@ -172,6 +189,16 @@ describe('getNarrowFromNarrowLink (part 1)', () => {
// TODO: Test with modern-style stream links that use stream IDs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's because we're effectively doing this at the same time.

Sure, seems fine given this is a legacy codebase we don't want to spend too much effort polishing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's right.

@gnprice gnprice merged commit c5b7a78 into zulip:main Apr 25, 2024
@chrisbobbe chrisbobbe deleted the pr-recognize-narrow-link-channel-operator branch April 25, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 high-priority server release goal Things we should try to coordinate with a major Zulip Server release. webapp parity Features that exist in the webapp that we need to port to mobile. We aren't aiming for full parity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recognize "channel" as operator in narrow links
2 participants