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.

Sign up for GitHub

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

Jump to bottom

fix: [DHIS2-15888] multi text element in program rules #3482

Merged
merged 4 commits into from
Jan 23, 2024
Merged

fix: [DHIS2-15888] multi text element in program rules #3482

merged 4 commits into from
Jan 23, 2024

Conversation

simonadomnisoru
Copy link
Contributor

@simonadomnisoru simonadomnisoru commented Dec 8, 2023 •
edited by jira bot
Loading

DHIS2-15888

Tech summary

  • Right now, it is not possible to check specific multi-text values. DHIS2-16211 and DHIS2-13436 must be implemented first.
  • I added logic to check that a multi-text field has a value, therefore rules like the one mentioned in the ticket description A{AllergiesMulti} != "" should work correctly.
  • The same rule actions that can be applied to an element with a single value option can also be applied to an element with multiple value options (Hide, Hide option, Show error, Show warning, Make filed mandatory, Assign a value, etc.).
  • To assign multiple values, the expression to be evaluated and assigned must contain valid options codes separated by , (for example, a valid expression for the rule mentioned in the ticket is 'Penicillin,Other,Anticonvulsants')
  • added some unit tests for the new logic

@simonadomnisoru simonadomnisoru requested a review from a team as a code owner December 8, 2023 11:50
superskip
superskip reviewed Jan 16, 2024
View reviewed changes
Copy link
Contributor

@superskip superskip left a comment

Choose a reason for hiding this comment

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

Made three comments 👀
I'm trusting the tests are alright 🙂

packages/rules-engine/src/helpers/OptionSetHelper.js Show resolved Hide resolved
src/core_modules/capture-core/rules/postProcessRulesEffects.js Outdated Show resolved Hide resolved
src/core_modules/capture-core/rules/postProcessRulesEffects.js Outdated Show resolved Hide resolved
@simonadomnisoru
Copy link
Contributor Author

Made three comments 👀

@superskip I updated the code with your suggestions. Can you have a look? Thanks!

superskip
superskip reviewed Jan 18, 2024
View reviewed changes
Copy link
Contributor

@superskip superskip left a comment

Choose a reason for hiding this comment

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

Just noticed one more minor naming thing that should be addressed before proceeding, but otherwise it looks ready.

src/core_modules/capture-core/rules/converters/outputConverter.js Show resolved Hide resolved
src/core_modules/capture-core/rules/postProcessRulesEffects.js Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

🚀 Deployed on https://deploy-preview-3482--dhis2-capture.netlify.app

@simonadomnisoru simonadomnisoru merged commit fd0776d into master Jan 23, 2024
37 checks passed
@simonadomnisoru simonadomnisoru deleted the DHIS2-15888 branch January 23, 2024 11:09
dhis2-bot added a commit that referenced this pull request Jan 23, 2024
@dhis2-bot
chore(release): cut 100.50.7 [skip release]
12a571a 
## [100.50.7](v100.50.6...v100.50.7) (2024-01-23)

### Bug Fixes

* [DHIS2-15888] multi text element in program rules ([#3482](#3482)) ([fd0776d](fd0776d))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.50.7 🎉

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
Reviewers

@superskip superskip superskip approved these changes

Assignees
No one assigned
Labels
released testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@simonadomnisoru @dhis2-bot @superskip