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

chore(plg): provide user Cody plan on a route level #63409

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

taras-yemets
Copy link
Contributor

@taras-yemets taras-yemets commented Jun 20, 2024

1. Lifts the user Cody plan query to Sourcegraph GraphQL backend up to the route level.

All Cody Pro pages require the user’s Cody Pro plan to determine their behavior. Previously, we had repetitive logic to fetch this data and handle loading and error states. This PR moves the fetching of the user’s Cody plan to a higher level, simplifying the code. Now, each Cody Pro page component receives the subscription data as a prop.

2. Creates a Custom Hook for Fetching the User’s Cody Plan.

3. Refactors "/cody/subscription" page component.

  • Updates the CodySubscriptionPage component to use the withAuthenticatedUser higher-order component. This change removes the need for an authenticated user check within the component itself, making it more consistent with other Cody Pro page components.
  • Removes redundant isCodyEnabled() check.

Test plan

  • CI
  • Run Sourcegraph in dotcom mode locally and compare the authenticated user related logic with sourcegraph.com

@cla-bot cla-bot bot added the cla-signed label Jun 20, 2024
@taras-yemets taras-yemets marked this pull request as ready for review June 20, 2024 23:28
@taras-yemets taras-yemets requested a review from a team June 20, 2024 23:28
Base automatically changed from ty/plg-remove-seats to main June 20, 2024 23:35
@taras-yemets taras-yemets changed the title Ty/plg-provide-user-plan-on-route-level chore(plg): provide user Cody plan on a route level Jun 20, 2024
Copy link
Contributor

@vdavid vdavid left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the refactor, so much nicer now.

Note: I think you forgot to finish or remove the "Changelog" section.


if (!data.currentUser) {
// Cody plan is not available if the user is not authenticated. Redirecting to the sign-in page.
return <Navigate to={`/sign-in?returnTo=${CodyProRoutes.Manage}`} replace={true} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I wonder if we can do better, and redirect the user back to the page they wanted to view, including the query params. Would be a lot helpful to users.
Could be out of scope for this PR, but if so, it'd be great to track this as a new gap.

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