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

(feat) O3-4065 Allow manual entry of auto generated identifiers #1338

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
10 changes: 10 additions & 0 deletions packages/esm-patient-registration-app/src/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export interface RegistrationConfig {
month: number;
};
};
id: {
allowAutoGenerationManualEntry: boolean;
};
phone: {
personAttributeUuid: string;
validation?: {
Expand Down Expand Up @@ -332,6 +335,13 @@ export const esmPatientRegistrationSchema = {
},
},
},
id: {
allowAutoGenerationManualEntry: {
_type: Type.Boolean,
_default: false,
_description: 'Allow manual entry of auto-generated identifiers values',
},
},
phone: {
personAttributeUuid: {
_type: Type.UUID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ export function setIdentifierSource(
selectedSource: IdentifierSource;
} {
const autoGeneration = identifierSource?.autoGenerationOption?.automaticGenerationEnabled;
const manualEntryEnabled = identifierSource?.autoGenerationOption?.manualEntryEnabled;
return {
selectedSource: identifierSource,
autoGeneration,
identifierValue: autoGeneration
? 'auto-generated'
: identifierValue !== 'auto-generated'
? identifierValue
: initialValue,
identifierValue:
autoGeneration && !manualEntryEnabled
? 'auto-generated'
: identifierValue !== 'auto-generated'
? identifierValue
: initialValue,
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React from 'react';
import userEvent from '@testing-library/user-event';
import { getDefaultsFromConfigSchema, useConfig } from '@openmrs/esm-framework';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Form, Formik } from 'formik';
import { getDefaultsFromConfigSchema, useConfig } from '@openmrs/esm-framework';
import { Identifiers } from './id-field.component';
import { mockOpenmrsId, mockIdentifierTypes, mockPatient, mockSession } from '__mocks__';
import { type RegistrationConfig, esmPatientRegistrationSchema } from '../../../config-schema';
import { type Resources, ResourcesContext } from '../../../offline.resources';
import React from 'react';
import { mockIdentifierTypes, mockOpenmrsId, mockPatient, mockSession } from '__mocks__';
import { esmPatientRegistrationSchema, type RegistrationConfig } from '../../../config-schema';
import { ResourcesContext, type Resources } from '../../../offline.resources';
import { PatientRegistrationContext, type PatientRegistrationContextProps } from '../../patient-registration-context';
import { type IdentifierSource } from '../../patient-registration.types';
import { Identifiers, setIdentifierSource } from './id-field.component';

const mockUseConfig = jest.mocked(useConfig<RegistrationConfig>);

Expand Down Expand Up @@ -120,3 +121,21 @@ describe('Identifiers', () => {
expect(screen.getByRole('button', { name: 'Close overlay' })).toBeInTheDocument();
});
});

describe('setIdentifierSource', () => {
describe('auto-generation', () => {
it('should return auto-generated as the identifier value', () => {
const identifierSource = { autoGenerationOption: { automaticGenerationEnabled: true } } as IdentifierSource;
const { identifierValue } = setIdentifierSource(identifierSource, '', '');
expect(identifierValue).toBe('auto-generated');
});

it('should return the identifier value when manual entry enabled', () => {
const identifierSource = {
autoGenerationOption: { automaticGenerationEnabled: true, manualEntryEnabled: true },
} as IdentifierSource;
const { identifierValue } = setIdentifierSource(identifierSource, '10001V', '');
expect(identifierValue).toBe('10001V');
});
});
});
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { FormManager } from './form-manager';
import { type FormValues } from './patient-registration.types';
import { generateIdentifier } from './patient-registration.resource';

jest.mock('./patient-registration.resource');

const mockGenerateIdentifier = generateIdentifier as jest.Mock;

const formValues: FormValues = {
patientUuid: '',
givenName: '',
Expand Down Expand Up @@ -66,5 +69,18 @@ describe('FormManager', () => {
},
]);
});
it('should generate identifier if it has autoGeneration and manual entry disabled', async () => {
formValues.identifiers.foo.autoGeneration = true;
formValues.identifiers.foo.selectedSource.autoGenerationOption.manualEntryEnabled = false;
mockGenerateIdentifier.mockResolvedValue({ data: { identifier: '10001V' } });
await FormManager.savePatientIdentifiers(true, undefined, formValues.identifiers, {}, 'Nyc');
expect(mockGenerateIdentifier.mock.calls).toHaveLength(1);
});
it('should not generate identifiers if manual entry enabled and identifier value given', async () => {
formValues.identifiers.foo.autoGeneration = true;
formValues.identifiers.foo.selectedSource.autoGenerationOption.manualEntryEnabled = true;
await FormManager.savePatientIdentifiers(true, undefined, formValues.identifiers, {}, 'Nyc');
expect(mockGenerateIdentifier.mock.calls).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,15 @@ export class FormManager {
initialValue,
} = patientIdentifier;

const identifier = !autoGeneration
? identifierValue
: await (
await generateIdentifier(selectedSource.uuid)
).data.identifier;
const autoGenerationManualEntry =
autoGeneration && selectedSource?.autoGenerationOption?.manualEntryEnabled && !!identifierValue;

const identifier =
!autoGeneration || autoGenerationManualEntry
? identifierValue
: await (
await generateIdentifier(selectedSource.uuid)
).data.identifier;
const identifierToCreate = {
uuid: identifierUuid,
identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,17 @@ interface IdentifierInputProps {
}

const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fieldName }) => {
const { autoGeneration, initialValue, identifierValue, identifierName, required, selectedSource } = patientIdentifier;
const { t } = useTranslation();
const { defaultPatientIdentifierTypes } = useConfig();
const { defaultPatientIdentifierTypes, fieldConfigurations } = useConfig();
const { identifierTypes } = useContext(ResourcesContext);
const { isOffline, values, setFieldValue } = useContext(PatientRegistrationContext);
const identifierType = useMemo(
() => identifierTypes.find((identifierType) => identifierType.uuid === patientIdentifier.identifierTypeUuid),
[patientIdentifier, identifierTypes],
);
const { autoGeneration, initialValue, identifierValue, identifierName, required } = patientIdentifier;
const manualEntryEnabled =
fieldConfigurations.id.allowAutoGenerationManualEntry && selectedSource?.autoGenerationOption?.manualEntryEnabled;
Copy link
Member

Choose a reason for hiding this comment

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

Is there reason we need to add the new O3 config property here (ie "allowAutoGenerationManulEntry") instead of entirely relying on the "autogenerationOption.manualEntryEnabled" config that comes from the IDGen module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mogoodrich The config property is because I was not sure if this feat should be enabled by default. It can be removed.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we need extra configuration here. Makes sense to remove it.

const [hideInputField, setHideInputField] = useState(autoGeneration || initialValue === identifierValue);
const name = `identifiers.${fieldName}.identifierValue`;
const [identifierField, identifierFieldMeta] = useField(name);
Expand All @@ -46,8 +48,8 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
setFieldValue(`identifiers.${fieldName}`, {
...patientIdentifier,
identifierValue: initialValue,
selectedSource: null,
autoGeneration: false,
selectedSource,
autoGeneration,
} as PatientIdentifierValue);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [initialValue, setHideInputField]);
Expand All @@ -57,6 +59,7 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
setFieldValue(`identifiers.${fieldName}`, {
...patientIdentifier,
...setIdentifierSource(identifierType?.identifierSources?.[0], initialValue, initialValue),
...(autoGeneration && manualEntryEnabled && { identifierValue: initialValue || '' }),
});
};

Expand All @@ -83,9 +86,12 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
}
};

const showEditButton = !required && hideInputField && (!!initialValue || manualEntryEnabled);
const showResetButton =
(!!initialValue && initialValue !== identifierValue) || (!hideInputField && manualEntryEnabled);
return (
<div className={styles.IDInput}>
{!autoGeneration && !hideInputField ? (
{!hideInputField ? (
<Input
id={name}
labelText={identifierName}
Expand All @@ -99,8 +105,10 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
/>
) : (
<div className={styles.textID}>
<p className={styles.label}>{identifierName}</p>
<p data-testid="identifier-label" className={styles.bodyShort02}>
<p data-testid="identifier-label" className={styles.label}>
{required ? identifierName : `${identifierName} (${t('optional', 'optional')})`}
Copy link
Member

Choose a reason for hiding this comment

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

Different languages have different requirements around word order, so it would be better to make this whole thing translatable. See the i18next docs.

</p>
<p data-testid="identifier-placeholder" className={styles.bodyShort02}>
{autoGeneration ? t('autoGeneratedPlaceholderText', 'Auto-generated') : identifierValue}
</p>
<input data-testid="identifier-input" type="hidden" {...identifierField} disabled />
Expand All @@ -111,9 +119,10 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
</div>
)}
<div style={{ marginBottom: '1rem' }}>
{!patientIdentifier.required && patientIdentifier.initialValue && hideInputField && (
{showEditButton && (
<UserHasAccess privilege="Edit Patient Identifiers">
<Button
data-testid="edit-button"
size="md"
kind="ghost"
onClick={handleEdit}
Expand All @@ -124,7 +133,7 @@ const IdentifierInput: React.FC<IdentifierInputProps> = ({ patientIdentifier, fi
</Button>
</UserHasAccess>
)}
{initialValue && initialValue !== identifierValue && (
{showResetButton && (
<UserHasAccess privilege="Edit Patient Identifiers">
<Button
size="md"
Expand Down
Loading
Loading