-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: data sets validation fields [DHIS2-18707] #513
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dhis2-maintenance-app-beta ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Additional note: For the compulsory data element operands:
|
|
||
return ( | ||
<> | ||
<Field name="compulsoryDataElementOperands"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Field
is from ReactFinalForm
and this creates a subscription. You already have the subscription through useField
-above, so this is not needed.
} | ||
|
||
export const useGetCompulsoryDataElementOperandsOptions = () => { | ||
const { input: dseInput } = useField('dataSetElements') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to take this as a parameter. This makes it more clear in CompulsoryDataElementsTransfer
that the field depends on dataSetElements
Thanks a lot for implementing this! The However, I do feel like this implementation strays quite a lot from the rest of the app - and especially with the eg. Something like this should suffice: export const useCompulsoryDataElementOperandsQuery = ({
dataSetElements,
}: {
dataSetElements: DataSetElement[]
}) => {
const queryFn = useBoundResourceQueryFn()
const relevantCatCombos = getRelevantCategoryCombos(dataSetElements)
const queryResult = useQuery({
queryKey: [
{
resource: 'categoryCombos',
params: {
fields: [
'id,displayName,categoryOptionCombos[id,displayName]',
],
filter: [`id:in:[${relevantCatCombos.join(',')}]`],
paging: false,
},
},
],
queryFn: queryFn<{ categoryCombos: CategoryComboTruncated[] }>,
enabled: relevantCatCombos?.length > 0,
staleTime: 60 * 1000,
select: (data) => // might be a good idea to wrap this in useMemo
getOptions({
categoryCombos: data.categoryCombos,
dataSetElements,
}),
})
return queryResult
} |
({ id }: { id: string }) => optionIds.includes(id) | ||
) | ||
input.onChange(filteredSelected) | ||
}, [options]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently violating exhaustive deps - and there's not really a good way to do add input
without causing an infinite loop. But please add a comment descriping why input
is not part of it.
This PR adds the Validation section for Data Set forms: DHIS2-18707
Notes:
timelyDays
attribute, based on the old app we are adding a default value 15Validation form
