Conversation
chrsmith
left a comment
There was a problem hiding this comment.
I don't believe this is correct, and will break things if we merge it.
The subscription/CodySubscriptionPage.tsx is shown on sourcegraph.com (here), and is live. So we have to have this continue to redirect the user to https://accounts.sourcegraph.com/cody, right? Otherwise if we merge this it will essentially break things for all users?
When we go live with the new UI, we will need to replace all of these links. But shouldn't we just update the manageSubscriptionRedirectURL value to (depending on the site configuration) point to the new location?
That way, it will continue to work for Sourcegraph.com, but for the cody-services-dev.sgdev.dev it will do the right thing.
Or am I just mistaken, and the CodySubscriptionPage is part of our "new UI". (Which in that case, yes, we can just link directly to the page and not need to bother with manageSubscriptionRedirectURL.)
|
@chrsmith, take a closer look: if you extend the code right above my changes, you'll see it's all within a |
There was a problem hiding this comment.
I see. I absolutely did not notice that!
... but taking a closer look at the code, this is (at least to me) confusing enough that we should rewrite it.
I'll aprove this so we can just move on and start testing things. But we need to have a firm definition of what is and is not acceptable for the React codebase.
In this case, I'd argue this definitely isn't acceptable. In that it's super confusing and easy to miss really important details... such as me being concerned this will break things, but it is actually just fine.
{isProUser ? (
<ManyLinesOfReactComponents>
) : useEmbeddedCodyUI ? (
<ManyLinesOfReactComponents>
) : (
<ManyLinesOfReactComponents>
)}
When written like this it is impossible to reason about the context some code renders in without carefully walking back N-lines of the source tree, and paying super-close attention to elements like ) : ( or ) : useEmbeddedCodyUI ? (.
|
@chrsmith, yup, let's get back to this and make it an org-wide discussion. |
The "Create team" and "Upgrade to Pro" links currently point to the old SSC site.
This PR makes both of them point to the new PLG checkout experience, behind the feature flag.
Test plan
Tested it manually, both buttons work now as they should
Changelog
fix(plg): make upgrade links point to the right pages for Teams & Invites