Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix(svelte): Properly redirect to cody marketing page#64331

Merged
fkling merged 1 commit intomainfrom
fkling-srch-851-clicking-on-cody-on-dotcom-when-not-signed-in-doesnt
Aug 7, 2024
Merged

fix(svelte): Properly redirect to cody marketing page#64331
fkling merged 1 commit intomainfrom
fkling-srch-851-clicking-on-cody-on-dotcom-when-not-signed-in-doesnt

Conversation

@fkling
Copy link
Contributor

@fkling fkling commented Aug 7, 2024

Fixes srch-851

A consequence of https://github.com/sourcegraph/sourcegraph/pull/64272 is that redirecting to the cody marketing page on dotcom didn't work. That's because /cody is not provided by the server as a known page.

A simple fix would be to mark the link as external, but we'd have to keep in mind to do this in all places (present and future).

A more "central" fix is to add this page to a hardcoded list of known pages that are not provided by the server.

Test plan

Manual testing

A consequence of https://github.com/sourcegraph/sourcegraph/pull/64272
is that redirecting to the cody marketing page on dotcom didn't work.
That's because `/cody` is not provided by the server as a known page.

A simple fix would be to mark the link as external, but we'd have to
keep in mind to do this in all places (present and future).

A more "central" fix is to add this page to a hardcoded list of known
pages that are not provided by the server.
@fkling fkling requested a review from a team August 7, 2024 12:24
@cla-bot cla-bot bot added the cla-signed label Aug 7, 2024
@fkling fkling requested a review from a team August 7, 2024 13:21
@fkling fkling merged commit e4e1f55 into main Aug 7, 2024
@fkling fkling deleted the fkling-srch-851-clicking-on-cody-on-dotcom-when-not-signed-in-doesnt branch August 7, 2024 13:24
// Having a separate list is error-prone and should be avoided if possible.
const additionalKnownRoutes: string[] = [
// Cody's marketing page
'^/cody/?$',
Copy link
Member

Choose a reason for hiding this comment

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

Are there others here? e.g. routes handled by cloudflare and not the webapp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably. I tested the dotcom bottom links and they all work fine because we open them in a new tab.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants