SSC: Three small fixes for Teams & invites#62818
Conversation
chrsmith
left a comment
There was a problem hiding this comment.
Just one thing that may be A-OK and I'm just confused about, but otherwise all good things!
| <Link | ||
| className="text-center" | ||
| to="https://sourcegraph.com/contact/request-info?utm_source=cody_subscription_page" | ||
| to={manageSubscriptionRedirectURL} |
There was a problem hiding this comment.
export const manageSubscriptionRedirectURL = `${window.context.frontendCodyProConfig?.sscBaseUrl}/subscription`So the current way this is configured, this will redirect users to https://accounts.sourcegraph.com/cody/subscription. ( ✅ )
But down the road, do we plan on setting sscBaseUrl to https://sourcegraph.com/cody/manage? And then expect the /cody/manage/subscription page to redirect to /cody/manage/subscription/new if the user doesn't have an active Cody Pro subscription? Since (IIRC) we don't have a /cody/manage/subscription page wired up yet, that seems like maybe a dangerous assumption to make?
So for clarity, would we want to export a new createNewCodyProSubscriptionRedirectUrl just to remove any ambiguity? Since in this case, we expect the user to create a new subscription/Cody Pro team, not manage an existing one.
There was a problem hiding this comment.
JFYI, and perhaps this will fix it, it looks like this is broken. (At least in our dev environment.) It's very likely I have misconfigured something.
https://cody-services-dev.sgdev.dev/subscription shows this link:
And the "Upgrade yourself to Pro" points to: https://cody-services-dev.sgdev.dev/subscription which 404s. (Which is surprising, since I didn't set the sscBackendUrl value in the site config yet. So it should have defaulted to an entirely different domain, accounts.sourcegraph.com...)
Update: After setting the sscBackendUrl the link now works, but it redirects to the exact same page (with the behavior that is checked-in now).
There was a problem hiding this comment.
But down the road, do we plan on setting sscBaseUrl to https://sourcegraph.com/cody/manage?
I mean, if we want to migrate this to some other URL, we can change the definition of manageSubscriptionRedirectURL, right? So I'd not be too forward-looking with this.
Update: After setting the sscBackendUrl the link now works, but it redirects to the exact same page (with the behavior that is checked-in now).
Did you figure out what was the problem? TBH I didn't fully get what you meant was wrong here.
Your PR https://github.com/sourcegraph/sourcegraph/pull/62835 fixes the behavior when sscBackedUrl is not defined, but when it is defined, the link worked in my tests.
Finally, I could start testing the experience end-to-end.
I found the first few mistakes during that.
Three changes:
event.preventDefault()was not called.The three changes are three commits in this PR to make the review easier.
Test plan
Tested manually.