mirror of
https://github.com/Unleash/unleash.git
synced 2025-07-26 13:48:33 +02:00
fix: add actions validation (#6481)
https://linear.app/unleash/issue/2-2022/improve-actions-validation Improves our current actions form validation. Empty actions are now ignored on the payload and we get errors in actions where any of the required fields are empty. Also refactored our current actions into a constant map that can be shared across frontend and backend.
This commit is contained in:
parent
62a7633ed5
commit
7d827442ee
@ -57,6 +57,8 @@ interface IProjectActionsFormProps {
|
||||
setActions: React.Dispatch<React.SetStateAction<ActionsActionState[]>>;
|
||||
errors: ProjectActionsFormErrors;
|
||||
validateName: (name: string) => boolean;
|
||||
validateSourceId: (sourceId: number) => boolean;
|
||||
validateActorId: (actorId: number) => boolean;
|
||||
validated: boolean;
|
||||
}
|
||||
|
||||
@ -77,6 +79,8 @@ export const ProjectActionsForm = ({
|
||||
setActions,
|
||||
errors,
|
||||
validateName,
|
||||
validateSourceId,
|
||||
validateActorId,
|
||||
validated,
|
||||
}: IProjectActionsFormProps) => {
|
||||
const { serviceAccounts, loading: serviceAccountsLoading } =
|
||||
@ -142,6 +146,7 @@ export const ProjectActionsForm = ({
|
||||
setSourceId={setSourceId}
|
||||
filters={filters}
|
||||
setFilters={setFilters}
|
||||
validateSourceId={validateSourceId}
|
||||
/>
|
||||
|
||||
<ProjectActionsFormStepActions
|
||||
@ -151,6 +156,8 @@ export const ProjectActionsForm = ({
|
||||
setActions={setActions}
|
||||
actorId={actorId}
|
||||
setActorId={setActorId}
|
||||
validateActorId={validateActorId}
|
||||
validated={validated}
|
||||
/>
|
||||
|
||||
<ConditionallyRender
|
||||
|
@ -8,7 +8,8 @@ import { ProjectActionsFormItem } from '../ProjectActionsFormItem';
|
||||
import useProjectOverview from 'hooks/api/getters/useProjectOverview/useProjectOverview';
|
||||
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
|
||||
import { useServiceAccountAccessMatrix } from 'hooks/api/getters/useServiceAccountAccessMatrix/useServiceAccountAccessMatrix';
|
||||
import { useMemo } from 'react';
|
||||
import { useEffect, useMemo } from 'react';
|
||||
import { ACTIONS } from '@server/util/constants/actions';
|
||||
|
||||
const StyledItemBody = styled('div')(({ theme }) => ({
|
||||
display: 'flex',
|
||||
@ -28,25 +29,13 @@ const StyledFieldContainer = styled('div')({
|
||||
flex: 1,
|
||||
});
|
||||
|
||||
const options = [
|
||||
{
|
||||
label: 'Enable flag',
|
||||
key: 'TOGGLE_FEATURE_ON',
|
||||
permissions: ['UPDATE_FEATURE_ENVIRONMENT'],
|
||||
},
|
||||
{
|
||||
label: 'Disable flag',
|
||||
key: 'TOGGLE_FEATURE_OFF',
|
||||
permissions: ['UPDATE_FEATURE_ENVIRONMENT'],
|
||||
},
|
||||
];
|
||||
|
||||
interface IProjectActionsItemProps {
|
||||
action: ActionsActionState;
|
||||
index: number;
|
||||
stateChanged: (action: ActionsActionState) => void;
|
||||
actorId: number;
|
||||
onDelete: () => void;
|
||||
validated: boolean;
|
||||
}
|
||||
|
||||
export const ProjectActionsActionItem = ({
|
||||
@ -55,22 +44,23 @@ export const ProjectActionsActionItem = ({
|
||||
stateChanged,
|
||||
actorId,
|
||||
onDelete,
|
||||
validated,
|
||||
}: IProjectActionsItemProps) => {
|
||||
const { action: actionName } = action;
|
||||
const { action: actionName, executionParams, error } = action;
|
||||
const projectId = useRequiredPathParam('projectId');
|
||||
const { project } = useProjectOverview(projectId);
|
||||
const { permissions } = useServiceAccountAccessMatrix(
|
||||
actorId,
|
||||
projectId,
|
||||
action.executionParams.environment as string,
|
||||
executionParams.environment as string,
|
||||
);
|
||||
|
||||
const hasPermission = useMemo(() => {
|
||||
const requiredPermissions = options.find(
|
||||
({ key }) => key === actionName,
|
||||
)?.permissions;
|
||||
const actionDefinition = ACTIONS.get(actionName);
|
||||
|
||||
const { environment: actionEnvironment } = action.executionParams;
|
||||
const hasPermission = useMemo(() => {
|
||||
const requiredPermissions = actionDefinition?.permissions;
|
||||
|
||||
const { environment: actionEnvironment } = executionParams;
|
||||
|
||||
if (
|
||||
permissions.length === 0 ||
|
||||
@ -87,7 +77,24 @@ export const ProjectActionsActionItem = ({
|
||||
environment === actionEnvironment,
|
||||
),
|
||||
);
|
||||
}, [actionName, permissions]);
|
||||
}, [actionDefinition, permissions]);
|
||||
|
||||
useEffect(() => {
|
||||
stateChanged({
|
||||
...action,
|
||||
error: undefined,
|
||||
});
|
||||
if (
|
||||
actionDefinition?.required.some(
|
||||
(required) => !executionParams[required],
|
||||
)
|
||||
) {
|
||||
stateChanged({
|
||||
...action,
|
||||
error: 'Please fill all required fields.',
|
||||
});
|
||||
}
|
||||
}, [actionDefinition, executionParams]);
|
||||
|
||||
const environments = project.environments.map(
|
||||
({ environment }) => environment,
|
||||
@ -116,7 +123,10 @@ export const ProjectActionsActionItem = ({
|
||||
<GeneralSelect
|
||||
label='Action'
|
||||
name='action'
|
||||
options={options}
|
||||
options={[...ACTIONS].map(([key, { label }]) => ({
|
||||
key,
|
||||
label,
|
||||
}))}
|
||||
value={actionName}
|
||||
onChange={(selected) =>
|
||||
stateChanged({
|
||||
@ -135,12 +145,12 @@ export const ProjectActionsActionItem = ({
|
||||
label: environment,
|
||||
key: environment,
|
||||
}))}
|
||||
value={action.executionParams.environment as string}
|
||||
value={executionParams.environment as string}
|
||||
onChange={(selected) =>
|
||||
stateChanged({
|
||||
...action,
|
||||
executionParams: {
|
||||
...action.executionParams,
|
||||
...executionParams,
|
||||
environment: selected,
|
||||
},
|
||||
})
|
||||
@ -156,12 +166,12 @@ export const ProjectActionsActionItem = ({
|
||||
label: feature.name,
|
||||
key: feature.name,
|
||||
}))}
|
||||
value={action.executionParams.featureName as string}
|
||||
value={executionParams.featureName as string}
|
||||
onChange={(selected) =>
|
||||
stateChanged({
|
||||
...action,
|
||||
executionParams: {
|
||||
...action.executionParams,
|
||||
...executionParams,
|
||||
featureName: selected,
|
||||
},
|
||||
})
|
||||
@ -170,6 +180,10 @@ export const ProjectActionsActionItem = ({
|
||||
/>
|
||||
</StyledFieldContainer>
|
||||
</StyledItemRow>
|
||||
<ConditionallyRender
|
||||
condition={validated && Boolean(error)}
|
||||
show={<Alert severity='error'>{error}</Alert>}
|
||||
/>
|
||||
<ConditionallyRender
|
||||
condition={!hasPermission}
|
||||
show={
|
||||
|
@ -29,6 +29,8 @@ interface IProjectActionsFormStepActionsProps {
|
||||
setActions: React.Dispatch<React.SetStateAction<ActionsActionState[]>>;
|
||||
actorId: number;
|
||||
setActorId: React.Dispatch<React.SetStateAction<number>>;
|
||||
validateActorId: (actorId: number) => boolean;
|
||||
validated: boolean;
|
||||
}
|
||||
|
||||
export const ProjectActionsFormStepActions = ({
|
||||
@ -38,6 +40,8 @@ export const ProjectActionsFormStepActions = ({
|
||||
setActions,
|
||||
actorId,
|
||||
setActorId,
|
||||
validateActorId,
|
||||
validated,
|
||||
}: IProjectActionsFormStepActionsProps) => {
|
||||
const projectId = useRequiredPathParam('projectId');
|
||||
|
||||
@ -91,6 +95,7 @@ export const ProjectActionsFormStepActions = ({
|
||||
options={serviceAccountOptions}
|
||||
value={`${actorId}`}
|
||||
onChange={(v) => {
|
||||
validateActorId(Number(v));
|
||||
setActorId(parseInt(v));
|
||||
}}
|
||||
/>
|
||||
@ -107,6 +112,7 @@ export const ProjectActionsFormStepActions = ({
|
||||
actions.filter((a) => a.id !== action.id),
|
||||
)
|
||||
}
|
||||
validated={validated}
|
||||
/>
|
||||
))}
|
||||
<StyledButtonContainer>
|
||||
|
@ -37,6 +37,7 @@ interface IProjectActionsFormStepSourceProps {
|
||||
setSourceId: React.Dispatch<React.SetStateAction<number>>;
|
||||
filters: ActionsFilterState[];
|
||||
setFilters: React.Dispatch<React.SetStateAction<ActionsFilterState[]>>;
|
||||
validateSourceId: (sourceId: number) => boolean;
|
||||
}
|
||||
|
||||
export const ProjectActionsFormStepSource = ({
|
||||
@ -44,6 +45,7 @@ export const ProjectActionsFormStepSource = ({
|
||||
setSourceId,
|
||||
filters,
|
||||
setFilters,
|
||||
validateSourceId,
|
||||
}: IProjectActionsFormStepSourceProps) => {
|
||||
const { signalEndpoints, loading: signalEndpointsLoading } =
|
||||
useSignalEndpoints();
|
||||
@ -102,6 +104,7 @@ export const ProjectActionsFormStepSource = ({
|
||||
options={signalEndpointOptions}
|
||||
value={`${sourceId}`}
|
||||
onChange={(v) => {
|
||||
validateSourceId(Number(v));
|
||||
setSourceId(parseInt(v));
|
||||
}}
|
||||
/>
|
||||
|
@ -23,6 +23,7 @@ export type ActionsActionState = Omit<
|
||||
'id' | 'createdAt' | 'createdByUserId'
|
||||
> & {
|
||||
id: string;
|
||||
error?: string;
|
||||
};
|
||||
|
||||
const DEFAULT_PROJECT_ACTIONS_FORM_ERRORS = {
|
||||
@ -99,12 +100,13 @@ export const useProjectActionsForm = (action?: IActionSet) => {
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
clearError(ErrorField.FILTERS);
|
||||
if (filters.some(({ error }) => error)) {
|
||||
setError(ErrorField.FILTERS, 'One or more filters have errors.');
|
||||
}
|
||||
validateFilters(filters);
|
||||
}, [filters]);
|
||||
|
||||
useEffect(() => {
|
||||
validateActions(actions);
|
||||
}, [actions]);
|
||||
|
||||
const isEmpty = (value: string) => !value.length;
|
||||
|
||||
const isNameNotUnique = (value: string) =>
|
||||
@ -137,6 +139,16 @@ export const useProjectActionsForm = (action?: IActionSet) => {
|
||||
return true;
|
||||
};
|
||||
|
||||
const validateFilters = (filters: ActionsFilterState[]) => {
|
||||
if (filters.some(({ error }) => error)) {
|
||||
setError(ErrorField.FILTERS, 'One or more filters have errors.');
|
||||
return false;
|
||||
}
|
||||
|
||||
clearError(ErrorField.FILTERS);
|
||||
return true;
|
||||
};
|
||||
|
||||
const validateActorId = (sourceId: number) => {
|
||||
if (isIdEmpty(sourceId)) {
|
||||
setError(ErrorField.ACTOR, 'Service account is required.');
|
||||
@ -148,11 +160,17 @@ export const useProjectActionsForm = (action?: IActionSet) => {
|
||||
};
|
||||
|
||||
const validateActions = (actions: ActionsActionState[]) => {
|
||||
if (actions.length === 0) {
|
||||
if (actions.filter(({ action }) => Boolean(action)).length === 0) {
|
||||
setError(ErrorField.ACTIONS, 'At least one action is required.');
|
||||
return false;
|
||||
}
|
||||
|
||||
clearError(ErrorField.ACTIONS);
|
||||
if (actions.some(({ error }) => error)) {
|
||||
setError(ErrorField.ACTIONS, 'One or more actions have errors.');
|
||||
return false;
|
||||
}
|
||||
|
||||
clearError(ErrorField.ACTIONS);
|
||||
return true;
|
||||
};
|
||||
@ -160,12 +178,19 @@ export const useProjectActionsForm = (action?: IActionSet) => {
|
||||
const validate = () => {
|
||||
const validName = validateName(name);
|
||||
const validSourceId = validateSourceId(sourceId);
|
||||
const validFilters = validateFilters(filters);
|
||||
const validActorId = validateActorId(actorId);
|
||||
const validActions = validateActions(actions);
|
||||
|
||||
setValidated(true);
|
||||
|
||||
return validName && validSourceId && validActorId && validActions;
|
||||
return (
|
||||
validName &&
|
||||
validSourceId &&
|
||||
validFilters &&
|
||||
validActorId &&
|
||||
validActions
|
||||
);
|
||||
};
|
||||
|
||||
return {
|
||||
@ -187,6 +212,8 @@ export const useProjectActionsForm = (action?: IActionSet) => {
|
||||
setErrors,
|
||||
validated,
|
||||
validateName,
|
||||
validateSourceId,
|
||||
validateActorId,
|
||||
validate,
|
||||
reloadForm,
|
||||
};
|
||||
|
@ -81,6 +81,8 @@ export const ProjectActionsModal = ({
|
||||
setActions,
|
||||
errors,
|
||||
validateName,
|
||||
validateSourceId,
|
||||
validateActorId,
|
||||
validate,
|
||||
validated,
|
||||
reloadForm,
|
||||
@ -127,11 +129,13 @@ export const ProjectActionsModal = ({
|
||||
),
|
||||
},
|
||||
actorId,
|
||||
actions: actions.map(({ action, sortOrder, executionParams }) => ({
|
||||
action,
|
||||
sortOrder,
|
||||
executionParams,
|
||||
})),
|
||||
actions: actions
|
||||
.filter(({ action }) => Boolean(action))
|
||||
.map(({ action, sortOrder, executionParams }) => ({
|
||||
action,
|
||||
sortOrder,
|
||||
executionParams,
|
||||
})),
|
||||
};
|
||||
|
||||
const formatApiCode = () => `curl --location --request ${
|
||||
@ -206,6 +210,8 @@ export const ProjectActionsModal = ({
|
||||
setActions={setActions}
|
||||
errors={errors}
|
||||
validateName={validateName}
|
||||
validateSourceId={validateSourceId}
|
||||
validateActorId={validateActorId}
|
||||
validated={validated}
|
||||
/>
|
||||
<StyledButtonContainer>
|
||||
|
@ -1,11 +1,11 @@
|
||||
import { useMemo } from 'react';
|
||||
import { formatApiPath } from 'utils/formatPath';
|
||||
import handleErrorResponses from '../httpErrorResponseHandler';
|
||||
import useSWR from 'swr';
|
||||
import { IRole } from 'interfaces/role';
|
||||
import { IServiceAccount } from 'interfaces/service-account';
|
||||
import { IMatrixPermission } from 'interfaces/permissions';
|
||||
import { IPermission } from 'interfaces/user';
|
||||
import { useConditionalSWR } from '../useConditionalSWR/useConditionalSWR';
|
||||
|
||||
interface IServiceAccountAccessMatrix {
|
||||
root: IMatrixPermission[];
|
||||
@ -30,7 +30,7 @@ interface IServiceAccountAccessMatrixOutput
|
||||
}
|
||||
|
||||
export const useServiceAccountAccessMatrix = (
|
||||
id: number,
|
||||
id?: number,
|
||||
project?: string,
|
||||
environment?: string,
|
||||
): IServiceAccountAccessMatrixOutput => {
|
||||
@ -39,10 +39,9 @@ export const useServiceAccountAccessMatrix = (
|
||||
}`;
|
||||
const url = `api/admin/service-account/${id}/permissions${queryParams}`;
|
||||
|
||||
const { data, error, mutate } = useSWR<IServiceAccountAccessMatrixResponse>(
|
||||
formatApiPath(url),
|
||||
fetcher,
|
||||
);
|
||||
const { data, error, mutate } = useConditionalSWR<
|
||||
IServiceAccountAccessMatrixResponse | undefined
|
||||
>(Boolean(id), undefined, formatApiPath(url), fetcher);
|
||||
|
||||
return useMemo(
|
||||
() => ({
|
||||
|
24
src/lib/util/constants/actions.ts
Normal file
24
src/lib/util/constants/actions.ts
Normal file
@ -0,0 +1,24 @@
|
||||
type ActionDefinition = {
|
||||
label: string;
|
||||
permissions: string[];
|
||||
required: string[];
|
||||
};
|
||||
|
||||
export const ACTIONS = new Map<string, ActionDefinition>([
|
||||
[
|
||||
'TOGGLE_FEATURE_ON',
|
||||
{
|
||||
label: 'Enable flag',
|
||||
permissions: ['UPDATE_FEATURE_ENVIRONMENT'],
|
||||
required: ['project', 'environment', 'featureName'],
|
||||
},
|
||||
],
|
||||
[
|
||||
'TOGGLE_FEATURE_OFF',
|
||||
{
|
||||
label: 'Disable flag',
|
||||
permissions: ['UPDATE_FEATURE_ENVIRONMENT'],
|
||||
required: ['project', 'environment', 'featureName'],
|
||||
},
|
||||
],
|
||||
]);
|
Loading…
Reference in New Issue
Block a user