Fix permission form's submit cache behavior and bug when setting relationships

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/9894
GitOrigin-RevId: 68587edeb5e3a923f52154d597c8002428645326
This commit is contained in:
Julian 2023-07-24 14:59:56 -03:00 committed by hasura-bot
parent a833379b86
commit 8bbb1bec34
7 changed files with 139 additions and 77 deletions

View File

@ -1,4 +1,4 @@
import React, { useEffect, useRef } from 'react';
import React, { useRef } from 'react';
import { useConsoleForm } from '../../../new-components/Form';
import { Button } from '../../../new-components/Button';
import { IndicatorCard } from '../../../new-components/IndicatorCard';
@ -21,12 +21,19 @@ import {
RowPermissionsSectionWrapper,
} from './components';
import { useFormData, useUpdatePermissions } from './hooks';
import {
createDefaultValues,
useFormData,
useUpdatePermissions,
} from './hooks';
import ColumnRootFieldPermissions from './components/RootFieldPermissions/RootFieldPermissions';
import { useListAllTableColumns } from '../../Data';
import { useMetadataSource } from '../../MetadataAPI';
import useScrollIntoView from './hooks/useScrollIntoView';
import { getAllowedFilterKeys } from './hooks/dataFetchingHooks/useFormData/createFormData/index';
import {
createFormData,
getAllowedFilterKeys,
} from './hooks/dataFetchingHooks/useFormData/createFormData/index';
import Skeleton from 'react-loading-skeleton';
export interface ComponentProps {
@ -36,7 +43,8 @@ export interface ComponentProps {
roleName: string;
accessType: AccessType;
handleClose: () => void;
data: ReturnType<typeof useFormData>['data'];
defaultValues: ReturnType<typeof createDefaultValues>;
formData: ReturnType<typeof createFormData>;
}
const Component = (props: ComponentProps) => {
@ -47,9 +55,11 @@ const Component = (props: ComponentProps) => {
roleName,
accessType,
handleClose,
data,
defaultValues,
formData,
} = props;
const permissionSectionRef = useRef(null);
useScrollIntoView(permissionSectionRef, [roleName], { behavior: 'smooth' });
const { data: metadataTables } = useMetadata(
@ -77,7 +87,6 @@ const Component = (props: ComponentProps) => {
await updatePermissions.submit(formData);
handleClose();
} catch (e) {
reset();
reset(newValues);
}
};
@ -87,8 +96,6 @@ const Component = (props: ComponentProps) => {
handleClose();
};
const { formData, defaultValues } = data || {};
const {
methods: { getValues, reset },
Form,
@ -99,13 +106,6 @@ const Component = (props: ComponentProps) => {
},
});
// Reset form when default values change
// E.g. when switching tables
useEffect(() => {
const newValues = getValues();
reset({ ...newValues, ...defaultValues, clonePermissions: [] });
}, [roleName, defaultValues]);
const key = `${JSON.stringify(table)}-${queryType}-${roleName}`;
const filterType = getValues('filterType');
@ -144,7 +144,7 @@ const Component = (props: ComponentProps) => {
subQueryType={queryType === 'update' ? 'pre_update' : undefined}
permissionsKey={filterKeys[0]}
dataSourceName={dataSourceName}
supportedOperators={data?.defaultValues?.supportedOperators ?? []}
supportedOperators={defaultValues?.supportedOperators ?? []}
defaultValues={defaultValues}
/>
{queryType === 'update' && (
@ -163,9 +163,7 @@ const Component = (props: ComponentProps) => {
}
permissionsKey={filterKeys[1]}
dataSourceName={dataSourceName}
supportedOperators={
data?.defaultValues?.supportedOperators ?? []
}
supportedOperators={defaultValues?.supportedOperators ?? []}
defaultValues={defaultValues}
/>
</div>
@ -267,7 +265,30 @@ export const PermissionsForm = (props: PermissionsFormProps) => {
table,
queryType,
roleName,
});
if (!data || !metadataSource) {
return <Skeleton width={'100%'} height={300} />;
}
const defaultValues = createDefaultValues({
queryType,
roleName,
dataSourceName,
metadata: data?.metadata,
table,
tableColumns,
defaultQueryRoot: data.defaultQueryRoot,
metadataSource,
supportedOperators: data.supportedOperators,
});
const formData = createFormData({
dataSourceName,
table,
metadata: data.metadata,
tableColumns,
trackedTables: metadataSource.tables,
metadataSource,
});
@ -279,15 +300,24 @@ export const PermissionsForm = (props: PermissionsFormProps) => {
if (
isLoading ||
!data ||
isLoadingTables ||
!tableColumns ||
tableColumns?.length === 0 ||
!metadataSource ||
!data.defaultValues
tableColumns?.length === 0
) {
return <Skeleton width={'100%'} height={300} />;
}
return <Component data={data} {...props} />;
return (
<Component
// Reset component when defaultValues change
// Otherwise the form keeps the old values
// Tried using react-hook-form's `reset` but it's overwritten by old default values (before submitting)
key={`${dataSourceName}-${JSON.stringify(
table
)}-${queryType}-${roleName}-${JSON.stringify(defaultValues)}`}
defaultValues={defaultValues}
formData={formData}
{...props}
/>
);
};

View File

@ -907,3 +907,64 @@ export const OperatorDropdownHandling: StoryObj<typeof RowPermissionsInput> = {
);
},
};
export const ReplaceArrayWithColumn: StoryObj<typeof RowPermissionsInput> = {
render: args => {
const [permissions, setPermissions] = useState<Permissions>({ _and: [{}] });
return (
<RowPermissionsInput
onPermissionsChange={p => {
setPermissions(p);
}}
table={{ dataset: 'bigquery_sample', name: 'sample_table' }}
tables={tables}
comparators={comparators}
logicalModel={undefined}
logicalModels={[]}
permissions={permissions}
/>
);
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// Should be able to select the _and dropdown and change it to be a column
await userEvent.selectOptions(
canvas.getByTestId('_and-operator'),
'Series_reference'
);
expect(canvas.getByTestId('Series_reference-operator')).toBeInTheDocument();
},
};
// The difference between this and the previous story is that this one does not have a value in the array
// There was a bug where this case did not work, so adding a test for it
export const ReplaceEmptyArrayWithColumn: StoryObj<typeof RowPermissionsInput> =
{
render: args => {
const [permissions, setPermissions] = useState<Permissions>({ _and: [] });
return (
<RowPermissionsInput
onPermissionsChange={p => {
setPermissions(p);
}}
table={{ dataset: 'bigquery_sample', name: 'sample_table' }}
tables={tables}
comparators={comparators}
logicalModel={undefined}
logicalModels={[]}
permissions={permissions}
/>
);
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// Should be able to select the _and dropdown and change it to be a column
await userEvent.selectOptions(
canvas.getByTestId('_and-operator'),
'Series_reference'
);
expect(
canvas.getByTestId('Series_reference-operator')
).toBeInTheDocument();
},
};

View File

@ -26,11 +26,17 @@ const getKeyPath = ({
newKey !== '_exists' && // ignore _exists which is a special comparator
path.length >= 1;
if (!isEmpty(value) || type === 'relationship' || isNestedComparator) {
const previousKey = keyPath[keyPath.length - 1];
if (
// Replacing an `_and` comparator that's empty (as opposed to the default `{}`) with a column key
isEmptyArray(value, previousKey) ||
!isEmpty(value) ||
type === 'relationship' ||
isNestedComparator
) {
path = replacePath(keyPath, permissionsState);
}
const previousKey = keyPath[keyPath.length - 1];
if ((previousKey === '_not' && newKey === '_and') || newKey === '_or') {
path = replacePath(keyPath, permissionsState);
}
@ -140,3 +146,11 @@ export function isColumnComparator(comparator: string) {
comparator === '_cle'
);
}
function isEmptyArray(value: string, previousKey: string) {
return (
Array.isArray(value) &&
isEmpty(value) &&
(previousKey === '_and' || previousKey === '_or' || previousKey === '_not')
);
}

View File

@ -37,7 +37,7 @@ export interface CreateDefaultValuesArgs {
roleName: string;
table: unknown;
dataSourceName: string;
metadata: Metadata;
metadata: Metadata | undefined;
tableColumns: TableColumn[];
defaultQueryRoot: string | never[];
metadataSource: MetadataDataSource | undefined;

View File

@ -70,8 +70,8 @@ export interface CreateFormDataArgs {
table: unknown;
metadata: Metadata;
tableColumns: TableColumn[];
metadataSource: MetadataDataSource | undefined;
trackedTables: TableEntry[] | undefined;
metadataSource: MetadataDataSource;
trackedTables: TableEntry[];
}
export const createFormData = (props: CreateFormDataArgs) => {

View File

@ -4,21 +4,17 @@ import {
DataSource,
exportMetadata,
Operator,
TableColumn,
} from '../../../../../DataSource';
import { useHttpClient } from '../../../../../Network';
import { createDefaultValues } from './createDefaultValues';
import { createFormData } from './createFormData';
import { MetadataDataSource } from '../../../../../../metadata/types';
export type Args = {
dataSourceName: string;
table: unknown;
roleName: string;
queryType: 'select' | 'insert' | 'update' | 'delete';
tableColumns: TableColumn[];
metadataSource: MetadataDataSource | undefined;
};
export type ReturnValue = {
@ -31,27 +27,11 @@ export type ReturnValue = {
* creates data for displaying in the form e.g. column names, roles etc.
* creates default values for form i.e. existing permissions from metadata
*/
export const useFormData = ({
dataSourceName,
table,
roleName,
queryType,
tableColumns = [],
metadataSource,
}: Args) => {
export const useFormData = ({ dataSourceName, table }: Args) => {
const httpClient = useHttpClient();
return useQuery<ReturnValue, Error>({
queryKey: [
dataSourceName,
'permissionFormData',
JSON.stringify(table),
roleName,
tableColumns,
queryType,
],
return useQuery({
queryKey: [dataSourceName, 'permissionFormData', table],
queryFn: async () => {
if (tableColumns.length === 0)
return { formData: undefined, defaultValues: undefined };
const metadata = await exportMetadata({ httpClient });
const defaultQueryRoot = await DataSource(httpClient).getDefaultQueryRoot(
@ -67,30 +47,7 @@ export const useFormData = ({
dataSourceName,
})) as Operator[];
const defaultValues = {
...createDefaultValues({
queryType,
roleName,
dataSourceName,
metadata,
table,
tableColumns,
defaultQueryRoot,
metadataSource,
supportedOperators: supportedOperators ?? [],
}),
};
const formData = createFormData({
dataSourceName,
table,
metadata,
tableColumns,
trackedTables: metadataSource?.tables,
metadataSource,
});
return { formData, defaultValues };
return { defaultQueryRoot, supportedOperators, metadata };
},
refetchOnWindowFocus: false,
});

View File

@ -329,7 +329,7 @@ export const useRolePermissions = ({
{ supportedQueries: QueryType[]; rolePermissions: RolePermission[] },
Error
>({
queryKey: [dataSourceName, 'permissionsTable', JSON.stringify(table)],
queryKey: [dataSourceName, 'permissionsTable', table],
queryFn: async () => {
const metadata = await exportMetadata({ httpClient });
// get table columns for metadata table from db introspection