Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JWT refactor #227

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

JWT refactor #227

wants to merge 2 commits into from

Conversation

joncombe
Copy link
Collaborator

@joncombe joncombe commented Nov 14, 2024

Summary of changes

  • Refactored JWT token handling and calls direct to the cluster. All JWT related code is now handled in a single zustand store, JWTManager.
  • GCContextProvider is no longer needed and have been removed.
  • The schema tree is now loaded via SWR and so is cached in memory (no more waiting for it to load each visit to the page). As a result, the SchemaTreeContextProvider is no longer required and has been removed.
  • Migrated the TableShards route to this repo from cloud-ui.

Checklist

  • Link to issue this PR refers to: https://github.com/crate/cloud/issues/2222
  • Relevant changes are reflected in CHANGES.md.
  • Added or changed code is covered by tests.
  • Required Grand Central APIs are already merged.

@cla-bot cla-bot bot added the cla-signed label Nov 14, 2024
@joncombe joncombe force-pushed the jc/jwt-refactor branch 9 times, most recently from e8085fd to f6ff581 Compare November 15, 2024 08:59
@joncombe joncombe marked this pull request as ready for review November 15, 2024 10:55
@alexdametto alexdametto self-requested a review November 18, 2024 08:12
Copy link
Collaborator

@alexdametto alexdametto left a comment

Choose a reason for hiding this comment

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

Amazing job 💯 This will also improve DX a lot.

@@ -1,10 +1,4 @@
export enum ConnectionStatus {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good job with this refactor, there were 2 of almost the same type. Thanks!


const getSchemaTableColumns = () => {
const schemaTableColumnsParsed = schemaTableColumnMock.rows.map(
const schemaTableColumnsParsed = useSchemaTreeMock.rows.map(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't know if it's the same mapping but if it is we could export the postFetch method and use it here just to avoid code duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants