Skip to content
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

fix: [DHIS2-14938] trigger error on complete #3594

Merged
merged 3 commits into from
Apr 22, 2024
Merged

fix: [DHIS2-14938] trigger error on complete #3594

merged 3 commits into from
Apr 22, 2024

Conversation

simonadomnisoru
Copy link
Contributor

@simonadomnisoru simonadomnisoru commented Apr 3, 2024

DHIS2-14938

Tech summary:
The completeDataEntryFieldValue prop can take different values (i.e., undefined, null, 'true', 'false'). The values come from the redux store key dataEntriesFieldsValue which is initialized and handled differently by different forms. I fixed the edit event code to return the value of onIsCompleting a boolean value.

@simonadomnisoru simonadomnisoru requested a review from a team as a code owner April 3, 2024 09:44
@@ -235,7 +235,7 @@ const getSaveHandler = (
} = this.props;

const filteredProps = onFilterProps ? onFilterProps(passOnProps) : passOnProps;
this.isCompleting = !!(onIsCompleting && onIsCompleting(this.props));
this.isCompleting = Boolean(onIsCompleting && onIsCompleting(this.props) === 'true');
Copy link
Member

Choose a reason for hiding this comment

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

Can we change the edit event code instead of the generic withSaveHandler component? here:

onIsCompleting: (props: Object) => props.completeDataEntryFieldValue,

I think it makes sense to keep the return value of onIsCompleting a boolean value.

Comment on lines +166 to +174
filter((action) => {
const {
meta: { triggerAction },
} = action;
return (
triggerAction === enrollmentSiteActionTypes.COMMIT_ENROLLMENT_EVENT ||
triggerAction === enrollmentEditEventActionTypes.EVENT_SAVE_ENROLLMENT_COMPLETE_SUCCESS
);
}),
Copy link
Member

Choose a reason for hiding this comment

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

Is this really needed? Seems like we return EMPTY on line 183. Is it good enough to do one or the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my localhoast, the return EMPTY statement is causing an error and the page is no longer responding when editing a single event. I remove it and filter by triggerAction instead. This is not an issue in play so should I just rollback the change and leave it as it is? 🤔

Screenshot 2024-04-04 at 17 54 42

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the reason EMPTY crashes the app is that it counts as an observable and not an action; so it works as return value for switchMap but not for map. I think using filter is the best solution here. I approve the last commit 👍

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks. Current PR looks good to me.

Copy link
Member

@JoakimSM JoakimSM left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

github-actions bot commented Apr 9, 2024

Copy link

@geethaalwan geethaalwan left a comment

Choose a reason for hiding this comment

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

Tested successfully on 2.42,2.41.0,2.40.4,2.39.6,2.38.7 versions

@JoakimSM JoakimSM merged commit 005877c into master Apr 22, 2024
76 of 81 checks passed
@JoakimSM JoakimSM deleted the DHIS2-14938 branch April 22, 2024 14:21
dhis2-bot added a commit that referenced this pull request Apr 22, 2024
## [100.67.12](v100.67.11...v100.67.12) (2024-04-22)

### Bug Fixes

* [DHIS2-14938] trigger error on complete ([#3594](#3594)) ([005877c](005877c))
* [DHIS2-16999] filter assign rule effects in the view event page ([#3597](#3597)) ([f2007e3](f2007e3))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.67.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants