Skip to content

Commit

Permalink
feat: Oauth2 in DatabaseSelector (#30082)
Browse files Browse the repository at this point in the history
  • Loading branch information
betodealmeida committed Sep 4, 2024
1 parent 0415ed3 commit 09dfe2f
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 11 deletions.
29 changes: 25 additions & 4 deletions superset-frontend/src/components/DatabaseSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import {
useRef,
useCallback,
} from 'react';
import { styled, SupersetClient, t } from '@superset-ui/core';
import { styled, SupersetClient, SupersetError, t } from '@superset-ui/core';
import type { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import rison from 'rison';
import { AsyncSelect, Select } from 'src/components';
import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
import Label from 'src/components/Label';
import { FormLabel } from 'src/components/Form';
import RefreshLabel from 'src/components/RefreshLabel';
Expand Down Expand Up @@ -154,6 +155,7 @@ export default function DatabaseSelector({
}: DatabaseSelectorProps) {
const showCatalogSelector = !!db?.allow_multi_catalog;
const [currentDb, setCurrentDb] = useState<DatabaseValue | undefined>();
const [errorPayload, setErrorPayload] = useState<SupersetError | null>();
const [currentCatalog, setCurrentCatalog] = useState<
CatalogOption | null | undefined
>(catalog ? { label: catalog, value: catalog, title: catalog } : undefined);
Expand Down Expand Up @@ -267,6 +269,7 @@ export default function DatabaseSelector({
dbId: currentDb?.value,
catalog: currentCatalog?.value,
onSuccess: (schemas, isFetched) => {
setErrorPayload(null);
if (schemas.length === 1) {
changeSchema(schemas[0]);
} else if (
Expand All @@ -279,7 +282,13 @@ export default function DatabaseSelector({
addSuccessToast('List refreshed');
}
},
onError: () => handleError(t('There was an error loading the schemas')),
onError: error => {
if (error?.errors) {
setErrorPayload(error?.errors?.[0]);
} else {
handleError(t('There was an error loading the schemas'));
}
},
});

const schemaOptions = schemaData || EMPTY_SCHEMA_OPTIONS;
Expand All @@ -299,6 +308,7 @@ export default function DatabaseSelector({
} = useCatalogs({
dbId: showCatalogSelector ? currentDb?.value : undefined,
onSuccess: (catalogs, isFetched) => {
setErrorPayload(null);
if (!showCatalogSelector) {
changeCatalog(null);
} else if (catalogs.length === 1) {
Expand All @@ -315,9 +325,13 @@ export default function DatabaseSelector({
addSuccessToast('List refreshed');
}
},
onError: () => {
onError: error => {
if (showCatalogSelector) {
handleError(t('There was an error loading the catalogs'));
if (error?.errors) {
setErrorPayload(error?.errors?.[0]);
} else {
handleError(t('There was an error loading the catalogs'));
}
}
},
});
Expand Down Expand Up @@ -423,9 +437,16 @@ export default function DatabaseSelector({
);
}

function renderError() {
return errorPayload ? (
<ErrorMessageWithStackTrace error={errorPayload} source="crud" />
) : null;
}

return (
<DatabaseSelectorWrapper data-test="DatabaseSelector">
{renderDatabaseSelect()}
{renderError()}
{showCatalogSelector && renderCatalogSelect()}
{renderSchemaSelect()}
</DatabaseSelectorWrapper>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ function OAuth2RedirectMessage({
>
provide authorization
</a>{' '}
in order to run this query.
in order to run this operation.
</>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,19 @@ import { FieldPropTypes } from '../../types';

const FIELD_TEXT_MAP = {
account: {
label: 'Account',
helpText: t(
'Copy the identifier of the account you are trying to connect to.',
),
placeholder: t('e.g. xy12345.us-east-2.aws'),
},
warehouse: {
label: 'Warehouse',
placeholder: t('e.g. compute_wh'),
className: 'form-group-w-50',
},
role: {
label: 'Role',
placeholder: t('e.g. AccountAdmin'),
className: 'form-group-w-50',
},
Expand All @@ -54,7 +57,7 @@ export const validatedInputField = ({
errorMessage={validationErrors?.[field]}
placeholder={FIELD_TEXT_MAP[field].placeholder}
helpText={FIELD_TEXT_MAP[field].helpText}
label={field}
label={FIELD_TEXT_MAP[field].label || field}
onChange={changeMethods.onParametersChange}
className={FIELD_TEXT_MAP[field].className || field}
/>
Expand Down
11 changes: 9 additions & 2 deletions superset-frontend/src/hooks/apiResources/catalogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import { useCallback, useEffect } from 'react';
import { ClientErrorObject } from '@superset-ui/core';
import useEffectEvent from 'src/hooks/useEffectEvent';
import { api, JsonResponse } from './queryApi';

Expand All @@ -30,7 +31,7 @@ export type FetchCatalogsQueryParams = {
dbId?: string | number;
forceRefresh: boolean;
onSuccess?: (data: CatalogOption[], isRefetched: boolean) => void;
onError?: () => void;
onError?: (error: ClientErrorObject) => void;
};

type Params = Omit<FetchCatalogsQueryParams, 'forceRefresh'>;
Expand Down Expand Up @@ -77,6 +78,12 @@ export function useCatalogs(options: Params) {
},
);

useEffect(() => {
if (result.isError) {
onError?.(result.error as ClientErrorObject);
}
}, [result.isError, result.error, onError]);

const fetchData = useEffectEvent(
(dbId: FetchCatalogsQueryParams['dbId'], forceRefresh = false) => {
if (dbId && (!result.currentData || forceRefresh)) {
Expand All @@ -85,7 +92,7 @@ export function useCatalogs(options: Params) {
onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
}
if (isError) {
onError?.();
onError?.(result.error as ClientErrorObject);
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/hooks/apiResources/queryApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ test('supersetClientQuery should return error when unsuccessful', async () => {
getBaseQueryApiMock(store),
{},
);
expect(result.error).toEqual({ error: expectedError });
expect(result.error).toEqual({ error: expectedError, errors: [] });
});

test('supersetClientQuery should return parsed response by parseMethod', async () => {
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/hooks/apiResources/queryApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export const supersetClientQuery: BaseQueryFn<
getClientErrorObject(response).then(errorObj => ({
error: {
error: errorObj?.message || errorObj?.error || response.statusText,
errors: errorObj?.errors || [], // used by <ErrorMessageWithStackTrace />
status: response.status,
},
})),
Expand Down
11 changes: 9 additions & 2 deletions superset-frontend/src/hooks/apiResources/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import { useCallback, useEffect } from 'react';
import { ClientErrorObject } from '@superset-ui/core';
import useEffectEvent from 'src/hooks/useEffectEvent';
import { api, JsonResponse } from './queryApi';

Expand All @@ -31,7 +32,7 @@ export type FetchSchemasQueryParams = {
catalog?: string;
forceRefresh: boolean;
onSuccess?: (data: SchemaOption[], isRefetched: boolean) => void;
onError?: () => void;
onError?: (error: ClientErrorObject) => void;
};

type Params = Omit<FetchSchemasQueryParams, 'forceRefresh'>;
Expand Down Expand Up @@ -81,6 +82,12 @@ export function useSchemas(options: Params) {
},
);

useEffect(() => {
if (result.isError) {
onError?.(result.error as ClientErrorObject);
}
}, [result.isError, result.error, onError]);

const fetchData = useEffectEvent(
(
dbId: FetchSchemasQueryParams['dbId'],
Expand All @@ -94,7 +101,7 @@ export function useSchemas(options: Params) {
onSuccess?.(data || EMPTY_SCHEMAS, forceRefresh);
}
if (isError) {
onError?.();
onError?.(result.error as ClientErrorObject);
}
},
);
Expand Down

0 comments on commit 09dfe2f

Please sign in to comment.