(feat) Offline Form Improvements #611
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
manuelroemer
requested review from
donaldkibet,
FlorianRappl,
denniskigen and
ibacher
|
@donaldkibet @denniskigen Tagging you guys as reviewers since you probably know most about how forms work. It would be great if you guys could have a look at these changes and check if I missed something! |
|
@ibacher Do you know if it is possible/a huge amount of effort to add more forms to Dev3? I used the legacy openmrs-spa env a lot since there are a lot more forms available, but that was not a good experience, really. Ideally, we'd have more forms available in our new dev environment. Maybe there is even a way to move the old forms from openmrs-spa over to the new dev3 version? |
|
@manuelroemer It shouldn't be a big effort. Essentially just adding the JSON for forms to this directory should automatically add them to dev3. |
|
Warning: The comment body was truncated to fit GitHub limit on comment length. File size impactMerging feature/form-offline-improvements into master impact files as follow: @openmrs/esm-form-entry-app (+0.68%)
@openmrs/esm-generic-patient-widgets-app (+0.37%)
@openmrs/esm-patient-allergies-app (+0.82%)
@openmrs/esm-patient-appointments-app (+1.11%)
@openmrs/esm-patient-attachments-app (+1.59%)
@openmrs/esm-patient-banner-app (+2.44%)
@openmrs/esm-patient-biometrics-app (+0.5%)
@openmrs/esm-patient-chart-app (+0.24%)
@openmrs/esm-patient-clinical-view-app (+1.73%)
@openmrs/esm-patient-common-lib (no impact)No file in @openmrs/esm-patient-common-lib group (see config below). {
"./packages/esm-patient-common-lib/dist/*.js": true,
"./packages/esm-patient-common-lib/dist/*.css": true,
"./packages/esm-patient-common-lib/dist/*.map": false,
"./packages/esm-patient-common-lib/dist/*.txt": false,
"./packages/esm-patient-common-lib/dist/*.json": false
}@openmrs/esm-patient-conditions-app (+1.16%)
@openmrs/esm-patient-forms-app (+0.55%)
@openmrs/esm-patient-immunizations-app (+1.32%)
@openmrs/esm-patient-medications-app (+0.99%)… Generated by @jsenv/file-size-impact during Report bundle size#1988658563 on 31104df
|
donaldkibet
approved these changes
Mar 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
The PR contains major improvements towards how offline forms are handled. As part of this work, I had to make some (rather large) adjustments in various locations of
esm-form-entry-appin order to support the new offline form submission flow. Among these, I tried my best to improve the code and make it easier to work with (introduce better typings, extract certain tasks into services, removed unused code, ...).With that being said, I focused on not making any behavioral changes in the code whenever possible. The services (and the whole module, actually) should still do the same thing as before, except, of course, for the changes with regard to offline form handling.
Changes Regarding Offline Forms
Previously, offline forms were handled like this:
esm-form-entry-appinterceptedPOSTrequests submitted by the MF with the help of the service worker. This was functional for the requirements back then, but by now, we needed offline forms to support more complex flows (like editing offline forms from the offline actions).To support such tasks, I refactored
esm-form-entry-appso that it is aware of offline mode and handles offline synchronization items directly, i.e. without any network interception magic. This allows us, and other MFs, to better interact with the queued data. It should further make the entire process of handling offline forms much much much more stable because the way data is submitted during create/edit flows is now handled differently.With these changes in place, editing offline forms should now also be possible (though I will have to test this more extensively in the coming days, ideally with more forms). If there are follow-up issues, they can be addressed in another PR.
Other Changes / Refactorings
The task described above required some deep changes in
esm-form-entry-app. I had to touch a lot of files and took the chance to make some improvements (this is always subjective, I know) to the code:fe-wrapper.component.tsxby extracting functions into separate services.Screenshots
No UI changes.
Related Issue
https://issues.openmrs.org/browse/O3-1032
https://issues.openmrs.org/browse/O3-949 (foundational work for this issue)
Other
If this PR is merged the way it is, there are some follow-up tasks that should be done: