Skip to content

Conversation

@henrikmv
Copy link
Contributor

@henrikmv henrikmv commented Oct 23, 2025

DHIS2-20352

This pull request prevents runtime errors and improves logging for misconfigured indicators.

  • Added null checks for data elements and attributes in getDirectAddressedVariable to prevent undefined property access and throw an error for missing references.

  • Added validation in buildIndicatorRuleAndVariables to log an error and skip program indicators with missing references.

@henrikmv henrikmv requested a review from a team as a code owner October 23, 2025 15:26
@henrikmv henrikmv requested a review from Copilot October 23, 2025 21:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a runtime error in the Capture app when program indicators reference missing data elements or tracked entity attributes. The fix adds null safety checks and validation to prevent the app from failing to load due to misconfigured indicators.

Key Changes:

  • Added null checks in getDirectAddressedVariable to handle missing data elements and attributes
  • Added validation in buildIndicatorRuleAndVariables to detect and skip indicators with invalid references
  • Enhanced logging to report configuration errors for problematic indicators

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@henrikmv henrikmv marked this pull request as draft October 24, 2025 08:20
@henrikmv henrikmv marked this pull request as ready for review October 24, 2025 10:12
Copy link
Contributor

@simonadomnisoru simonadomnisoru 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
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.

So the false assumption in the original code is that programData will contain all data elements and attributes referenced in the PI expressions. The solution is to ignore any PI which violates this assumption.

I'm uncertain about the condition variables.length < expectedVariables. Looking closely at the edited file, I'm starting to think ìt might be easier and more accurate to throw an error in getDirectAddressedVariable if a DE or attribute is missing, and wrap the call to getVariables in a try-catch block.

@github-actions
Copy link

github-actions bot commented Oct 30, 2025

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

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.

Looks great!

@henrikmv henrikmv added testing and removed testing labels Nov 12, 2025
@henrikmv henrikmv merged commit e9fa7c2 into master Nov 12, 2025
43 checks passed
@henrikmv henrikmv deleted the hv/fix/DHIS2-20352_CaptureBreakWhenMissingAttributeForIndicator branch November 12, 2025 15:12
dhis2-bot added a commit that referenced this pull request Nov 12, 2025
## [104.6.5](v104.6.4...v104.6.5) (2025-11-12)

### Bug Fixes

* [DHIS2-20352] Capture app fails to load when DE or TEA referenced in indicator is missing ([#4391](#4391)) ([e9fa7c2](e9fa7c2))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 104.6.5 🎉

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