Skip to content

fix: Create program rule model [DHIS2-17243] #17807

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

Merged
merged 23 commits into from
Jul 1, 2024

Conversation

enricocolasante
Copy link
Contributor

@enricocolasante enricocolasante commented Jun 17, 2024

The goal of this PR is to remove the dependency of rule-engine library from dhis-service-tracker module.
To achieve that, we are creating an API layer in dhis-program-rule module and making tracker use that model instead of rule-engine one.
This approach has 2 advantages:

  • we decouple rule-engine implementation from tracker
  • we can return the result from rule-engine in a shape that is more convenient for tracker to handle

import org.hisp.dhis.tracker.imports.TrackerImportStrategy;
import org.junit.jupiter.api.Test;

/**
* @author Zubair Asghar
*/
class TrackerSideEffectDataBundleTest {
class TrackerSideValidationEffectDataBundleTest {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: TrackerSideValidationEffectDataBundleTest is not referenced within this codebase. If not used as an external API it should be removed.
@enricocolasante enricocolasante force-pushed the DHIS2-17243-model-change branch from 5fe80dc to 3ee8b96 Compare June 26, 2024 07:14
@enricocolasante enricocolasante marked this pull request as ready for review June 26, 2024 07:47
Copy link
Contributor

@teleivo teleivo left a comment

Choose a reason for hiding this comment

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

Is anyone other than tracker going to use the program-rule/api? If not should this code not live in tracker? It could still sit as a layer in between any tracker code interacting with program rules/rule engine.


// These are needed for rule engine validation
RuleEngineEffects ruleEffects =
RuleEngineEffects.merge(
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to merge them if we pull them apart again right after in

bundle.setEventNotificationEffects(ruleEffects.getEventNotificationEffects());

Maybe you can walk us through how the RuleEffects/RuleEngineEffects enter into tracker. I wonder if we can reduce some of the mapping/merging.

@enricocolasante enricocolasante requested a review from teleivo June 27, 2024 13:14
Copy link

sonarqubecloud bot commented Jul 1, 2024

@enricocolasante enricocolasante merged commit 1f2ee1e into master Jul 1, 2024
15 checks passed
@enricocolasante enricocolasante deleted the DHIS2-17243-model-change branch July 1, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants