(fix) O3-4271 fix callback registration in visit form #2151
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
3 tasks
|
Size Change: +54 B (0%) Total Size: 16 MB ℹ️ View Unchanged
|
chibongho
requested review from
ibacher,
mseaton,
mogoodrich,
ojwanganto,
denniskigen and
donaldkibet
Have you tried |
ibacher
approved these changes
Dec 12, 2024
| patientUuid, | ||
| setVisitFormCallbacks: (callbacks) => { | ||
| setVisitFormCallbacks((old) => { | ||
| return new Map(old).set(extension.id, callbacks); |
There was a problem hiding this comment.
Do we need a new Map on every call? It might simplify things if it's possible to do an update-in-place.
There was a problem hiding this comment.
I think we do, if we keep using useState, as the Map should be immutable. (This is akin of editing / adding an element to an array state). That said, I think we should move to useRef.
That's a good idea. Filed O3-4274 to track this. I'll commit this in the meantime as it's a blocker. |
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
See also corresponding PR in patient-management.
This is a regression from O3-4200. In the service queues app, the form for creating a new queue entry for patient with existing visit is broken. The actual fix is in corresponding PR in patient management; this PR makes necessary change on how the visit form registers
onVisitCreatedOrUpdatedcallbacks.Root cause:
[state, setState = useState<Function>()is generally a bad idea, becausesetStateis overloaded to take in either 1) the new state or 2) a lambda that returns the new state. When doingsetState(callback), JavaScript has no way to distinguish between the two, and erroneously treats it as case 2. The visit form, which storesonVisitCreatedOrUpdatedcallbacks in aMap<string, Callbacks>, doesn't actually have this problem, but the form for creating a new queue entry for patient with existing visit does.This PR addresses this by wrapping the callback states in an object.
Unrelated, the visit form has a non-functional / performance issue with infinite state updates when the extensions in
VisitFormExtensionSlotregister callbacks. (Generally, the pattern of a child component updating the parent's state is dangerous if the state is an object.) This PR address that with a combination ofuseCallbacks,useMemoandmemoin various places. (It feels like it should be possible to reduce the use of these memoing functions, but I haven't been able to simplify it.)Screenshots
On submission on start visit form to: start a visit, update visit attributes, check-in appointment, create queue entry.
On submission of form to create queue entry for patient with existing visit:
Related Issue
https://openmrs.atlassian.net/browse/O3-4271
Other