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-13346] assigned value #3087

Merged
merged 11 commits into from
Feb 6, 2023
Merged

fix: [DHIS2-13346] assigned value #3087

merged 11 commits into from
Feb 6, 2023

Conversation

jasminenguyennn
Copy link
Contributor

No description provided.

@jasminenguyennn jasminenguyennn marked this pull request as ready for review November 8, 2022 08:35
@jasminenguyennn jasminenguyennn requested review from a team as code owners November 8, 2022 08:35
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.

I had no success with this solution when I tested it (used a rule to set a compulsory field when another field was filled in). Nevertheless, looking into it, we should approach this a bit differently.

Currently we save the result of a field validation in the Redux store when you exit a field ("onblur" for most of the field types). If you use the "assign" action in the rules engine, we presume that you fill in a valid value and that value is not validated (The better approach here would have been to validate on render. This is a big change however, one we should carefully consider the implications of before doing it)

My suggestion is to still presume that any value you set using the "assign" action in the rules engine is valid. There is a missing implementation detail here however. Any validation errors that existed prior to the assignment is not cleared. So, we will have to reset the relevant parts of the formsSectionsFieldsUI in the Redux store for that field. You should be able to add this logic in the reducer.

The formsSectionsFieldsUI is grouped by sections, so we will have to look in multiple groups. The relevant properties are: valid, errorMessage, errorData and validatingMessage (I think)

Let me know if you have questions.

@github-actions
Copy link

github-actions bot commented Jan 24, 2023

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.

We can't use formBuilderId I believe. That is because the assignment can refer to a field in another section. We will have to look for the data element / tracked entity attribute in all sections that is starting with the formId.

Something like: Object.keys(state).filter(key => key.startsWith(formId))......

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.

Looks good 👍

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.40,2.39.2,2.38.3 versions

@jasminenguyennn jasminenguyennn merged commit 41e5d27 into master Feb 6, 2023
@jasminenguyennn jasminenguyennn deleted the DHIS2-13346 branch February 6, 2023 11:32
dhis2-bot added a commit that referenced this pull request Feb 6, 2023
## [100.25.1](v100.25.0...v100.25.1) (2023-02-06)

### Bug Fixes

* [DHIS2-13346] assigned value ([#3087](#3087)) ([41e5d27](41e5d27))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.25.1 🎉

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