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

fix: [DHIS2-15814] missing orgunit names #3449

Merged
merged 15 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from 13 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 @@ -15,7 +15,7 @@ Then('the enrollment widget should be closed', () => {

Then('the enrollment widget should be opened', () => {
cy.get('[data-test="widget-enrollment"]').within(() => {
cy.get('[data-test="widget-contents"]').children().should('exist');
cy.get('[data-test="widget-enrollment-contents"]').children().should('exist');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type EventMain = {
+programStageId?: string,
+programStageName?: string,
+orgUnitId?: string,
+orgUnitName?: string,
+trackedEntityInstanceId?: string,
+enrollmentId?: string,
+enrollmentStatus?: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { searchScopes } from '../SearchBox';
import { enrollmentTypes } from './CardList.constants';
import { ListEntry } from './ListEntry.component';
import { dataElementTypes, getTrackerProgramThrowIfNotFound } from '../../metaData';
import { useOrgUnitName } from '../../metadataRetrieval/orgUnitName';
import type { ListItem, RenderCustomCardActions } from './CardList.types';


Expand Down Expand Up @@ -96,24 +97,24 @@ const deriveEnrollmentType =
return enrollmentTypes.DONT_SHOW_TAG;
};

const deriveEnrollmentOrgUnitAndDate = (enrollments, enrollmentType, currentProgramId): {orgUnitName?: string, enrolledAt?: string} => {
const deriveEnrollmentOrgUnitIdAndDate = (enrollments, enrollmentType, currentProgramId): {orgUnitId?: string, enrolledAt?: string} => {
if (!enrollments?.length) { return {}; }
if (!currentProgramId && enrollments.length) {
const { orgUnitName, enrolledAt } = enrollments[0];
const { orgUnit: orgUnitId, enrolledAt } = enrollments[0];

return {
orgUnitName,
orgUnitId,
enrolledAt,
};
}
const { orgUnitName, enrolledAt } =
const { orgUnit: orgUnitId, enrolledAt } =
enrollments
.filter(({ program }) => program === currentProgramId)
.filter(({ status }) => status === enrollmentType)
.sort((a, b) => moment.utc(a.lastUpdated).diff(moment.utc(b.lastUpdated)))[0]
|| {};

return { orgUnitName, enrolledAt };
return { orgUnitId, enrolledAt };
};

const deriveProgramFromEnrollment = (enrollments, currentSearchScopeType) => {
Expand All @@ -137,7 +138,8 @@ const CardListItemIndex = ({
}: Props) => {
const enrollments = item.tei ? item.tei.enrollments : [];
const enrollmentType = deriveEnrollmentType(enrollments, currentProgramId);
const { orgUnitName, enrolledAt } = deriveEnrollmentOrgUnitAndDate(enrollments, enrollmentType, currentProgramId);
const { orgUnitId, enrolledAt } = deriveEnrollmentOrgUnitIdAndDate(enrollments, enrollmentType, currentProgramId);
const { displayName: orgUnitName } = useOrgUnitName(orgUnitId);
const program = enrollments && enrollments.length
? deriveProgramFromEnrollment(enrollments, currentSearchScopeType)
: undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useScopeInfo } from '../../../hooks/useScopeInfo';
import { scopeTypes } from '../../../metaData';
import { TrackedEntityInstanceDataEntry } from '../TrackedEntityInstance';
import { useCurrentOrgUnitId } from '../../../hooks/useCurrentOrgUnitId';
import { useCoreOrgUnit } from '../../../metadataRetrieval/coreOrgUnit';
import { useOrgUnitName } from '../../../metadataRetrieval/orgUnitName';
import type { Props, PlainProps } from './TeiRegistrationEntry.types';
import { DiscardDialog } from '../../Dialogs/DiscardDialog.component';
import { withSaveHandler } from '../../DataEntry';
Expand Down Expand Up @@ -56,8 +56,7 @@ const TeiRegistrationEntryPlain =
const { scopeType } = useScopeInfo(selectedScopeId);
const { formId, formFoundation } = useMetadataForRegistrationForm({ selectedScopeId });
const orgUnitId = useCurrentOrgUnitId();
const { orgUnit } = useCoreOrgUnit(orgUnitId); // Tony: [DHIS2-15814] Change this to new hook
const orgUnitName = orgUnit ? orgUnit.name : '';
const { displayName: orgUnitName } = useOrgUnitName(orgUnitId);

const handleOnCancel = () => {
if (!isUserInteractionInProgress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const EnrollmentPageDefaultPlain = ({
}: PlainProps) => {
const [mainContentVisible, setMainContentVisibility] = useState(true);
const [addRelationShipContainerElement, setAddRelationshipContainerElement] =
useState<?HTMLDivElement>(undefined);
useState<HTMLDivElement | void>(undefined);

const toggleVisibility = useCallback(() => setMainContentVisibility(current => !current), []);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export type Event = {|
occurredAt: string,
updatedAt: string,
orgUnit: string,
orgUnitName: string,
program: string,
programStage: string,
status: 'ACTIVE' | 'VISITED' | 'COMPLETED' | 'SCHEDULE' | 'OVERDUE' | 'SKIPPED',
Expand All @@ -34,7 +33,6 @@ export type EnrollmentData = {|
updatedAt: string,
updatedAtClient: string,
orgUnit: string,
orgUnitName: string,
program: string,
status: string,
storedBy: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const useEventsData = (enrollment, program) => {
programId: event.program,
programStageId: event.programStage,
orgUnitId: event.orgUnit,
orgUnitName: event.orgUnitName,
trackedEntityInstanceId: event.trackedEntityInstance,
enrollmentId: event.enrollment,
enrollmentStatus: event.enrollmentStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import React, { type ComponentType, useEffect, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { ScopeSelectorComponent } from './ScopeSelector.component';
import type { OwnProps } from './ScopeSelector.types';
import { useOrganizationUnit } from './hooks';
import { useOrgUnitName } from '../../metadataRetrieval/orgUnitName';
import { resetOrgUnitIdFromScopeSelector } from './ScopeSelector.actions';


const deriveReadiness = (lockedSelectorLoads, selectedOrgUnitId, selectedOrgUnitName) => {
const deriveReadiness = (lockedSelectorLoads, selectedOrgUnitId, selectedOrgUnitName, displayName) => {
// because we want the orgUnit to be fetched and stored
// before allowing the user to view the locked selector
if (selectedOrgUnitId && selectedOrgUnitName) {
return true;
if (selectedOrgUnitId && (!selectedOrgUnitName || selectedOrgUnitName !== displayName)) {
return false;
}
return !lockedSelectorLoads;
};
Expand All @@ -32,21 +32,20 @@ export const ScopeSelector: ComponentType<OwnProps> = ({
children,
}) => {
const dispatch = useDispatch();
const { refetch: refetchOrganisationUnit, displayName } = useOrganizationUnit();
const [selectedOrgUnit, setSelectedOrgUnit] = useState({ name: displayName, id: selectedOrgUnitId });
const [selectedOrgUnit, setSelectedOrgUnit] = useState({ name: undefined, id: selectedOrgUnitId });
const { displayName } = useOrgUnitName(selectedOrgUnit.id);

useEffect(() => {
const missName = !selectedOrgUnit.name;
const hasDifferentId = selectedOrgUnit.id !== selectedOrgUnitId;

selectedOrgUnitId &&
(hasDifferentId || missName) &&
refetchOrganisationUnit({ variables: { selectedOrgUnitId } });
}, [selectedOrgUnitId]); // eslint-disable-line react-hooks/exhaustive-deps
if (displayName && selectedOrgUnit.name !== displayName) {
setSelectedOrgUnit(prevSelectedOrgUnit => ({ ...prevSelectedOrgUnit, name: displayName }));
}
}, [displayName, selectedOrgUnit, setSelectedOrgUnit]);

useEffect(() => {
displayName && setSelectedOrgUnit(prevSelectedOrgUnit => ({ ...prevSelectedOrgUnit, name: displayName }));
}, [displayName, setSelectedOrgUnit]);
if (selectedOrgUnitId && !selectedOrgUnit.id) {
selectedOrgUnitId && setSelectedOrgUnit(prevSelectedOrgUnit => ({ ...prevSelectedOrgUnit, id: selectedOrgUnitId }));
}
}, [selectedOrgUnitId, selectedOrgUnit, setSelectedOrgUnit]);

const handleSetOrgUnit = (orgUnitId, orgUnitObject) => {
setSelectedOrgUnit(orgUnitObject);
Expand All @@ -59,7 +58,7 @@ export const ScopeSelector: ComponentType<OwnProps> = ({
previousOrgUnitId: app.previousOrgUnitId,
}
));
const ready = deriveReadiness(lockedSelectorLoads, selectedOrgUnitId, selectedOrgUnit.name);
const ready = deriveReadiness(lockedSelectorLoads, selectedOrgUnit.id, selectedOrgUnit.name, displayName);

return (
<ScopeSelectorComponent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,3 @@ export { useResetStageId } from './useResetStageId';
export { useResetEventId } from './useResetEventId';
export { useReset } from './useReset';
export { useResetViewEventId } from './useResetViewEventId';

export { useOrganizationUnit } from './useOrganizationUnit';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { Status } from './Status';
import { convertValue as convertValueServerToClient } from '../../converters/serverToClient';
import { convertValue as convertValueClientToView } from '../../converters/clientToView';
import { dataElementTypes } from '../../metaData';
import { useOrgUnitName } from '../../metadataRetrieval/orgUnitName';
import { Date } from './Date';
import { Actions } from './Actions';

Expand Down Expand Up @@ -68,6 +69,7 @@ export const WidgetEnrollmentPlain = ({
const [open, setOpenStatus] = useState(true);
const { fromServerDate } = useTimeZoneConversion();
const geometryType = getGeometryType(enrollment?.geometry?.type);
const { displayName: orgUnitName } = useOrgUnitName(enrollment.orgUnit);

return (
<div data-test="widget-enrollment">
Expand All @@ -84,7 +86,7 @@ export const WidgetEnrollmentPlain = ({
)}
{loading && <LoadingMaskElementCenter />}
{!initError && !loading && (
<div className={classes.enrollment}>
<div className={classes.enrollment} data-test="widget-enrollment-contents">
<div className={classes.statuses} data-test="widget-enrollment-status">
{enrollment.followUp && (
<Tag className={classes.followup} negative>
Expand Down Expand Up @@ -125,7 +127,7 @@ export const WidgetEnrollmentPlain = ({
<IconDimensionOrgUnit16 color={colors.grey600} />
</span>
{i18n.t('Started at {{orgUnitName}}', {
orgUnitName: enrollment.orgUnitName,
orgUnitName,
interpolation: { escapeValue: false },
})}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import { errorCreator } from 'capture-core-utils';
import log from 'loglevel';
import { WidgetEnrollment as WidgetEnrollmentComponent } from './WidgetEnrollment.component';
import { useOrganizationUnit } from './hooks/useOrganizationUnit';
import { useOrgUnitName } from '../../metadataRetrieval/orgUnitName';
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO the self-contained WidgetEnrollment should not rely on external cached metadata and it should have its own logic to retrieve the orgUnitName. Can we make the widget pull its own metadata? WDYT?

Copy link
Contributor Author

@superskip superskip Oct 31, 2023

Choose a reason for hiding this comment

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

I think @JoakimSM was of the opinion that the file which useOrgUnitName comes from can be made into a proper dependency. (I.e. a dependency of the sort that one puts into package.json.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Moving useOrgUnitName to a package dependency sounds good to me (perhaps it can be one of the hooks provided by the app runtime library 🤔). To be able to follow up on this, can you create a TECH issue and link it to this ticket? Thank you!

import { useTrackedEntityInstances } from './hooks/useTrackedEntityInstances';
import { useEnrollment } from './hooks/useEnrollment';
import { useProgram } from './hooks/useProgram';
Expand Down Expand Up @@ -42,7 +42,7 @@ export const WidgetEnrollment = ({
enrollments,
refetch: refetchTEI,
} = useTrackedEntityInstances(teiId, programId);
const { error: errorOrgUnit, displayName } = useOrganizationUnit(ownerOrgUnit);
const { error: errorOrgUnit, displayName } = useOrgUnitName(typeof ownerOrgUnit === 'string' ? ownerOrgUnit : undefined);
const { error: errorLocale, locale } = useUserLocale();
const canAddNew = enrollments
.filter(item => item.program === programId)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export type EnrollmentEvent = {|
programId: string,
programStageId: string,
orgUnitId: string,
orgUnitName: string,
trackedEntityInstanceId: string,
enrollmentId: string,
enrollmentStatus: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ export type Props = {|
eventCountInOrgUnit: number,
suggestedScheduleDate?: ?string,
hideDueDate?: boolean,
orgUnit: Object,
orgUnit: { id: string, name: string },
...CssClasses,
|};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import i18n from '@dhis2/d2-i18n';
import { useDispatch } from 'react-redux';
import moment from 'moment';
import { getProgramAndStageForProgram, TrackerProgram, getProgramEventAccess } from '../../metaData';
import { useOrganisationUnit } from '../../dataQueries';
import { useOrgUnitName } from '../../metadataRetrieval/orgUnitName';
import { useLocationQuery } from '../../utils/routing';
import type { ContainerProps } from './widgetEventSchedule.types';
import { WidgetEventScheduleComponent } from './WidgetEventSchedule.component';
Expand Down Expand Up @@ -35,7 +35,7 @@ export const WidgetEventSchedule = ({
}: ContainerProps) => {
const { program, stage } = useMemo(() => getProgramAndStageForProgram(programId, stageId), [programId, stageId]);
const dispatch = useDispatch();
const { orgUnit } = useOrganisationUnit(orgUnitId, 'displayName');
const orgUnit = { id: orgUnitId, name: useOrgUnitName(orgUnitId).displayName };
const { programStageScheduleConfig } = useScheduleConfigFromProgramStage(stageId);
const { programConfig } = useScheduleConfigFromProgram(programId);
const suggestedScheduleDate = useDetermineSuggestedScheduleDate({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { useMemo } from 'react';
import { convertValue } from '../../../../converters/serverToClient';
import { dataElementTypes } from '../../../../metaData';
import { useOrgUnitNames } from '../../../../metadataRetrieval/orgUnitName';
Copy link
Contributor

Choose a reason for hiding this comment

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

Same concerns here about adding a dependency to the self-contained WidgetProfile on external metadata.


const convertDate = date => convertValue(date, dataElementTypes.DATE);

Expand All @@ -14,16 +15,26 @@ const getClientFormattedDataValuesAsObject = (dataValues, elementsById) =>
return acc;
}, {});

export const useEvents = (enrollment: any, elementsById: Array<any>) =>
useMemo(
const getOrgUnitIds = (enrollment: any): Array<string> =>
(enrollment ? enrollment.events.reduce((acc, event) => {
if (event.orgUnit) {
acc.push(event.orgUnit);
}
return acc;
}, []) : []);

export const useEvents = (enrollment: any, elementsById: Array<any>) => {
const orgUnitIds = useMemo(() => getOrgUnitIds(enrollment), [enrollment]);
const { orgUnitNames } = useOrgUnitNames(orgUnitIds);
return useMemo(
() =>
enrollment &&
enrollment && orgUnitNames &&
enrollment.events.map(event => ({
eventId: event.event,
programId: event.program,
programStageId: event.programStage,
orgUnitId: event.orgUnit,
orgUnitName: event.orgUnitName,
orgUnitName: orgUnitNames[event.orgUnit],
trackedEntityInstanceId: event.trackedEntityInstance,
enrollmentId: event.enrollment,
enrollmentStatus: event.enrollmentStatus,
Expand All @@ -32,5 +43,6 @@ export const useEvents = (enrollment: any, elementsById: Array<any>) =>
dueDate: convertDate(event.dueDate),
...getClientFormattedDataValuesAsObject(event.dataValues, elementsById),
})),
[elementsById, enrollment],
[elementsById, enrollment, orgUnitNames],
);
};
Loading
Loading