(refactor) Refactor conditions form validation logic (#1881)

* (refactor) Refactor conditions form validation logic

* Fixup

* Update tests

packages/esm-patient-conditions-app/src/conditions/conditions-form.test.tsx

@@ -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';
@@ -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));
 
@@ -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();
 
@@ -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,
@@ -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 () => {
@@ -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} />);
 }

packages/esm-patient-conditions-app/src/conditions/conditions-form.workspace.tsx

@@ -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';
@@ -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(),
+  });
+};
+
+export type ConditionsFormSchema = z.infer<ReturnType<typeof createSchema>>;
 
 const ConditionsForm: React.FC<ConditionFormProps> = ({
   closeWorkspace,
@@ -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}
@@ -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>

packages/esm-patient-conditions-app/src/conditions/conditions-overview.test.tsx

@@ -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';
@@ -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');
@@ -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();
 
@@ -53,7 +52,7 @@ describe('ConditionsOverview: ', () => {
       },
     };
 
-    mockedOpenmrsFetch.mockRejectedValueOnce(error);
+    mockOpenmrsFetch.mockRejectedValueOnce(error);
 
     renderConditionsOverview();
 
@@ -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();
 
@@ -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();