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

Fix woes related to undefined frontendCodyProConfig.sscBaseUrl#62835

Merged
chrsmith merged 1 commit into
mainfrom
chrsmith/fix-broken-links
May 21, 2024
Merged

Fix woes related to undefined frontendCodyProConfig.sscBaseUrl#62835
chrsmith merged 1 commit into
mainfrom
chrsmith/fix-broken-links

Conversation

@chrsmith

Copy link
Copy Markdown
Contributor

We need a URL to point the user to the webpage to manage their Cody Pro subscription in a couple places. And to support testing in lower environments, we made that configurable in https://github.com/sourcegraph/sourcegraph/pull/62790/files#diff-70a08658447c0eeaf1f548b8e325cac30a08567323d32e50b1953786404a5b07.

There was a problem in that PR however, the "default" value in the site.schema.json doesn't do what you'd think. So at runtime the value for frontendCodyProConfig.sscBaseUrl was always always the empty string. This leads to confusing errors whenever you try to manage your Cody Pro subscription.

I tried to just update the Sourcegraph.com site config to supply that value like intended, however in doing that the Sourcegraph.com frontend jobs would start to crash loop. (Because only supplying the sscBaseUrl config value means that the codyProConfig settings contain "invalid values" for sscBackendOrigin.

... since things have been broken in production for XX hours now, and I'd like to making any logic changes under duress, this PR just backs out the problematic line of code in that earlier PR. (So instead of reading the uninitialized config setting, it just hard-codes the default value.)

The ideal fix for this, IMHO, would be to expose a new config settings showEmbeddedUi so that the function isEmbeddedCodyProUIEnabled() can rely on that. Since that way, we can supply all of the codyProConfig configuration data for Sourcegraph.com, without any fears of unintentionally enabling the new UI. Otherwise, it seems dangerous to set all of the config values, but somehow not the Stripe Publishable key because that is the thing that controls the UI we display.

Test Plan

Tested locally.

@chrsmith chrsmith requested a review from vdavid May 21, 2024 23:06
@cla-bot cla-bot Bot added the cla-signed label May 21, 2024
@chrsmith chrsmith requested review from bobheadxi and unknwon May 21, 2024 23:07
@chrsmith chrsmith enabled auto-merge (squash) May 21, 2024 23:12
@chrsmith chrsmith merged commit 59b5156 into main May 21, 2024
@chrsmith chrsmith deleted the chrsmith/fix-broken-links branch May 21, 2024 23:22
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.

2 participants