fix: [DHIS2-15814] missing orgunit names #3449
Conversation
|
🚀 Deployed on https://deploy-preview-3449--dhis2-capture.netlify.app |
| @@ -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'; | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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!
| @@ -2,6 +2,7 @@ | |||
| import { useMemo } from 'react'; | |||
| import { convertValue } from '../../../../converters/serverToClient'; | |||
| import { dataElementTypes } from '../../../../metaData'; | |||
| import { useOrgUnitNames } from '../../../../metadataRetrieval/orgUnitName'; | |||
There was a problem hiding this comment.
Same concerns here about adding a dependency to the self-contained WidgetProfile on external metadata.
| [setFetching, setError], | ||
| ); | ||
|
|
||
| const { refetch } = useDataQuery( |
There was a problem hiding this comment.
The ticket description mentions using react-query, what was the reason behind using the app-runtime hook instead? Thanks!
There was a problem hiding this comment.
I had a look at react-query, but came to the conclusion that it wasn't the right tool for the job; for one it caches by default based on all parameters in a request. In the case of working lists we want to fetch several names in a single request and cache each name separately. According to @eirikhaugstulen it should be possible to set up react-query so that it caches this way, but it implies having to battle with the system. Secondly I got the impression that one of the main merits with using react-query is the built-in memory management system, with stale times and automatic garbage collection. In the case of translated orgunit names we agreed there was no reason to expect changes during a single session. Thinking about it now it could be used for saving a little memory though, but hardly noticeable most likely.
There was a problem hiding this comment.
Sounds good! Thanks for the explanation 😄
## [100.44.3](v100.44.2...v100.44.3) (2023-11-10) ### Bug Fixes * [DHIS2-15814] missing orgunit names ([#3449](#3449)) ([488f8c0](488f8c0))
|
🎉 This PR is included in version 100.44.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
DHIS2-15814
This should fix all the currently consistently failing cypress tests!