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

(refactor) Refactor conditions form validation logic #1881

Merged
merged 3 commits into from
Jul 1, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React from 'react';
import dayjs from 'dayjs';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { showSnackbar } from '@openmrs/esm-framework';
import { searchedCondition } from '__mocks__';
import { type FetchResponse, openmrsFetch, showSnackbar } from '@openmrs/esm-framework';
import { mockFhirConditionsResponse, searchedCondition } from '__mocks__';
import { getByTextWithMarkup, mockPatient } from 'tools';
import { createCondition, useConditionsSearch } from './conditions.resource';
import ConditionsForm from './conditions-form.workspace';
Expand All @@ -29,15 +29,18 @@ const utc = require('dayjs/plugin/utc');
dayjs.extend(utc);

const testProps = {
condition: null,
closeWorkspace: jest.fn(),
closeWorkspaceWithSavedChanges: jest.fn(),
patientUuid: mockPatient.id,
formContext: 'creating' as const,
promptBeforeClosing: jest.fn(),
formContext: 'creating' as 'creating' | 'editing',
};

const mockCreateCondition = createCondition as jest.Mock;
const mockUseConditionsSearch = useConditionsSearch as jest.Mock;
const mockshowSnackbar = showSnackbar as jest.Mock;
const mockShowSnackbar = showSnackbar as jest.Mock;
const mockOpenmrsFetch = jest.mocked(openmrsFetch);

jest.mock('lodash-es/debounce', () => jest.fn((fn) => fn));

Expand All @@ -50,20 +53,26 @@ jest.mock('@openmrs/esm-framework', () => {
};
});

jest.mock('./conditions.resource', () => ({
createCondition: jest.fn(),
editCondition: jest.fn(),
useConditions: jest.fn().mockImplementation(() => ({
mutate: jest.fn(),
})),
useConditionsSearch: jest.fn().mockImplementation(() => ({
conditions: [],
error: null,
isSearching: false,
})),
}));

describe('Conditions Form', () => {
jest.mock('./conditions.resource', () => {
const originalModule = jest.requireActual('./conditions.resource');

return {
...originalModule,
createCondition: jest.fn(),
editCondition: jest.fn(),
useConditionsSearch: jest.fn().mockImplementation(() => ({
conditions: [],
error: null,
isSearching: false,
})),
};
});

describe('Conditions form', () => {
beforeEach(() => {
mockShowSnackbar.mockClear();
});

it('renders the conditions form with all the relevant fields and values', () => {
renderConditionsForm();

Expand Down Expand Up @@ -127,7 +136,8 @@ describe('Conditions Form', () => {
it('renders a success toast notification upon successfully recording a condition', async () => {
const user = userEvent.setup();

mockCreateCondition.mockReturnValue(Promise.resolve({ status: 201, body: 'Condition created' }));
mockOpenmrsFetch.mockResolvedValue({ data: [] } as FetchResponse);
mockCreateCondition.mockResolvedValue({ status: 201, body: 'Condition created' });
mockUseConditionsSearch.mockReturnValue({
searchResults: searchedCondition,
error: null,
Expand All @@ -142,11 +152,20 @@ describe('Conditions Form', () => {
const conditionSearchInput = screen.getByRole('searchbox', { name: /enter condition/i });
const onsetDateInput = screen.getByRole('textbox', { name: /onset date/i });
expect(cancelButton).not.toBeDisabled();

await user.type(conditionSearchInput, 'Headache');
await user.click(screen.getByRole('menuitem', { name: /headache/i }));
await user.click(activeStatusInput);
await user.type(onsetDateInput, '2020-05-05');
await user.click(submitButton);

// TODO: Figure out why this isn't working
// expect(mockShowSnackbar).toHaveBeenCalled();
// expect(mockShowSnackbar).toHaveBeenCalledWith({
// kind: 'success',
// subtitle: 'It is now visible on the Conditions page',
// title: 'Condition saved',
// });
});

it('renders an error notification if there was a problem recording a condition', async () => {
Expand Down Expand Up @@ -175,8 +194,77 @@ describe('Conditions Form', () => {
expect(submitButton).not.toBeDisabled();
await user.click(submitButton);
});

it('validates the form against the provided zod schema before submitting it', async () => {
const user = userEvent.setup();

mockOpenmrsFetch.mockResolvedValue({ data: [] } as FetchResponse);
mockCreateCondition.mockResolvedValue({ status: 201, body: 'Condition created' });
mockUseConditionsSearch.mockReturnValue({
searchResults: searchedCondition,
error: null,
isSearching: false,
});

renderConditionsForm();

const conditionSearchInput = screen.getByRole('searchbox', { name: /enter condition/i });
const submitButton = screen.getByRole('button', { name: /save & close/i });

await user.click(submitButton);

expect(screen.getByText(/a condition is required/i)).toBeInTheDocument();
expect(screen.getByText(/a clinical status is required/i)).toBeInTheDocument();

await user.type(conditionSearchInput, 'Headache');
await user.click(screen.getByRole('menuitem', { name: /headache/i }));
await user.click(submitButton);

expect(screen.getByText(/a clinical status is required/i)).toBeInTheDocument();

await user.click(screen.getByRole('radio', { name: 'Active' }));
await user.click(submitButton);

expect(screen.queryByText(/a condition is required/i)).not.toBeInTheDocument();
expect(screen.queryByText(/a clinical status is required/i)).not.toBeInTheDocument();

expect(mockShowSnackbar).toHaveBeenCalled();
expect(mockShowSnackbar).toHaveBeenCalledWith({
kind: 'success',
subtitle: 'It is now visible on the Conditions page',
title: 'Condition saved',
});
});

it('launching the form with an existing condition prepopulates the form with the condition details', async () => {
const user = userEvent.setup();

const conditionToEdit = {
clinicalStatus: 'Active',
conceptId: '117399AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA',
display: 'Hypertension',
abatementDateTime: undefined,
onsetDateTime: '2020-08-19T00:00:00+00:00',
recordedDate: '2020-08-19T18:34:48+00:00',
id: 'f4ee2cfe-3880-4ea2-a5a6-82aa8a0f6389',
};

testProps.condition = conditionToEdit;
testProps.formContext = 'editing' as 'creating' | 'editing';

mockOpenmrsFetch.mockResolvedValue({ data: mockFhirConditionsResponse } as FetchResponse);
renderConditionsForm();

expect(screen.queryByRole('searchbox', { name: /Enter condition/i })).not.toBeInTheDocument();

const inactiveStatusInput = screen.getByRole('radio', { name: 'Inactive' });
const submitButton = screen.getByRole('button', { name: /save & close/i });

await user.click(inactiveStatusInput);
await user.click(submitButton);
});
});

function renderConditionsForm() {
render(<ConditionsForm promptBeforeClosing={() => {}} {...testProps} />);
render(<ConditionsForm {...testProps} />);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';
import classNames from 'classnames';
import { type TFunction, useTranslation } from 'react-i18next';
import { useForm, FormProvider, type SubmitHandler } from 'react-hook-form';
import { zodResolver } from '@hookform/resolvers/zod';
import { z } from 'zod';
Expand All @@ -15,14 +16,26 @@ interface ConditionFormProps extends DefaultPatientWorkspaceProps {
formContext: 'creating' | 'editing';
}

const schema = z.object({
abatementDateTime: z.date().optional().nullable(),
clinicalStatus: z.string(),
conditionName: z.string({ required_error: 'A condition is required' }),
onsetDateTime: z.date().nullable(),
});
const createSchema = (formContext: 'creating' | 'editing', t: TFunction) => {
const isCreating = formContext === 'creating';

export type ConditionSchema = z.infer<typeof schema>;
const clinicalStatusValidation = z.string().refine((clinicalStatus) => !isCreating || !!clinicalStatus, {
message: t('clinicalStatusRequired', 'A clinical status is required'),
});

const conditionNameValidation = z.string().refine((conditionName) => !isCreating || !!conditionName, {
message: t('conditionRequired', 'A condition is required'),
});

return z.object({
abatementDateTime: z.date().optional().nullable(),
clinicalStatus: clinicalStatusValidation,
conditionName: conditionNameValidation,
onsetDateTime: z.date().nullable(),
});
};
Comment on lines +19 to +36
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change. I've moved the validation logic into the Zod schema declaration. Previously, error handling was handled manually using the setError function. Following this change, errors are handled automatically based on the rules described in the schema.


export type ConditionsFormSchema = z.infer<ReturnType<typeof createSchema>>;

const ConditionsForm: React.FC<ConditionFormProps> = ({
closeWorkspace,
Expand All @@ -33,78 +46,52 @@ const ConditionsForm: React.FC<ConditionFormProps> = ({
promptBeforeClosing,
}) => {
const { t } = useTranslation();

const isTablet = useLayoutType() === 'tablet';
const { conditions } = useConditions(patientUuid);
const [isSubmittingForm, setIsSubmittingForm] = useState(false);
const [errorCreating, setErrorCreating] = useState(null);
const [errorUpdating, setErrorUpdating] = useState(null);
const isEditing = formContext === 'editing';

const matchingCondition = conditions?.find((c) => c?.id === condition?.id);

const methods = useForm<ConditionSchema>({
const schema = createSchema(formContext, t);

const defaultValues = {
abatementDateTime:
isEditing && matchingCondition?.abatementDateTime ? new Date(matchingCondition?.abatementDateTime) : null,
conditionName: '',
clinicalStatus: isEditing ? matchingCondition?.clinicalStatus?.toLowerCase() ?? '' : '',
onsetDateTime: isEditing && matchingCondition?.onsetDateTime ? new Date(matchingCondition?.onsetDateTime) : null,
};

const methods = useForm<ConditionsFormSchema>({
mode: 'all',
resolver: zodResolver(schema),
defaultValues: {
abatementDateTime:
formContext == 'editing'
? matchingCondition?.abatementDateTime
? new Date(matchingCondition?.abatementDateTime)
: null
: null,
conditionName: '',
clinicalStatus: condition?.cells?.find((cell) => cell?.info?.header === 'clinicalStatus')?.value ?? '',
onsetDateTime:
formContext == 'editing'
? matchingCondition?.onsetDateTime
? new Date(matchingCondition?.onsetDateTime)
: null
: null,
},
defaultValues,
});

const {
setError,
formState: { isDirty },
} = methods;

useEffect(() => {
promptBeforeClosing(() => isDirty);
}, [isDirty, promptBeforeClosing]);

const onSubmit: SubmitHandler<ConditionSchema> = (payload) => {
setIsSubmittingForm(true);

if (formContext === 'creating') {
if (!payload.conditionName.trim()) {
setError('conditionName', {
type: 'manual',
message: t('conditionRequired', 'A condition is required'),
});
}
if (!payload.clinicalStatus) {
setError('clinicalStatus', {
type: 'manual',
message: t('clinicalStatusRequired', 'A clinical status is required'),
});
}
setIsSubmittingForm(false);
}

const onSubmit: SubmitHandler<ConditionsFormSchema> = () => {
setIsSubmittingForm(true);
};

const onError = (e) => {
console.error('Error submitting condition: ', e);
setIsSubmittingForm(false);
};
const onError = () => setIsSubmittingForm(false);

return (
<FormProvider {...methods}>
<Form className={styles.form} onSubmit={methods.handleSubmit(onSubmit, onError)}>
<ConditionsWidget
closeWorkspaceWithSavedChanges={closeWorkspaceWithSavedChanges}
conditionToEdit={condition}
editing={formContext === 'editing'}
isEditing={isEditing}
isSubmittingForm={isSubmittingForm}
patientUuid={patientUuid}
setErrorCreating={setErrorCreating}
Expand Down Expand Up @@ -136,7 +123,7 @@ const ConditionsForm: React.FC<ConditionFormProps> = ({
/>
</div>
) : null}
<ButtonSet className={isTablet ? styles.tablet : styles.desktop}>
<ButtonSet className={classNames({ [styles.tablet]: isTablet, [styles.desktop]: !isTablet })}>
<Button className={styles.button} kind="secondary" onClick={closeWorkspace}>
{t('cancel', 'Cancel')}
</Button>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import React from 'react';
import { screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { openmrsFetch, useConfig } from '@openmrs/esm-framework';
import { type FetchResponse, openmrsFetch, useConfig } from '@openmrs/esm-framework';
import { launchPatientWorkspace } from '@openmrs/esm-patient-common-lib';

import { mockFhirConditionsResponse } from '__mocks__';
import { mockPatient, renderWithSwr, waitForLoadingToFinish } from 'tools';
import ConditionsOverview from './conditions-overview.component';
Expand All @@ -12,8 +11,8 @@ const testProps = {
patientUuid: mockPatient.id,
};

const mockedUseConfig = useConfig as jest.Mock;
const mockedOpenmrsFetch = openmrsFetch as jest.Mock;
const mockUseConfig = jest.mocked(useConfig);
const mockOpenmrsFetch = jest.mocked(openmrsFetch);

jest.mock('@openmrs/esm-patient-common-lib', () => {
const originalModule = jest.requireActual('@openmrs/esm-patient-common-lib');
Expand All @@ -24,14 +23,14 @@ jest.mock('@openmrs/esm-patient-common-lib', () => {
};
});

describe('ConditionsOverview: ', () => {
describe('ConditionsOverview', () => {
beforeEach(() => {
mockedOpenmrsFetch.mockClear();
mockedUseConfig.mockReturnValue({ conditionPageSize: 5 });
mockOpenmrsFetch.mockClear();
mockUseConfig.mockReturnValue({ conditionPageSize: 5 });
});

it('renders an empty state view if conditions data is unavailable', async () => {
mockedOpenmrsFetch.mockReturnValueOnce({ data: [] });
mockOpenmrsFetch.mockResolvedValueOnce({ data: [] } as FetchResponse);

renderConditionsOverview();

Expand All @@ -53,7 +52,7 @@ describe('ConditionsOverview: ', () => {
},
};

mockedOpenmrsFetch.mockRejectedValueOnce(error);
mockOpenmrsFetch.mockRejectedValueOnce(error);

renderConditionsOverview();

Expand All @@ -68,7 +67,7 @@ describe('ConditionsOverview: ', () => {
it("renders an overview of the patient's conditions when present", async () => {
const user = userEvent.setup();

mockedOpenmrsFetch.mockReturnValueOnce({ data: mockFhirConditionsResponse });
mockOpenmrsFetch.mockResolvedValueOnce({ data: mockFhirConditionsResponse } as FetchResponse);

renderConditionsOverview();

Expand Down Expand Up @@ -100,7 +99,7 @@ describe('ConditionsOverview: ', () => {
it('clicking the Add button or Record Conditions link launches the conditions form', async () => {
const user = userEvent.setup();

mockedOpenmrsFetch.mockReturnValueOnce({ data: [] });
mockOpenmrsFetch.mockResolvedValueOnce({ data: [] } as FetchResponse);

renderConditionsOverview();

Expand Down
Loading
Loading