fix: handle compulsory data element operands [DHIS2-17647] #426
Conversation
|
🚀 Deployed on https://pr-426--dhis2-data-entry.netlify.app |
| @@ -453,6 +464,7 @@ const metadata = { | |||
|
|
|||
| describe('<CategoryComboTableBody />', () => { | |||
| useMetadata.mockReturnValue({ data: metadata }) | |||
| useDataSetId.mockReturnValue(['dataSet1']) | |||
There was a problem hiding this comment.
(FYI: edit was necessary because of thenew call to the useIsCompulsoryDataElementOperand hook
|
|
||
| return () => { | ||
| let promise | ||
|
|
||
| if (isLoading && offline) { | ||
| if (hasCompulsoryDataElementOperandsToFillOut) { | ||
| setCompleteAttempted(true) |
There was a problem hiding this comment.
this is only set if there is a compulsory data element operand to fill out, so that invalid styling only gets applied if the compulsory data element operands need to be filled out (otherwise, they remain with just the *)
| @@ -4,6 +4,7 @@ import create from 'zustand' | |||
| const inititalState = { | |||
| errors: {}, | |||
| warnings: {}, | |||
| completeAttempted: false, | |||
There was a problem hiding this comment.
I couldn't totally decide where to put this, but thought that since it was going to apply error styling to cells, I would put it in the store with the errors information.
| import { useDataSetId } from './use-context-selection/use-context-selection.js' | ||
| import { useDataValueSet } from './use-data-value-set/use-data-value-set.js' | ||
|
|
||
| export const useIsCompulsoryDataElementOperand = ({ |
There was a problem hiding this comment.
I colocated these hooks because I didn't really want to create a new folder in src/shared directory. I'd also vote for moving the miscellaneous hooks in src/shared into src/shared/hooks, and then I could create a folder and two separate hooks for these, but I didn't want to create two separate files with the the existing file structure.
| @@ -12,7 +12,7 @@ import { | |||
| * @params {Object} options | |||
| * @params {Boolean} options.enabled - Defaults to `true` | |||
| **/ | |||
| export default function useImperativeValidate() { | |||
| export const useImperativeValidate = () => { | |||
There was a problem hiding this comment.
this is not necessary FYI, but I was having some trouble with jest mocking when it was a default, so I changed to be a named export.
| if (isLoading && offline) { | ||
| if (hasCompulsoryDataElementOperandsToFillOut) { | ||
| setCompleteAttempted(true) | ||
| cancelCompletionMutation() |
There was a problem hiding this comment.
I think that doing completeStatus: { ...previousDataValueSet.completeStatus, complete: !completed, }, in cancelCompletionMutation messes up the complete status a bit. In you try to mark as complete successfully, then mark as incomplete successfully and then finally mark as complete unsuccessfully you will see the complete status = true even if the completion did fail. maybe we just want to set it as completeStatus: { ...previousDataValueSet.completeStatus, complete: false, }, ?
There was a problem hiding this comment.
Thanks! Fixed this. I did't pay enough attention to what the cancelCompletionMutation function was doing before. I guess as it's used for cancelling incomplete functions as well, I've modified the hook to return a function to which you can pass the desired complete status (i.e. false here).
This works as far as I can tell though I have not gone through in total detail to review all of the logic with the completion.
## [100.11.4](v100.11.3...v100.11.4) (2024-12-10) ### Bug Fixes * handle compulsory data element operands [DHIS2-17647] ([#426](#426)) ([1d6c667](1d6c667))
|
🎉 This PR is included in version 100.11.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR addresses the referenced ticket (DHIS2-17647) and in general
(Note when I was testing this, I just made a couple of new data elements and assigned them to a new data set and then marked some as compulsory data element operands)
Before:
After:
*when they are empty. They then will get other applicable styling as values are entered (e.g. synced, error, warning)compulsoryFieldsCompleteOnly: true("Complete allowed only if compulsory fields are filled in" in the maintenance app), a snack bar message will appear saying that the form cannot be completed due to compulsory data element operands and the styling on the cells will be set to red/invalid/not saved (this styling will update as values are entered)Testing
useHasCompulsoryDataElementOperandsToFillOuthook and logic preventing completion inuseOnCompleteCallback)useIsCompulsoryDataElementOperandhook) partly because we didn't have existing tests and more because the InnerWrapper component where this can be tested is being refactored due to the performance PR (fix: remove FinalForm dependency in main form [DHIS2-18373] #425) so the mocking would have to be redone if I added some now. I find these tests less crucial in any case because the styling doesn't block user from entering data.I've also done some manual testing (see note above).