feat(data export): add default and per type id scheme options - #2286
Conversation
|
🚀 Deployed on https://pr-2286--dhis2-import-export.netlify.app |
Chisomchima
changed the title
Chisomchima
changed the title
980634f to
d861312
Compare
Add a "(Default)" option to the ID scheme dropdowns. Selecting it omits the corresponding *IdScheme parameter from the request so the server applies its own default for each object type, rather than forcing an explicit idScheme=UID. Add explicit dropdowns on the data export page for the remaining per-type ID scheme parameters that previously had no UI control: categoryIdScheme, categoryOptionIdScheme, categoryOptionComboIdScheme, dataSetIdScheme and attributeOptionComboIdScheme. These default to "(Default)" (param omitted) so existing exports are unchanged. Consolidate the three near-identical ElementSchemes ID scheme components into a single IdSchemeSelect. Guard every *IdScheme param across the export and import form helpers so an empty value drops the param.
- Add src/utils/idSchemeParams.js as the single source of truth for which ID scheme params each endpoint accepts and for dropping schemes left at "(Default)". All six export/import form helpers now use it instead of repeating the "empty value => omit param" logic in three different shapes. - Export valuesToParams from the Data/Event/TEI export helpers and add unit tests covering explicit, "(Default)", and unsupported-param cases, plus tests for idSchemeParams and IdSchemeSelect. - IdSchemeSelect: drop the exhaustive-deps suppression via a stable attributeTypes default, add an unmount guard, and replace the implicit reduce with a named intersect helper. - Offer attribute-based schemes for Category / Category option / Data set (these object types support custom unique attributes); note why the combo types do not. - Map the five new inputs in the Cypress nameToDataTest helper, delete two empty EventIdScheme placeholder files, regenerate i18n/en.pot.
73f5495 to
380f19a
Compare
Extract the org-unit, assigned-user, program-filter and last-updated sections into small pure helpers that each return a params object, and build the query string by merging them. Behaviour is unchanged; this brings valuesToParams under the SonarQube cognitive-complexity limit.
tomzemp
left a comment
tomzemp
left a comment
There was a problem hiding this comment.
I think it might be good to check with Jan B and product team that they are happy with how the options are presented
From a code perspective: I don't see the real advantage of all the individual IdScheme components, like it seems like this increases the maintenance burden to have individually defined components that are more or less identical? If we just do something like
<IdSchemeSelect
name='categoryIdScheme'
label='input-category-id-scheme'
dataTest={ i18n.t('Category ID scheme')}
attributeTypes={['categoryAttribute']}
/>
we'd achieve the same as what we do with CategoryIdScheme without creating a separate file?
The pattern of having multiple separate components seems useful to me in cases where we reasonably expect the inputs to diverge and hence want to already plan for that, but since we don't have that expectation, it seems like reusing one generic component is less cluttered.
The recorded Cypress network-shim fixtures match requests by exact path string, so the id-scheme params must stay ahead of paging/ totalPages to match; the complexity-refactor had moved them after, breaking CI. Also guard idSchemeEntries against an unknown endpoint and add coverage for it. Also strip explanatory comments from the source/test files this branch touched, per request.
The previous commit stripped comments from every file this branch touched, but only the comments this PR itself introduced were meant to go - restore the comments that already existed in these files on master (PAGE INFO markers, xhr/URL-generation notes, the TEI org-unit/program-status explanations, etc). Comments on code that's new to this PR (idSchemeParams.js, IdSchemeSelect.jsx, the `compact` helper) stay removed.
Chisomchima
changed the title
Category, category option, category option combo, data set, and attribute option combo ID scheme fields were each a near-identical wrapper component used in exactly one place (Data Export). Replace them with direct IdSchemeSelect usage and centralise their shared attribute-type arrays, removing duplicated boilerplate without touching the fields (idScheme, dataElementIdScheme, orgUnitIdScheme) that are genuinely reused across multiple pages.
tomzemp
left a comment
tomzemp
left a comment
There was a problem hiding this comment.
Looks good!
Maybe we should default to the default rather than UID now for data element/org unit/id scheme? (e.g. change to use '' here:
import-export-app/src/pages/DataExport/DataExport.jsx
Lines 67 to 69 in a6575df
I also think you might have deleted some comments unrelated to your work? (Like in src/utils/helper.js?)
Jan wanted the empty option worded differently depending on what it means: "(Default)" only on the fallback idScheme dropdown (defers to the server's default), "(Undefined)" on the per-object-type ones (defers to the fallback dropdown instead). He also asked for Data import to offer the same per-type dropdowns as Data export. Following that up, Tom/Jan agreed the per-type and fallback dropdowns should default to unset rather than an explicit UID, so that deferring is the default behavior rather than something users have to opt into. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Jan wanted the empty option worded differently depending on what it means: "(Default)" only on the fallback idScheme dropdown (defers to the server's default), "(Undefined)" on the per-object-type ones (defers to the fallback dropdown instead). He also asked for Data import to offer the same per-type dropdowns as Data export. Following that up, Tom/Jan agreed the per-type and fallback dropdowns should default to unset rather than an explicit UID, so that deferring is the default behavior rather than something users have to opt into.
…rt-export-app into feat/export-id-scheme-options
|
Thanks for the suggestion! Checked with Jan and he agreed — data element/org unit/idScheme now default to "(Undefined)"/"(Default)" instead of explicit UID, matching the other 5 dropdowns. Pushed. FYI, only the Id scheme has "Default" on the list now, the rest is "Undefined". |
This branch was previously deployed
4 participants
Screen.Recording.2026-09-15.at.16.24.33.mov
implements https://dhis2.atlassian.net/browse/DHIS2-20434