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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f9d5a54
(feat) O3-4065 Allow manual entry of auto generated identifiers
ynurmahomed Oct 3, 2024
16b36c3
Merge branch 'main' into O3-4065
ynurmahomed Oct 14, 2024
287a237
Merge branch 'main' into O3-4065
ynurmahomed Oct 16, 2024
a930046
Merge branch 'main' into O3-4065
ynurmahomed Oct 18, 2024
b61fc45
Remove id field configuration
ynurmahomed Oct 18, 2024
e4f41e2
Merge branch 'main' into O3-4065
ynurmahomed Oct 22, 2024
ecf4f7a
Update packages/esm-patient-registration-app/src/patient-registration…
ynurmahomed Oct 22, 2024
0de4369
Restore line
ynurmahomed Oct 22, 2024
c4daf81
Update packages/esm-patient-registration-app/src/patient-registration…
ynurmahomed Oct 22, 2024
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
View file
Open in desktop
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
12 changes: 7 additions & 5 deletions ...ges/esm-patient-registration-app/src/patient-registration/field/id/id-field.component.tsx
View file
Open in desktop
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
33 changes: 26 additions & 7 deletions packages/esm-patient-registration-app/src/patient-registration/field/id/id-field.test.tsx
View file
Open in desktop
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');
});
});
});
16 changes: 16 additions & 0 deletions packages/esm-patient-registration-app/src/patient-registration/form-manager.test.ts
View file
Open in desktop
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 () => {
ynurmahomed marked this conversation as resolved.
Show resolved Hide resolved
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);
});
});
});
14 changes: 9 additions & 5 deletions packages/esm-patient-registration-app/src/patient-registration/form-manager.ts
View file
Open in desktop
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
27 changes: 18 additions & 9 deletions ...app/src/patient-registration/input/custom-input/identifier/identifier-input.component.tsx
View file
Open in desktop
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;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why this line was moved.

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 || '' }),
ynurmahomed marked this conversation as resolved.
Show resolved Hide resolved
});
};

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