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

feat: Filter events by existing data element [DHIS2-15954] #19760

Merged
merged 7 commits into from
Jan 28, 2025
Merged

Conversation

muilpp
Copy link
Contributor

@muilpp muilpp commented Jan 23, 2025

Part 1

Requesting events with a filter like filter={DE_UID} doesn't work as expected, events are returned regardless of the DEs they contain. This PR aims to fix this issue.

The DEs are stored in a map that uses the UID as the key and a List<QueryFilter> as the value. In this case, the value list is always empty. The issue is that this map is also used when sorting. Even though the DEs are present in the order list, we are still using the DE map to add fields to the select clause.

I am changing this in favor of the same approach we use when sorting TEAs, using the order list to add any DE to the select statement.

Next

This fix will allow users to filter for events containing a value in a given DE.
The next PR will enable filtering for events not containing a value in a given DE.

@muilpp muilpp marked this pull request as ready for review January 24, 2025 10:51
@muilpp muilpp requested a review from a team as a code owner January 24, 2025 10:51
EventQueryParams params = new EventQueryParams();

params.orderBy(de1, SortDirection.ASC);

assertEquals(List.of(new Order(de1, SortDirection.ASC)), params.getOrder());
assertEquals(Map.of(de1, List.of()), params.getDataElements());
assertTrue(params.getDataElements().isEmpty());
Copy link
Contributor

Choose a reason for hiding this comment

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

use assertIsEmpty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a Map, not a Collection, that's why I didn't use it
I didn't deem it necessary to create a helper method for this one

@muilpp muilpp merged commit 4d0f921 into master Jan 28, 2025
17 checks passed
@muilpp muilpp deleted the DHIS2-15954 branch January 28, 2025 11:43
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