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-4071: Improve Start Visit Form to support "Visit Locations" #2103

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
10 changes: 8 additions & 2 deletions packages/esm-patient-chart-app/src/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ export const esmPatientChartSchema = {
},
disableChangingVisitLocation: {
_type: Type.Boolean,
_description:
"Whether the visit location field in the Start Visit form should be view-only. If so, the visit location will always be set to the user's login location.",
_description: 'Whether the visit location field in the Start Visit form should be view-only.',
Copy link
Member Author

Choose a reason for hiding this comment

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

The second half of this statement is no longer true, so I just removed it.

_default: false,
},
disableEmptyTabs: {
Expand Down Expand Up @@ -61,6 +60,12 @@ export const esmPatientChartSchema = {
_description: 'The UUID of the visit type to be used for the automatically created offline visits.',
_default: 'a22733fa-3501-4020-a520-da024eeff088',
},
restrictByVisitLocationTag: {
_type: Type.Boolean,
_description:
'On the start visit form, whether to restrict the visit location to locations with the Visit Location tag',
_default: false,
Copy link
Member Author

Choose a reason for hiding this comment

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

There's an argument to be made that this should default to 'true', though assumedly this would potentially break some existing implementations.

Copy link
Contributor

@chibongho chibongho Nov 13, 2024

Choose a reason for hiding this comment

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

Maybe we should mention that this requires the EMR API to be installed on the backend? (And maybe even provide a warning if it's set to true without it installed)

Copy link
Member Author

Choose a reason for hiding this comment

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

And yes, good point, we should definitely note that, will add.

},
showAllEncountersTab: {
_type: Type.Boolean,
_description: 'Shows the All Encounters Tab of Patient Visits section in Patient Chart',
Expand Down Expand Up @@ -147,6 +152,7 @@ export interface ChartConfig {
notesConceptUuids: string[];
numberOfVisitsToLoad: number;
offlineVisitTypeUuid: string;
restrictByVisitLocationTag: boolean;
showAllEncountersTab: boolean;
showExtraVisitAttributesSlot: boolean;
showRecommendedVisitTypeTab: boolean;
Expand Down
10 changes: 10 additions & 0 deletions packages/esm-patient-chart-app/src/routes.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
{
"$schema": "https://json.openmrs.org/routes.schema.json",
"optionalBackendDependencies": {
"emrapi": {
"version": "^2.0.0",
"feature": {
"flagName": "emrapi-module",
"label": "EMR API Module",
"description": "This module, if installed, provides core EMR business logic."
}
}
},
"extensions": [
{
"name": "charts-summary-dashboard",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { type FetchResponse, openmrsFetch, useConfig } from '@openmrs/esm-framew
import useSWRImmutable from 'swr/immutable';
import { type ChartConfig } from '../../config-schema';

export const useDefaultLoginLocation = () => {
export const useDefaultFacilityLocation = () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed this to a more accurate name (IMO)

const config = useConfig() as ChartConfig;
const apiUrl = config.defaultFacilityUrl;
const { data, error, isLoading } = useSWRImmutable<FetchResponse>(apiUrl, openmrsFetch);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { type FetchResponse, type Location, openmrsFetch, restBaseUrl } from '@openmrs/esm-framework';
import { useEffect } from 'react';
import useSWR from 'swr';

/**
* Fetches the default visit location based on the location and the restrictByVisitLocationTag
* If restrictByVisitLocationTag is true, it fetches the nearest ancestor location that supports visits, otherwise it returns the passed-in location
*
* @param location
* @param restrictByVisitLocationTag
*/
export function useDefaultVisitLocation(location: Location, restrictByVisitLocationTag: boolean) {
let url = `${restBaseUrl}/emrapi/locationThatSupportsVisits?location=${location?.uuid}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to check the feature flag for whether emrapi is installed

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point... I was going this would be "buyer beware" and the implementer would need to know that the EMR API module needed to be installed, but no reason not to add this check as well.


const { data, error } = useSWR<FetchResponse>(restrictByVisitLocationTag ? url : null, openmrsFetch);

useEffect(() => {
if (error) {
console.error(error);
}
}, [error]);

return !restrictByVisitLocationTag ? location : data?.data;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { type FetchResponse, openmrsFetch, type OpenmrsResource, restBaseUrl } from '@openmrs/esm-framework';
import useSWRImmutable from 'swr/immutable';
import { useMemo } from 'react';

interface EmrApiConfigurationResponse {
atFacilityVisitType: OpenmrsResource;
// TODO: extract this into Core and include all fields (see Ward app)
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 hook now existing in two places, here and in the Ward app... I can ticket adding this hook to Core.

}

const customRepProps = [['atFacilityVisitType', 'ref']];

const customRep = `custom:${customRepProps.map((prop) => prop.join(':')).join(',')}`;

export function useEmrConfiguration(isEmrApiModuleInstalled: boolean) {
const url = isEmrApiModuleInstalled ? `${restBaseUrl}/emrapi/configuration?v=${customRep}` : null;

const swrData = useSWRImmutable<FetchResponse<EmrApiConfigurationResponse>>(url, openmrsFetch);

const results = useMemo(
() => ({
emrConfiguration: swrData.data?.data,
isLoadingEmrConfiguration: swrData.isLoading,
mutateEmrConfiguration: swrData.mutate,
errorFetchingEmrConfiguration: swrData.error,
}),
[swrData],
);
return results;
}
27 changes: 0 additions & 27 deletions packages/esm-patient-chart-app/src/visit/hooks/useLocations.tsx

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,42 +1,56 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import classNames from 'classnames';
import isEmpty from 'lodash/isEmpty';
import { ComboBox } from '@carbon/react';
import { useTranslation } from 'react-i18next';
import { type Control, Controller } from 'react-hook-form';
import { type Location, type OpenmrsResource, useConfig, useSession } from '@openmrs/esm-framework';
import {
type Location,
type OpenmrsResource,
useConfig,
useSession,
useLocations,
useFeatureFlag,
} from '@openmrs/esm-framework';
import { type VisitFormData } from './visit-form.resource';
import { useDefaultLoginLocation } from '../hooks/useDefaultLocation';
import { useLocations } from '../hooks/useLocations';
import { useDefaultFacilityLocation } from '../hooks/useDefaultFacilityLocation';
import { type ChartConfig } from '../../config-schema';
import styles from './visit-form.scss';
import { useDefaultVisitLocation } from '../hooks/useDefaultVisitLocation';

interface LocationSelectorProps {
control: Control<VisitFormData>;
}

const LocationSelector: React.FC<LocationSelectorProps> = ({ control }) => {
const { t } = useTranslation();
const session = useSession();
const [searchTerm, setSearchTerm] = useState('');
const selectedSessionLocation = useSession().sessionLocation;
const { locations } = useLocations(searchTerm);
const { defaultFacility, isLoading: loadingDefaultFacility } = useDefaultLoginLocation();
const config = useConfig<ChartConfig>();
const [searchTerm, setSearchTerm] = useState('');
const sessionLocation = useSession().sessionLocation;
const isEmrApiModuleInstalled = useFeatureFlag('emrapi-module');
const defaultVisitLocation = useDefaultVisitLocation(
sessionLocation,
config.restrictByVisitLocationTag && isEmrApiModuleInstalled,
);
const locations = useLocations(
config.restrictByVisitLocationTag && isEmrApiModuleInstalled ? 'Visit Location' : null,
searchTerm,
);
const { defaultFacility, isLoading: loadingDefaultFacility } = useDefaultFacilityLocation();
const disableChangingVisitLocation = config?.disableChangingVisitLocation;
const locationsToShow: Array<OpenmrsResource> =
!loadingDefaultFacility && !isEmpty(defaultFacility)
? [defaultFacility]
: locations
? locations
: selectedSessionLocation
? [selectedSessionLocation]
: [];
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 was making sure that if there were no valid locations, the selected session location still appears in the list... which seemed incorrect.

!loadingDefaultFacility && !isEmpty(defaultFacility) ? [defaultFacility] : locations ? locations : [];

const handleSearch = (searchString) => {
setSearchTerm(searchString);
};

useEffect(() => {
if (config.restrictByVisitLocationTag && !isEmrApiModuleInstalled) {
console.warn('EMR API module is not installed. Visit location will not be restricted by location tag.');
}
}, [config.restrictByVisitLocationTag, isEmrApiModuleInstalled]);

return (
<section data-testid="combo">
<div className={styles.sectionTitle}>{t('visitLocation', 'Visit Location')}</div>
Expand All @@ -62,7 +76,7 @@ const LocationSelector: React.FC<LocationSelectorProps> = ({ control }) => {
)}
/>
) : (
<p className={styles.bodyShort02}>{session?.sessionLocation?.display}</p>
<p className={styles.bodyShort02}>{defaultVisitLocation?.display}</p>
)}
</div>
</section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ import React from 'react';
import userEvent from '@testing-library/user-event';
import { render, screen } from '@testing-library/react';
import { useForm, FormProvider } from 'react-hook-form';
import { getDefaultsFromConfigSchema, useConfig, useSession } from '@openmrs/esm-framework';
import { getDefaultsFromConfigSchema, useConfig, useLocations, useSession } from '@openmrs/esm-framework';
import { esmPatientChartSchema, type ChartConfig } from '../../config-schema';
import { mockLocationsResponse, mockSessionDataResponse } from '__mocks__';
import { type VisitFormData } from './visit-form.resource';
import { useLocations } from '../hooks/useLocations';
import LocationSelector from './location-selector.component';

const mockSession = jest.mocked(useSession);
Expand All @@ -15,70 +14,73 @@ const mockUseConfig = jest.mocked(useConfig<ChartConfig>);

mockSession.mockReturnValue(mockSessionDataResponse.data);

jest.mock('../hooks/useLocations.tsx', () => ({
useLocations: jest.fn().mockReturnValue({
locations: mockLocationsResponse,
isLoading: false,
error: null,
}),
}));

beforeEach(() => {
mockUseConfig.mockReturnValue({
...getDefaultsFromConfigSchema(esmPatientChartSchema),
});
});

it('renders a paragraph with the location name if disableChangingVisitLocation is truthy', () => {
mockUseConfig.mockReturnValue({
...getDefaultsFromConfigSchema(esmPatientChartSchema),
disableChangingVisitLocation: true,
describe('tests location selector', () => {
it('renders a paragraph with the location name if disableChangingVisitLocation is truthy', () => {
mockUseConfig.mockReturnValue({
...getDefaultsFromConfigSchema(esmPatientChartSchema),
disableChangingVisitLocation: true,
});

renderLocationSelector();

expect(screen.getByText(/visit location/i)).toBeInTheDocument();
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
expect(screen.getByRole('paragraph')).toHaveTextContent('Inpatient Ward');
});

renderLocationSelector();
it('renders a combobox with options to pick from if disableChangingVisitLocation is falsy', () => {
renderLocationSelector();

expect(screen.getByText(/visit location/i)).toBeInTheDocument();
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
expect(screen.getByRole('paragraph')).toHaveTextContent('Inpatient Ward');
});
expect(screen.getByRole('combobox', { name: /select a location/i })).toBeInTheDocument();
expect(screen.queryByRole('paragraph')).not.toBeInTheDocument();
});

it('renders a combobox with options to pick from if disableChangingVisitLocation is falsy', () => {
renderLocationSelector();
it('defaults to showing the session location as the selected location if no locations are available', () => {
mockUseLocations.mockReturnValue([]);

expect(screen.getByRole('combobox', { name: /select a location/i })).toBeInTheDocument();
expect(screen.queryByRole('paragraph')).not.toBeInTheDocument();
});
renderLocationSelector();

it('defaults to showing the session location as the selected location if no locations are available', () => {
mockUseLocations.mockReturnValue({
locations: [],
isLoading: false,
error: null,
expect(screen.getByRole('combobox')).toHaveValue('Test Location');
});

renderLocationSelector();
it('renders a list of locations to pick from if locations are available', async () => {
const user = userEvent.setup();

expect(screen.getByRole('combobox')).toHaveValue('Test Location');
});
mockUseLocations.mockReturnValue(mockLocationsResponse);

renderLocationSelector();
expect(mockUseLocations).toHaveBeenCalledWith(null, '');
expect(mockUseLocations).toHaveBeenCalledWith(null, 'Test Location');

it('renders a list of locations to pick from if locations are available', async () => {
const user = userEvent.setup();
const locationSelector = screen.getByRole('combobox');
expect(locationSelector).toBeInTheDocument();

mockUseLocations.mockReturnValue({
locations: mockLocationsResponse,
isLoading: false,
error: null,
await user.click(locationSelector);

mockLocationsResponse.forEach((location) => {
expect(screen.getByRole('option', { name: location.display })).toBeInTheDocument();
});
});

renderLocationSelector();
it('should call use locations with Visit Location restriction when restrictByVisitLocationTag set true ', async () => {
mockUseConfig.mockReturnValue({
...getDefaultsFromConfigSchema(esmPatientChartSchema),
restrictByVisitLocationTag: true,
});

const locationSelector = screen.getByRole('combobox');
expect(locationSelector).toBeInTheDocument();
const user = userEvent.setup();

await user.click(locationSelector);
mockUseLocations.mockReturnValue(mockLocationsResponse);

mockLocationsResponse.forEach((location) => {
expect(screen.getByRole('option', { name: location.display })).toBeInTheDocument();
renderLocationSelector();
expect(mockUseLocations).toHaveBeenCalledWith('Visit Location', '');
expect(mockUseLocations).toHaveBeenCalledWith('Visit Location', 'Test Location');
});
});

Expand Down
Loading