From 613859bbea749f5348228a62a6c95436effda3ff Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 23 Jan 2025 15:57:10 +0100 Subject: [PATCH 1/7] feat: Filter events by existing data element [DHIS2-15954] From 06269e33eb0eb1ec68de754da9752f7a991c630b Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 23 Jan 2025 16:02:12 +0100 Subject: [PATCH 2/7] feat: Filter events by existing data element [DHIS2-15954] --- .../tracker/export/event/JdbcEventStore.java | 5 +++++ .../export/event/EventExporterTest.java | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java index 465fa3497125..7fe0fed94255 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java @@ -1444,6 +1444,11 @@ private StringBuilder dataElementAndFiltersSql( } } } + } else { + eventDataValuesWhereSql.append(hlp.whereAnd()); + eventDataValuesWhereSql.append(" (ev.eventdatavalues ?? '"); + eventDataValuesWhereSql.append(item.getKey().getUid()); + eventDataValuesWhereSql.append("')"); } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java index 83b4f9226bca..fad784b2c6b0 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.time.ZoneId; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; @@ -590,6 +591,25 @@ void testExportEventsWhenFilteringByNumericDataElements() assertContainsOnly(List.of("D9PbzJY8bJM"), events); } + @Test + void shouldFilterByEventsWithGivenDataValuesWhenFilterContainsDataElementUIDsOnly() + throws ForbiddenException, BadRequestException { + EventOperationParams params = + EventOperationParams.builder() + .eventParams(EventParams.FALSE) + .dataElementFilters( + Map.of( + UID.of("GieVkTxp4HH"), + new ArrayList<>(), + UID.of("GieVkTxp4HG"), + new ArrayList<>())) + .build(); + + List events = getEvents(params); + + assertContainsOnly(List.of("kWjSezkXHVp"), events); + } + @Test void testEnrollmentEnrolledBeforeSetToBeforeFirstEnrolledAtDate() throws ForbiddenException, BadRequestException { From 1ddbce80d118010bb9232ffb7f7de48cfcb6342d Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 24 Jan 2025 10:17:41 +0100 Subject: [PATCH 3/7] feat: Avoid using dataElements map when sorting DEs [DHIS2-15954] --- .../org/hisp/dhis/tracker/export/event/EventQueryParams.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventQueryParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventQueryParams.java index 7102c9eb5941..e593757a00d7 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventQueryParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventQueryParams.java @@ -414,7 +414,6 @@ public EventQueryParams orderBy(String field, SortDirection direction) { /** Order by the given data element {@code de} in given sort {@code direction}. */ public EventQueryParams orderBy(DataElement de, SortDirection direction) { this.order.add(new Order(de, direction)); - this.dataElements.putIfAbsent(de, new ArrayList<>()); return this; } From f19e3be2624d7724fc39762963d22cfab96c5a24 Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 24 Jan 2025 10:19:34 +0100 Subject: [PATCH 4/7] feat: Add order fields in select clause from params order [DHIS2-15954] --- .../tracker/export/event/JdbcEventStore.java | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java index 7fe0fed94255..74ae62deb43d 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java @@ -893,16 +893,8 @@ private String getEventSelectQuery( .append(COLUMN_EVENT_ATTRIBUTE_OPTION_COMBO_NAME) .append(", coc_agg.attributevalues as ") .append(COLUMN_EVENT_ATTRIBUTE_OPTION_COMBO_ATTRIBUTE_VALUES) - .append(", coc_agg.co_values AS co_values, coc_agg.co_count AS option_size, "); - - for (Order order : params.getOrder()) { - if (order.getField() instanceof TrackedEntityAttribute tea) - selectBuilder - .append(quote(tea.getUid())) - .append(".value AS ") - .append(tea.getUid()) - .append("_value, "); - } + .append(", coc_agg.co_values AS co_values, coc_agg.co_count AS option_size, ") + .append(addOrderFieldsToSelectClause(params)); return selectBuilder .append( @@ -928,6 +920,32 @@ private String getEventSelectQuery( .toString(); } + private String addOrderFieldsToSelectClause(EventQueryParams params) { + StringBuilder selectBuilder = new StringBuilder(); + + for (Order order : params.getOrder()) { + if (order.getField() instanceof TrackedEntityAttribute tea) { + selectBuilder + .append(quote(tea.getUid())) + .append(".value AS ") + .append(tea.getUid()) + .append("_value, "); + } else if (order.getField() instanceof DataElement de) { + final String dataValueValueSql = "ev.eventdatavalues #>> '{" + de.getUid() + ", value}'"; + selectBuilder + .append( + de.getValueType().isNumeric() + ? castToNumber(dataValueValueSql) + : lower(dataValueValueSql)) + .append(" as ") + .append(de.getUid()) + .append(", "); + } + } + + return selectBuilder.toString(); + } + private boolean checkForOwnership(EventQueryParams params) { return Optional.ofNullable(params.getProgram()) .filter( @@ -1447,7 +1465,7 @@ private StringBuilder dataElementAndFiltersSql( } else { eventDataValuesWhereSql.append(hlp.whereAnd()); eventDataValuesWhereSql.append(" (ev.eventdatavalues ?? '"); - eventDataValuesWhereSql.append(item.getKey().getUid()); + eventDataValuesWhereSql.append(deUid); eventDataValuesWhereSql.append("')"); } } From 2315c98d7aea3291a9a816e886a3a18a9a9e5f2b Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 24 Jan 2025 10:22:53 +0100 Subject: [PATCH 5/7] feat: Replace params for order list in method signature [DHIS2-15954] --- .../org/hisp/dhis/tracker/export/event/JdbcEventStore.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java index 74ae62deb43d..64dd9e941040 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java @@ -894,7 +894,7 @@ private String getEventSelectQuery( .append(", coc_agg.attributevalues as ") .append(COLUMN_EVENT_ATTRIBUTE_OPTION_COMBO_ATTRIBUTE_VALUES) .append(", coc_agg.co_values AS co_values, coc_agg.co_count AS option_size, ") - .append(addOrderFieldsToSelectClause(params)); + .append(addOrderFieldsToSelectClause(params.getOrder())); return selectBuilder .append( @@ -920,10 +920,10 @@ private String getEventSelectQuery( .toString(); } - private String addOrderFieldsToSelectClause(EventQueryParams params) { + private String addOrderFieldsToSelectClause(List orders) { StringBuilder selectBuilder = new StringBuilder(); - for (Order order : params.getOrder()) { + for (Order order : orders) { if (order.getField() instanceof TrackedEntityAttribute tea) { selectBuilder .append(quote(tea.getUid())) From edb875016ebf18e2bbfa6ceda75ae6ca4fff7b8c Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 24 Jan 2025 11:11:44 +0100 Subject: [PATCH 6/7] feat: Fix data element tests [DHIS2-15954] --- .../export/event/DefaultEventService.java | 7 ++++++- .../export/event/EventQueryParamsTest.java | 4 ++-- .../event/EventsExportControllerTest.java | 20 ++++++++++++++++++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventService.java index c20dad793c3d..1be03e3a7f7a 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/DefaultEventService.java @@ -128,7 +128,12 @@ private FileResource getFileResourceMetadata(UID eventUid, UID dataElementUid) "this must be a bug in how the EventOperationParams are built"); } if (events.getItems().isEmpty()) { - throw new NotFoundException(Event.class, eventUid); + throw new NotFoundException( + "Event " + + eventUid.getValue() + + " with data element " + + dataElementUid.getValue() + + " could not be found."); } Event event = events.getItems().get(0); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventQueryParamsTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventQueryParamsTest.java index 786c812455d7..3b26fc050fb6 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventQueryParamsTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventQueryParamsTest.java @@ -70,13 +70,13 @@ void shouldKeepExistingAttributeFiltersWhenOrderingByAttribute() { } @Test - void shouldAddDataElementToOrderAndDataElementsWhenOrderingByDataElement() { + void shouldAddDataElementToOrderButNotToDataElementsWhenOrderingByDataElement() { 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()); assertFalse(params.hasDataElementFilter()); } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportControllerTest.java index 0daf050f0cfe..72b264a0d3c9 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventsExportControllerTest.java @@ -534,7 +534,6 @@ void getDataValuesFileByDataElementIfUserDoesNotHaveReadAccessToDataElement() event.getEventDataValues().add(dataValue(de, file.getUid())); manager.update(event); - // this.switchContextToUser(user); switchContextToUser(user); GET("/tracker/events/{eventUid}/dataValues/{dataElementUid}/file", event.getUid(), de.getUid()) @@ -568,6 +567,25 @@ void getDataValuesFileByDataElementIfNoDataValueExists() { switchContextToUser(user); + assertEquals( + "Event " + event.getUid() + " with data element " + de.getUid() + " could not be found.", + GET( + "/tracker/events/{eventUid}/dataValues/{dataElementUid}/file", + event.getUid(), + de.getUid()) + .error(HttpStatus.NOT_FOUND) + .getMessage()); + } + + @Test + void getDataValuesFileByDataElementIfNoDataValueNull() { + Event event = event(enrollment(trackedEntity())); + DataElement de = dataElement(ValueType.IMAGE); + + event.getEventDataValues().add(dataValue(de, null)); + manager.flush(); + switchContextToUser(user); + assertEquals( "DataValue for data element " + de.getUid() + " could not be found.", GET( From 47fec18299d9e8a759fdb5ca36f27acc23d7aa10 Mon Sep 17 00:00:00 2001 From: Marc Date: Tue, 28 Jan 2025 12:17:45 +0100 Subject: [PATCH 7/7] feat: Address PR comments [DHIS2-15954] --- .../hisp/dhis/tracker/export/event/EventOperationParams.java | 2 +- .../org/hisp/dhis/tracker/export/event/JdbcEventStore.java | 4 ++-- .../org/hisp/dhis/tracker/export/event/EventExporterTest.java | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java index 842e15ac1393..44bea57ebb47 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java @@ -142,7 +142,7 @@ public class EventOperationParams { private Set enrollments; - private EventParams eventParams; + @Builder.Default private EventParams eventParams = EventParams.FALSE; @Builder.Default private TrackerIdSchemeParams idSchemeParams = TrackerIdSchemeParams.builder().build(); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java index 64dd9e941040..9ea820d6af15 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java @@ -894,7 +894,7 @@ private String getEventSelectQuery( .append(", coc_agg.attributevalues as ") .append(COLUMN_EVENT_ATTRIBUTE_OPTION_COMBO_ATTRIBUTE_VALUES) .append(", coc_agg.co_values AS co_values, coc_agg.co_count AS option_size, ") - .append(addOrderFieldsToSelectClause(params.getOrder())); + .append(getOrderFieldsForSelectClause(params.getOrder())); return selectBuilder .append( @@ -920,7 +920,7 @@ private String getEventSelectQuery( .toString(); } - private String addOrderFieldsToSelectClause(List orders) { + private String getOrderFieldsForSelectClause(List orders) { StringBuilder selectBuilder = new StringBuilder(); for (Order order : orders) { diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java index fad784b2c6b0..b5a139a3dc51 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java @@ -596,7 +596,6 @@ void shouldFilterByEventsWithGivenDataValuesWhenFilterContainsDataElementUIDsOnl throws ForbiddenException, BadRequestException { EventOperationParams params = EventOperationParams.builder() - .eventParams(EventParams.FALSE) .dataElementFilters( Map.of( UID.of("GieVkTxp4HH"),