[fix]: Compulsory DE operands import error#1055
[fix]: Compulsory DE operands import error#1055deeonwuli wants to merge 12 commits intodevelopmentfrom
Conversation
…oc (default if missing)
…on combo mocks in tests
BundleMonNo change in files bundle size Groups updated (1)
Final result: ✅ View report in BundleMon website ➡️ |
There was a problem hiding this comment.
[code-only review] Thanks @deeonwuli! Just some minor comments and clarifications :)
| const fixedCompulsoryDEOperands = dataSet.compulsoryDataElementOperands.map(operand => { | ||
| if (operand.categoryOptionCombo?.id) return operand; | ||
|
|
||
| return defaultCategoryOptionCombo |
There was a problem hiding this comment.
defaultCategoryOptionCombo will always be truthy at this point because a guard is in place after declaration. The ternary operation can be simplified into a spread directly
| } | ||
|
|
||
| when(mockedMetadataRepository.listAllMetadata(anything())).thenResolve([]); | ||
| when(mockedMetadataRepository.getCategoryOptionCombos(anything())).thenResolve([]); |
There was a problem hiding this comment.
Might be worth adding a test for when getCategoryOptionCombos returns a real combo, verifying that operands with a missing categoryOptionCombo.id get it replaced with the default one.
…rned early if undefined
…missing category option combo
… data element operands with default coc id
…-de-operands-import-error
gqcorneby
left a comment
There was a problem hiding this comment.
[code-only review] Thanks @deeonwuli!!
xurxodev
left a comment
There was a problem hiding this comment.
Thanks @deeonwuli
I tried test following the steps in the task using the dataset that indicate the task in our WIDP instance but I can't because it's failing a sync using dataset NTD GNARF(NnhyjiUbcJN)
I did a direct url call and the compulsory data element operands is ok.
This PR asume this bug and patch the payload if categoryOptionCombo.id is empty but what is the reason for this situation in the sync?
because a direct api url is ok how you can see in the image and this url: https://dev.eyeseetea.com/who-dev-41/api/dataSets/NnhyjiUbcJN?fields=id,name,compulsoryDataElementOperands
In this moment I have the doubt if it's possible fix the problem without patch the payload but in the previous calls. I don't know if you reviewed this possibility, if it's possible the patch is not necessary and the fix should be done before.
Anyway I can't tests because the sync is failing, can you review it?
…ameter and adjust related logic
|
Hi @xurxodev, thanks for the review. You were right that patching the payload was not fixing it at the source. I found that the root cause was Fix: Instead of correcting operands during payload construction, the fetch itself now preserves those nested default references: About the testing blocker (NTD GNARF / 500 error): I raised the issue with Arnau in this ClickUp task. This error has not been further explored and has not come up in any of our other projects yet. So it looks unrelated to this PR. |
📌 References
📝 Implementation
Problem: When dataSets are exported, compulsory data element operands may lack a
categoryOptionCombo.id, causing import failures.Solution: This fix ensures all data element operands in the dataset payload have valid coc id, defaulting to the "default" coc id if not present
📹 Screenshots/Screen capture
📑 Others
#869cemv66