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: IN filter now works with org units data elements [DHIS2-18845] #19727

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gnespolino
Copy link
Contributor

This pull request will fix a small bug with IN query operator, making it properly quotes values when needed (it was only quoting TEXT value types before).
Moreover, it will properly support Data Elements of type ORG_UNIT, by replacing the filter values with org unit UIDs when needed

Signed-off-by: Giuseppe Nespolino <[email protected]>
@gnespolino gnespolino enabled auto-merge (squash) January 20, 2025 13:27
@gnespolino gnespolino requested a review from vietnguyen January 20, 2025 13:28
Copy link
Contributor

@maikelarabori maikelarabori 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 @gnespolino , one minor comment.

Thx!

public List<String> getOrgUnitDimensionUid(
List<String> items, List<OrganisationUnit> userOrgUnits) {
return getOrgUnitDimensionItems(
items, userOrgUnits, IdScheme.UID, new ArrayList<>(), new ArrayList<>())
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use List.of(), instead of new ArrayList<>()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some reason, those lists need to be mutable. So cannot use List.of()

@maikelarabori maikelarabori added the run-api-analytics-tests Enables analytics e2e tests label Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-api-analytics-tests Enables analytics e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants