Skip to content

Commit

Permalink
fix: change dirty strategy in check form (#1037)
Browse files Browse the repository at this point in the history
* fix: change dirty strategy in check form

* fix: submit check without unsaved modal prompt
  • Loading branch information
ckbedwell authored Jan 8, 2025
1 parent e287f10 commit 25ee12f
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 97 deletions.
11 changes: 7 additions & 4 deletions src/components/CheckForm/CheckForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { toFormValues } from 'components/CheckEditor/checkFormTransformations';
import { CheckJobName } from 'components/CheckEditor/FormComponents/CheckJobName';
import { ChooseCheckType } from 'components/CheckEditor/FormComponents/ChooseCheckType';
import { ProbeOptions } from 'components/CheckEditor/ProbeOptions';
import { checkHasChanges } from 'components/CheckForm/checkForm.utils';
import { DNSCheckLayout } from 'components/CheckForm/FormLayouts/CheckDNSLayout';
import { GRPCCheckLayout } from 'components/CheckForm/FormLayouts/CheckGrpcLayout';
import { HttpCheckLayout } from 'components/CheckForm/FormLayouts/CheckHttpLayout';
Expand Down Expand Up @@ -93,9 +94,10 @@ export const CheckForm = ({ check, disabled }: CheckFormProps) => {
(checkType === CheckType.Browser && isOverBrowserLimit) ||
([CheckType.MULTI_HTTP, CheckType.Scripted].includes(checkType) && isOverScriptedLimit);
const isDisabled = disabled || !canWriteChecks || getLimitDisabled({ isExistingCheck, isLoading, overLimit });
const defaultValues = toFormValues(initialCheck, checkType);

const formMethods = useForm<CheckFormValues>({
defaultValues: toFormValues(initialCheck, checkType),
defaultValues,
shouldFocusError: false, // we manage focus manually
resolver: zodResolver(schema),
});
Expand Down Expand Up @@ -158,9 +160,10 @@ export const CheckForm = ({ check, disabled }: CheckFormProps) => {
</Stack>
);

const { isDirty, isSubmitSuccessful } = formMethods.formState;
// since we navigate on submit, we need this to not trigger the confirmation modal
const hasUnsavedChanges = isDirty && !isSubmitSuccessful;
const hasUnsavedChanges = error
? true
: checkHasChanges(defaultValues, formMethods.getValues()) && !formMethods.formState.isSubmitSuccessful;

const navModel = useMemo(() => {
return isExistingCheck
? createNavModel(
Expand Down
4 changes: 4 additions & 0 deletions src/components/CheckForm/checkForm.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { PROBES_FILTER_ID } from 'components/CheckEditor/CheckProbes/ProbesFilte
import { SCRIPT_TEXTAREA_ID } from 'components/CheckEditor/FormComponents/ScriptedCheckScript';
import { CHECK_FORM_ERROR_EVENT } from 'components/constants';

export function checkHasChanges(existing: CheckFormValues, incoming: CheckFormValues) {
return JSON.stringify(existing) !== JSON.stringify(incoming);
}

export function flattenKeys(errs: FieldErrors<CheckFormValues>) {
const build: string[] = [];

Expand Down

This file was deleted.

This file was deleted.

5 changes: 5 additions & 0 deletions src/page/EditCheck/__tests__/EditCheck.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,9 @@ describe(`<EditCheck />`, () => {
const submitButton = await screen.findByTestId(DataTestIds.CHECK_FORM_SUBMIT_BUTTON);
expect(submitButton).toBeDisabled();
});

it(`disables the save button when no edits have been made`, async () => {
await renderEditForm(BASIC_HTTP_CHECK.id);
expect(await screen.findByTestId(DataTestIds.CHECK_FORM_SUBMIT_BUTTON)).not.toBeEnabled();
});
});
36 changes: 36 additions & 0 deletions src/page/NewCheck/__tests__/NewCheck.journey.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,42 @@ describe(`<NewCheck /> journey`, () => {
expect(testButton).toBeDisabled();
});

it(`disables the submit button by default`, async () => {
await renderNewForm(CheckType.HTTP);
expect(screen.getByTestId(DataTestIds.CHECK_FORM_SUBMIT_BUTTON)).not.toBeEnabled();
});

it(`enables the submit button when a field is edited`, async () => {
const { user } = await renderNewForm(CheckType.HTTP);
const jobNameInput = await screen.findByLabelText('Job name', { exact: false });
await user.type(jobNameInput, 'My Job Name');
const submitButton = await screen.findByTestId(DataTestIds.CHECK_FORM_SUBMIT_BUTTON);
expect(submitButton).toBeEnabled();
});

it(`has the save button enabled after a failed submission`, async () => {
const { user } = await renderNewForm(CheckType.HTTP);

server.use(
apiRoute(`addCheck`, {
result: () => {
return {
status: 409,
json: {
err: 'target/job combination already exists',
msg: 'Failed to add check to database',
},
};
},
})
);

await fillMandatoryFields({ user, checkType: CheckType.HTTP });
await submitForm(user);
const submitButton = await screen.findByTestId(DataTestIds.CHECK_FORM_SUBMIT_BUTTON);
expect(submitButton).toBeEnabled();
});

// jsdom doesn't give us back the submitter of the form, so we can't test this
// https://github.com/jsdom/jsdom/issues/3117
it.skip(`should show an error message when it fails to test a check`, async () => {});
Expand Down

0 comments on commit 25ee12f

Please sign in to comment.