-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User Upload of Case Data #40
Conversation
…program source depending on the config in disease outbreak event
… and remove info from datastore
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.
Code review - wow @anagperal , thats a lot of logic! looks great, minor non-blocking changes requested.
Functional testing -
- I feel like there is a performance degrade in both dashboard and event tracker page, it could be a local issue. We can test from a performance aspect after uploading to instance.
- I dont know if its related to this PR, but if " Establish a coordination mechanism" date is filled, the save gives an error. If N/A is selected, it works fine.
- We need to review the access settings of the file created in DHIS, from the code we need to set the access setting to a user group. It should not be publicly accessible. Lets ask @bhavananarayanan which would eb the correct user group.
src/domain/entities/CasesFile.ts
Outdated
hazardTypes: Option[]; | ||
suspectedDiseases: Option[]; | ||
}): string { | ||
const { dataSource, outbreakValue, hazardTypes, suspectedDiseases } = 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.
almost same code in both getOutbreakKeyForCases and getOutbreakKey, can we abstract and reuse?
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.
In fact, I should have removed the getOutbreakKeyForCases
function since only getOutbreakKey
is used. Done!!
labels: labels, | ||
options: configurations.selectableOptions.eventTrackerConfigurations, | ||
}; | ||
return options.casesFileRepository.getTemplate().flatMap(casesFileTemplate => { |
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.
Is casesFileTemplate is required only if hasCasesDataFile? If yes, then should this getTempate fetch be moved inside the diseaseOutbreakEvent.casesDataSource === CasesDataSource.RTSL_ZEB_OS_CASE_DATA_SOURCE_USER_DEF check?
Also then, should we have two separate form data for DiseaseOutbreakEventFormData. One of type "disease-outbreak-event" which does not have any of the case related fields and another of type "disease-outbreak-event-case-data" which has the case related fields : caseDataFileTemplete, uploadedCasesDataFile,uploadedCasesDataFileId, hasInitiallyCasesDataFile
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.
Good idea! I have moved getTemplate() only when create a new event and on edit only when diseaseOutbreakEvent.casesDataSource === CasesDataSource.RTSL_ZEB_OS_CASE_DATA_SOURCE_USER_DEF
. It cannot be separated because the data is needed since the case data source is not yet known when a new event occurs. If in the future we can create a new event selecting the case data source from the outside, we should separate them into 2 forms
@@ -334,132 +342,238 @@ export function useForm(formType: FormType, id?: Id): State { | |||
[configurableForm] | |||
); | |||
|
|||
const onPrimaryButtonClick = useCallback(() => { | |||
const onSaveDiseaseOutbreakEvent = useCallback(() => { |
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.
should we move onSaveDiseaseOutbreakEvent and all related to logic to a seperate hook? useForm can have all the generic logic across all forms and the new hook can be specific to DiseaseOutbreakEvent
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.
Done!!
… diseaseOutbreakEvent.casesDataSource === CasesDataSource.RTSL_ZEB_OS_CASE_DATA_SOURCE_USER_DEF
…event id. Upload file with correct sharing settings
Thanks @9sneha-n !!
|
📌 References
📝 Implementation
📹 Screenshots/Screen capture
CASE_DATA_TEMPLATE (1).xlsx
Screencast.from.2024-12-16.16-10-55.webm
Screencast.from.2024-12-16.16-05-52.webm
Screencast.from.2024-12-16.16-15-41.webm
Screencast.from.2024-12-23.13-12-56.webm
🔥 Notes to the tester