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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@

## Unreleased

## 2024-11-14 - 0.18.0

- Dependabot: bump react-router-dom from 6.24.1 to 6.28.0.
- Dependabot: bump prettier from 3.3.2 to 3.3.3.
- Dependabot: bump prettier-plugin-tailwindcss from 0.5.14 to 0.6.8.
- Refactored JWT token handling and calls direct to the cluster.
- Migrated the TableShards route to this repo from cloud-ui.

## 2024-11-11 - 0.17.2

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@crate.io/crate-gc-admin",
"version": "0.17.2",
"version": "0.18.0",
"author": "crate.io",
"private": false,
"type": "module",
Expand Down
78 changes: 35 additions & 43 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,71 +1,63 @@
import { Link, Route, Routes } from 'react-router-dom';
import { bottomNavigation, topNavigation } from 'constants/navigation';
import routes from 'constants/routes';
import { useMemo, useState } from 'react';
import {
ConnectionStatus,
GCContextProvider,
SchemaTreeContextProvider,
} from 'contexts';
import { useEffect } from 'react';
import { ConnectionStatus } from 'types';
import { Layout, StatusBar, StatsUpdater } from 'components';
import logo from './assets/logo.svg';
import useGcApi from 'hooks/useGcApi';
import { apiGet } from 'utils/api';
import { GRAND_CENTRAL_SESSION_TOKEN_KEY } from 'constants/session';
import NotificationHandler from 'components/NotificationHandler';
import { root } from 'constants/paths';
import useJWTManagerStore from 'state/jwtManager';

function App() {
const [gcStatus, setGCStatus] = useState(ConnectionStatus.PENDING);
const hasToken = useJWTManagerStore(state => state.hasToken);
const setGcStatus = useJWTManagerStore(state => state.setGcStatus);
const gcApi = useGcApi();

useMemo(() => {
useEffect(() => {
if (!hasToken()) {
setGcStatus(ConnectionStatus.NOT_LOGGED_IN);
return;
}

apiGet(gcApi, `/api/`)
.then(res => {
if (res.status == 200) {
setGCStatus(ConnectionStatus.CONNECTED);
setGcStatus(ConnectionStatus.CONNECTED);
} else if (res.status == 401) {
setGCStatus(ConnectionStatus.NOT_LOGGED_IN);
setGcStatus(ConnectionStatus.NOT_LOGGED_IN);
} else {
setGCStatus(ConnectionStatus.ERROR);
setGcStatus(ConnectionStatus.ERROR);
}
})
.catch(() => {
setGCStatus(ConnectionStatus.ERROR);
setGcStatus(ConnectionStatus.ERROR);
});
}, []);

const gcUrl = process.env.REACT_APP_GRAND_CENTRAL_URL;
const crateUrl = process.env.REACT_APP_CRATE_URL;

return (
<GCContextProvider
gcStatus={gcStatus}
gcUrl={gcUrl}
crateUrl={crateUrl}
sessionTokenKey={GRAND_CENTRAL_SESSION_TOKEN_KEY}
>
<SchemaTreeContextProvider>
<StatsUpdater />
<Layout
topbarLogo={
<Link to={root.build()}>
<img alt="CrateDB logo" src={logo} />
</Link>
}
topbarContent={<StatusBar />}
bottomNavigation={bottomNavigation}
topNavigation={topNavigation}
>
<Routes>
{routes.map(route => (
<Route key={route.path} path={route.path} element={route.element} />
))}
</Routes>
</Layout>
<NotificationHandler />
</SchemaTreeContextProvider>
</GCContextProvider>
<>
<StatsUpdater />
<Layout
topbarLogo={
<Link to={root.build()}>
<img alt="CrateDB logo" src={logo} />
</Link>
}
topbarContent={<StatusBar />}
bottomNavigation={bottomNavigation}
topNavigation={topNavigation}
>
<Routes>
{routes.map(route => (
<Route key={route.path} path={route.path} element={route.element} />
))}
</Routes>
</Layout>
<NotificationHandler />
</>
);
}

Expand Down
5 changes: 3 additions & 2 deletions src/components/EnterpriseScreen/EnterpriseScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import React from 'react';
import { Result } from 'antd';
import Button from 'components/Button';
import { ConnectionStatus, useGCContext } from 'contexts';
import { ConnectionStatus } from 'types';
import useJWTManagerStore from 'state/jwtManager';

type EnterpriseScreenProps = {
children: React.ReactElement;
};

function EnterpriseScreen({ children }: EnterpriseScreenProps) {
const { gcStatus } = useGCContext();
const gcStatus = useJWTManagerStore(state => state.gcStatus);
if (gcStatus == ConnectionStatus.CONNECTED) {
return children;
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/GCSpin/GCSpin.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('The GCSpin component', () => {

it('will use specified test id', () => {
setup({
spinnedTestId: 'custom-test-id',
spinnerTestId: 'custom-test-id',
});

expect(screen.getByTestId('custom-test-id')).toBeInTheDocument();
Expand Down
6 changes: 3 additions & 3 deletions src/components/GCSpin/GCSpin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import { Spin } from 'antd';

export type GCSpinParams = PropsWithChildren<{
spinning: boolean;
spinnedTestId?: string;
spinnerTestId?: string;
}>;

function GCSpin({ children, spinning, spinnedTestId = 'spinner' }: GCSpinParams) {
function GCSpin({ children, spinning, spinnerTestId = 'spinner' }: GCSpinParams) {
if (!spinning) {
return children;
}
return <Spin data-testid={spinnedTestId} />;
return <Spin data-testid={spinnerTestId} />;
}

export default GCSpin;
5 changes: 3 additions & 2 deletions src/components/GCStatusIndicator/GCStatusIndicator.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConnectionStatus, useGCContext } from 'contexts';
import { ConnectionStatus } from 'types';
import useJWTManagerStore from 'state/jwtManager';

function renderStatus(status: ConnectionStatus) {
const prefix = 'GC';
Expand Down Expand Up @@ -27,7 +28,7 @@ function renderStatus(status: ConnectionStatus) {
}

function GCStatusIndicator() {
const { gcStatus } = useGCContext();
const gcStatus = useJWTManagerStore(state => state.gcStatus);

return renderStatus(gcStatus);
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/Layout/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useState } from 'react';
import type { LayoutProps } from './types';
import Navigation from './Navigation';
import TopBar from './TopBar';
import { useGCContext } from 'contexts';
import useJWTManagerStore from 'state/jwtManager';

function Layout({
bottomNavigation,
Expand All @@ -16,7 +16,7 @@ function Layout({
// but it would be just a case of passing the parameter should it be
// needed in future
const [navIsExpanded, setNavIsExpanded] = useState(true);
const { gcStatus } = useGCContext();
const gcStatus = useJWTManagerStore(state => state.gcStatus);

return (
<div className="absolute bottom-0 top-0 flex min-h-dvh w-full flex-col">
Expand Down
3 changes: 2 additions & 1 deletion src/components/Layout/Navigation.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConnectionStatus, NavigationProps, NavigationLinkProps } from './types';
import { ConnectionStatus } from 'types';
import { NavigationProps, NavigationLinkProps } from './types';
import { NavLink } from 'react-router-dom';
import {
ArrowUpOutlined,
Expand Down
8 changes: 1 addition & 7 deletions src/components/Layout/types.ts
Original file line number Diff line number Diff line change
@@ -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!

CONNECTED,
NOT_CONFIGURED,
NOT_LOGGED_IN,
ERROR,
PENDING,
}
import { ConnectionStatus } from 'types';

export type BurgerProps = {
navIsExpanded: boolean;
Expand Down
9 changes: 5 additions & 4 deletions src/components/SQLEditor/SQLEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import { Button } from 'components';
import { cn } from 'utils';
import { annotate } from './annotationUtils';
import { QueryStatus } from 'types/query';
import { useGCContext, useSchemaTreeContext } from 'contexts';
import SQLEditorSchemaTree from './SQLEditorSchemaTree';
import { useSchemaTree } from 'src/swr/jwt';
import useJWTManagerStore from 'state/jwtManager';

export type SQLEditorProps = {
value?: string | undefined | null;
Expand Down Expand Up @@ -52,8 +53,8 @@ function SQLEditor({
onViewHistory,
title,
}: SQLEditorProps) {
const { clusterId } = useGCContext();
const { refreshSchemaTree } = useSchemaTreeContext();
const clusterId = useJWTManagerStore(state => state.clusterId);
const { mutate: mutateSchemaTree } = useSchemaTree(clusterId);

const SQL_EDITOR_CONTENT_KEY =
localStorageKey && `crate.gc.admin.${localStorageKey}.${clusterId || ''}`;
Expand Down Expand Up @@ -112,7 +113,7 @@ function SQLEditor({

// refresh the schema tree if we believe the schema has changed
if (results && resultsIncludeSuccessfulDDLQuery(results)) {
refreshSchemaTree();
mutateSchemaTree();
}

// Compute and set Ace Editor annotations
Expand Down
43 changes: 23 additions & 20 deletions src/components/SQLEditor/SQLEditorSchemaTree.test.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
import { render, screen, waitFor, within } from 'test/testUtils';
import SQLEditorSchemaTree from './SQLEditorSchemaTree';
import { UserEvent } from '@testing-library/user-event';
import {
schemaTableColumnMock,
schemaTablesNonSystemMock,
} from 'test/__mocks__/schemaTableColumn';
import { SYSTEM_SCHEMAS } from 'constants/database';
import _ from 'lodash';
import { format as formatSQL } from 'sql-formatter';
import {
getTablesDDLQueryResult,
getViewsDDLQueryResult,
} from 'test/__mocks__/query';
import { SchemaDescription } from 'contexts';
import { useSchemaTreeMock } from 'test/__mocks__/useSchemaTreeMock';
import { SchemaDescription } from 'src/swr/jwt/useSchemaTree';

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.

r =>
({
table_schema: r[0] as string,
Expand Down Expand Up @@ -80,6 +77,8 @@ const triggerTreeItem = async (
}
};

const nonSystemSchemas = ['new_schema'];

const setup = async () => {
const renderResult = render(<SQLEditorSchemaTree />);
// wait for tables render
Expand Down Expand Up @@ -108,7 +107,7 @@ describe('The SQLEditorSchemaTree component', () => {
await setup();

// NON SYSTEM schemas should NOT have system text
schemaTablesNonSystemMock.forEach(schema => {
nonSystemSchemas.forEach(schema => {
expect(screen.getByTestId(`schema-${schema}`)).toBeInTheDocument();
expect(
within(screen.getByTestId(`schema-${schema}`)).queryByText('system'),
Expand All @@ -121,7 +120,7 @@ describe('The SQLEditorSchemaTree component', () => {
it('shows the names', async () => {
const { user } = await setup();

const schema = schemaTableColumns[0];
const schema = schemaTableColumns.find(schema => schema.schemaName === 'gc')!;

// open schema tree
await triggerTreeItem(user, schema.schemaName, schema.tables[0].tableName);
Expand All @@ -134,7 +133,7 @@ describe('The SQLEditorSchemaTree component', () => {
it('clicking on names copies the qualified table name', async () => {
const { user } = await setup();

const schema = schemaTableColumns[0];
const schema = schemaTableColumns.find(schema => schema.schemaName === 'gc')!;
const table = schema.tables[0];

// open schema tree
Expand Down Expand Up @@ -210,7 +209,7 @@ describe('The SQLEditorSchemaTree component', () => {

it('for non-system tables it copies the CREATE TABLE statement', async () => {
const schema = schemaTableColumns.find(
schema => schema.schemaName === schemaTablesNonSystemMock[0],
schema => schema.schemaName === nonSystemSchemas[0],
)!;
const table = schema.tables.find(
table => table.tableName === 'new_table',
Expand Down Expand Up @@ -307,9 +306,11 @@ describe('The SQLEditorSchemaTree component', () => {

// open gc schema tree, should contain all tables for that schema
await triggerTreeItem(user, 'gc');
schemaTableColumns[0].tables.forEach(table => {
expect(screen.getByText(table.tableName)).toBeInTheDocument();
});
schemaTableColumns
.find(schema => schema.schemaName === 'gc')!
.tables.forEach(table => {
expect(screen.getByText(table.tableName)).toBeInTheDocument();
});
});

it('entering a string which matches a table name, but not a schema name, shows that schema and matching tables', async () => {
Expand All @@ -323,13 +324,15 @@ describe('The SQLEditorSchemaTree component', () => {

// open gc schema tree, should contain all tables for that schema
await triggerTreeItem(user, 'gc');
schemaTableColumns[0].tables.forEach(table => {
if (table.tableName.includes('alembic')) {
expect(screen.getByText(table.tableName)).toBeInTheDocument();
} else {
expect(screen.queryByText(table.tableName)).not.toBeInTheDocument();
}
});
schemaTableColumns
.find(schema => schema.schemaName === 'gc')!
.tables.forEach(table => {
if (table.tableName.includes('alembic')) {
expect(screen.getByText(table.tableName)).toBeInTheDocument();
} else {
expect(screen.queryByText(table.tableName)).not.toBeInTheDocument();
}
});
});
});

Expand Down
Loading