Skip to content
This repository has been archived by the owner on Jan 4, 2023. It is now read-only.

[FhirResourceTree] Reset tree state on unmount #661

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

BPierrick
Copy link
Contributor

Fixes

Definition of Done

  • I followed the Arkhn Code Book (I swear!).
  • I have written tests for the code I added or updated. -> NA
  • I have updated the documentation according to my changes.
  • I have updated the deployment configuration if needed. -> NA

@BPierrick BPierrick added type/bug Something isn't working proj/new-pyrog labels Oct 7, 2021
@BPierrick BPierrick requested a review from a team October 7, 2021 09:50
@BPierrick BPierrick linked an issue Oct 7, 2021 that may be closed by this pull request
Copy link
Contributor

@simonvadee simonvadee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this deserves a test ? shoudn't all side effects be tested ?

Comment on lines +248 to +254
useEffect(() => {
return () => {
if (!node) {
dispatch(resourceTreeSliceStateReseted());
}
};
}, [dispatch, node]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
useEffect(() => {
return () => {
if (!node) {
dispatch(resourceTreeSliceStateReseted());
}
};
}, [dispatch, node]);
useEffect(() => () => !node && dispatch(resourceTreeSliceStateReseted()),
[dispatch, node]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to test whether this reset action has been called once when hook is unmounted

@BPierrick BPierrick force-pushed the pb/fix_empty_complex_nodes branch from 259777a to 6c9131a Compare October 11, 2021 09:58
@BPierrick BPierrick requested review from simonvadee and a team October 11, 2021 09:59
Copy link
Contributor

@simonvadee simonvadee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

server.listen({ onUnhandledRequest: "error" });
});

describe("useFhirResourceTreeData", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! looks simple

@BPierrick BPierrick force-pushed the pb/fix_empty_complex_nodes branch from 6c9131a to cac3752 Compare October 14, 2021 10:11
@BPierrick BPierrick merged commit d20de5e into v4 Oct 14, 2021
@BPierrick BPierrick deleted the pb/fix_empty_complex_nodes branch October 14, 2021 10:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
proj/new-pyrog type/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FhirResourceTree] Root Complex nodes get empty when going back
2 participants