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 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
6 changes: 1 addition & 5 deletions cypress/e2e/EnrollmentPage/StagesAndEventsWidget.feature
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,11 @@ Feature: User interacts with Stages and Events Widget
And you see the first 5 events in the table
And you see buttons in the footer list

Scenario: User can view more events
Scenario: User can view more events and then view less
Given you open the enrollment page which has multiples events and stages
When you click show more button in stages&event list
Then more events should be displayed
And reset button should be displayed

Scenario: User can reset events
Given you open the enrollment page which has multiples events and stages
When you click show more button in stages&event list
And you click reset button
Then there should be 5 rows in the table

Expand Down
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
@@ -1,11 +1,12 @@
// @flow
import { useMemo } from 'react';
import log from 'loglevel';
import { errorCreator } from 'capture-core-utils';
import i18n from '@dhis2/d2-i18n';
import type { apiProgramStage } from 'capture-core/metaDataStoreLoaders/programs/quickStoreOperations/types';
import { Program } from '../../../../../metaData';

export const useProgramStages = (program: Program, programStages?: Array<apiProgramStage>) => {
export const useProgramStages = (program: Program, programStages?: Array<apiProgramStage>) => useMemo(() => {
const stages = [];
if (program && programStages) {
program.stages.forEach((item) => {
Expand Down Expand Up @@ -48,4 +49,4 @@ export const useProgramStages = (program: Program, programStages?: Array<apiProg


return stages;
};
}, [program, programStages]);
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
Loading
Loading