From 9b2ce342812f77de1f997bd01fc89c6da8dc8b08 Mon Sep 17 00:00:00 2001 From: marc Date: Tue, 3 Dec 2024 14:05:00 +0100 Subject: [PATCH] feat: Sort & Filter attribute change logs [DHIS2-18475] (#19331) * feat: Migrate TEA change logs into new table [DHIS2-18474] * feat: Fix sonar issues [DHIS2-18474] * feat: Import one object in test[DHIS2-18474] * feat: Lowercase sql script [DHIS2-18474] * feat: Address PR comments [DHIS2-18474] * feat: Filter attribute change logs [DHIS2-18475] * feat: Filter attribute change logs [DHIS2-18475] * feat: Sort by date, user and attribute in change logs [DHIS2-18475] * feat: Add comment on filter builder method [DHIS2-18475] * feat: Add comment on filter builder method [DHIS2-18475] * feat: Address PR comment [DHIS2-18475] --- .../DefaultTrackedEntityChangeLogService.java | 6 + .../HibernateTrackedEntityChangeLogStore.java | 36 +- ...TrackedEntityChangeLogOperationParams.java | 16 + .../TrackedEntityChangeLogService.java | 10 + .../resources/tracker/simple_metadata.json | 15 + ...erAndFilterTrackedEntityChangeLogTest.java | 320 ++++++++++++++++++ .../TrackedEntityChangeLogServiceTest.java | 88 ++--- ...ckedEntitiesChangeLogsControllerTest.java} | 54 ++- .../export/RequestParamsValidator.java | 2 +- .../ChangeLogRequestParamsMapper.java | 18 +- .../TrackedEntitiesExportController.java | 4 +- 11 files changed, 509 insertions(+), 60 deletions(-) create mode 100644 dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/OrderAndFilterTrackedEntityChangeLogTest.java rename dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/{TrackedEntitiesExportControllerPostgresTest.java => TrackedEntitiesChangeLogsControllerTest.java} (88%) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java index 0fcf4a39bd42..b8e62d53d466 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityChangeLogService.java @@ -35,6 +35,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import lombok.RequiredArgsConstructor; +import org.apache.commons.lang3.tuple.Pair; import org.hisp.dhis.changelog.ChangeLogType; import org.hisp.dhis.common.IdentifiableObject; import org.hisp.dhis.common.UID; @@ -173,6 +174,11 @@ public Set getOrderableFields() { return hibernateTrackedEntityChangeLogStore.getOrderableFields(); } + @Override + public Set>> getFilterableFields() { + return hibernateTrackedEntityChangeLogStore.getFilterableFields(); + } + private Program validateProgram(String programUid) throws NotFoundException { Program program = programService.getProgram(programUid); if (program == null) { diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityChangeLogStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityChangeLogStore.java index bfdb2ababa65..d1ed1bc01b93 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityChangeLogStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityChangeLogStore.java @@ -34,12 +34,15 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.apache.commons.lang3.tuple.Pair; import org.hibernate.Session; import org.hisp.dhis.changelog.ChangeLogType; +import org.hisp.dhis.common.QueryFilter; import org.hisp.dhis.common.SortDirection; import org.hisp.dhis.common.UID; import org.hisp.dhis.program.UserInfoSnapshot; @@ -53,11 +56,22 @@ @Repository("org.hisp.dhis.tracker.export.trackedentity.HibernateTrackedEntityChangeLogStore") public class HibernateTrackedEntityChangeLogStore { private static final String COLUMN_CHANGELOG_CREATED = "tecl.created"; + private static final String COLUMN_CHANGELOG_USER = "tecl.createdByUsername"; + private static final String COLUMN_CHANGELOG_DATA_ELEMENT = "tea.uid"; + private static final String DEFAULT_ORDER = COLUMN_CHANGELOG_CREATED + " " + SortDirection.DESC.getValue(); private static final Map ORDERABLE_FIELDS = - Map.ofEntries(entry("createdAt", COLUMN_CHANGELOG_CREATED)); + Map.ofEntries( + entry("createdAt", COLUMN_CHANGELOG_CREATED), + entry("username", COLUMN_CHANGELOG_USER), + entry("attribute", COLUMN_CHANGELOG_DATA_ELEMENT)); + + private static final Map>, String> FILTERABLE_FIELDS = + Map.ofEntries( + entry(Pair.of("username", String.class), COLUMN_CHANGELOG_USER), + entry(Pair.of("attribute", UID.class), COLUMN_CHANGELOG_DATA_ELEMENT)); private final EntityManager entityManager; private final Session session; @@ -119,6 +133,18 @@ and tea.uid in (:attributes) """; } + Pair filter = operationParams.getFilter(); + if (filter != null) { + String filterField = + FILTERABLE_FIELDS.entrySet().stream() + .filter(entry -> entry.getKey().getLeft().equals(filter.getKey())) + .findFirst() + .map(Entry::getValue) + .get(); + + hql += String.format(" and %s = :filterValue ", filterField); + } + hql += String.format("order by %s".formatted(sortExpressions(operationParams.getOrder()))); Query query = entityManager.createQuery(hql); @@ -136,6 +162,10 @@ and tea.uid in (:attributes) query.setFirstResult((pageParams.getPage() - 1) * pageParams.getPageSize()); query.setMaxResults(pageParams.getPageSize() + 1); + if (filter != null) { + query.setParameter("filterValue", filter.getValue().getFilter()); + } + List results = query.getResultList(); List trackedEntityChangeLogs = results.stream() @@ -188,6 +218,10 @@ public Set getOrderableFields() { return ORDERABLE_FIELDS.keySet(); } + public Set>> getFilterableFields() { + return FILTERABLE_FIELDS.keySet(); + } + private static String sortExpressions(Order order) { if (order == null) { return DEFAULT_ORDER; diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogOperationParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogOperationParams.java index 01b7c55ace38..8530db3fc261 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogOperationParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogOperationParams.java @@ -31,6 +31,8 @@ import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Getter; +import org.apache.commons.lang3.tuple.Pair; +import org.hisp.dhis.common.QueryFilter; import org.hisp.dhis.common.SortDirection; import org.hisp.dhis.tracker.export.Order; @@ -39,6 +41,7 @@ @AllArgsConstructor(access = AccessLevel.PRIVATE) public class TrackedEntityChangeLogOperationParams { private Order order; + private Pair filter; public static class TrackedEntityChangeLogOperationParamsBuilder { @@ -50,10 +53,23 @@ private TrackedEntityChangeLogOperationParamsBuilder order(Order order) { return this; } + // Do not remove this unused method. This hides the filter field from the builder which Lombok + // does not support. The repeated filter field and private filter method prevent access to + // filter via the builder. + // Filter should be added via the filterBy builder methods. + private TrackedEntityChangeLogOperationParamsBuilder filter(Pair filter) { + return this; + } + public TrackedEntityChangeLogOperationParamsBuilder orderBy( String field, SortDirection direction) { this.order = new Order(field, direction); return this; } + + public TrackedEntityChangeLogOperationParamsBuilder filterBy(String field, QueryFilter filter) { + this.filter = Pair.of(field, filter); + return this; + } } } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogService.java index c8f6b16fd899..cbf9a243a181 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogService.java @@ -31,6 +31,7 @@ import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.apache.commons.lang3.tuple.Pair; import org.hisp.dhis.changelog.ChangeLogType; import org.hisp.dhis.common.UID; import org.hisp.dhis.feedback.BadRequestException; @@ -86,4 +87,13 @@ Page getTrackedEntityChangeLog( * PageParams)}. */ Set getOrderableFields(); + + /** + * Fields the {@link #getTrackedEntityChangeLog(UID, UID, TrackedEntityChangeLogOperationParams, + * PageParams)} can filter attribute change logs by. Filtering by fields other than these, is + * considered a programmer error. Validation of user provided field names should occur before + * calling {@link #getTrackedEntityChangeLog(UID, UID, TrackedEntityChangeLogOperationParams, + * PageParams)}. + */ + Set>> getFilterableFields(); } diff --git a/dhis-2/dhis-support/dhis-support-test/src/main/resources/tracker/simple_metadata.json b/dhis-2/dhis-support/dhis-support-test/src/main/resources/tracker/simple_metadata.json index 9b19ddb9f268..ef773fcfb09e 100644 --- a/dhis-2/dhis-support/dhis-support-test/src/main/resources/tracker/simple_metadata.json +++ b/dhis-2/dhis-support/dhis-support-test/src/main/resources/tracker/simple_metadata.json @@ -2000,6 +2000,21 @@ "id": "ja8NY4PW7Xm" }, "valueType": "TEXT" + }, + { + "displayInList": true, + "displayName": "Person Given name", + "displayShortName": "null Given name", + "id": "EIVt4l5vIOa", + "name": "Person Given name", + "searchable": true, + "trackedEntityAttribute": { + "id": "toUpdate000" + }, + "trackedEntityType": { + "id": "ja8NY4PW7Xm" + }, + "valueType": "TEXT" } ], "user": { diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/OrderAndFilterTrackedEntityChangeLogTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/OrderAndFilterTrackedEntityChangeLogTest.java new file mode 100644 index 000000000000..727f9415ae10 --- /dev/null +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/OrderAndFilterTrackedEntityChangeLogTest.java @@ -0,0 +1,320 @@ +/* + * Copyright (c) 2004-2024, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * Neither the name of the HISP project nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.hisp.dhis.tracker.export.trackedentity; + +import static org.hisp.dhis.test.utils.Assertions.assertContainsOnly; +import static org.hisp.dhis.tracker.Assertions.assertNoErrors; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.io.IOException; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.hisp.dhis.common.QueryFilter; +import org.hisp.dhis.common.QueryOperator; +import org.hisp.dhis.common.SortDirection; +import org.hisp.dhis.common.UID; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.ForbiddenException; +import org.hisp.dhis.feedback.NotFoundException; +import org.hisp.dhis.tracker.TrackerTest; +import org.hisp.dhis.tracker.export.Page; +import org.hisp.dhis.tracker.export.PageParams; +import org.hisp.dhis.tracker.imports.TrackerImportParams; +import org.hisp.dhis.tracker.imports.TrackerImportService; +import org.hisp.dhis.tracker.imports.domain.TrackerObjects; +import org.hisp.dhis.user.User; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.factory.annotation.Autowired; + +class OrderAndFilterTrackedEntityChangeLogTest extends TrackerTest { + @Autowired private TrackedEntityChangeLogService trackedEntityChangeLogService; + + @Autowired private TrackerImportService trackerImportService; + + private User importUser; + + private TrackerImportParams importParams; + + private final PageParams defaultPageParams = new PageParams(null, null, false); + + private TrackerObjects trackerObjects; + + @BeforeAll + void setUp() throws IOException { + injectSecurityContextUser(getAdminUser()); + setUpMetadata("tracker/simple_metadata.json"); + + importUser = userService.getUser("tTgjgobT1oS"); + injectSecurityContextUser(importUser); + + importParams = TrackerImportParams.builder().build(); + trackerObjects = fromJson("tracker/event_and_enrollment.json"); + + assertNoErrors(trackerImportService.importTracker(importParams, trackerObjects)); + } + + @BeforeEach + void resetSecurityContext() { + injectSecurityContextUser(importUser); + } + + private static Stream provideDateAndUsernameOrderParams() { + return Stream.of( + Arguments.of("createdAt", SortDirection.DESC), + Arguments.of("username", SortDirection.DESC), + Arguments.of("username", SortDirection.ASC)); + } + + @ParameterizedTest + @MethodSource("provideDateAndUsernameOrderParams") + void shouldSortChangeLogsByCreatedAtDescWhenOrderingByDateOrUsername( + String field, SortDirection sortDirection) + throws ForbiddenException, NotFoundException, BadRequestException { + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder().orderBy(field, sortDirection).build(); + String trackedEntity = "QS6w44flWAf"; + String trackedEntityAttribute = "numericAttr"; + String updatedValue = "100"; + + updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); + + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, params, defaultPageParams), + trackedEntityAttribute); + + assertNumberOfChanges(2, changeLogs); + + assertAll( + () -> assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.get(0)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(1))); + } + + @Test + void shouldSortChangeLogsWhenOrderingByCreatedAtAsc() + throws ForbiddenException, NotFoundException, BadRequestException { + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder() + .orderBy("createdAt", SortDirection.ASC) + .build(); + String trackedEntity = "QS6w44flWAf"; + String trackedEntityAttribute = "numericAttr"; + String updatedValue = "100"; + + updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); + + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, params, defaultPageParams), + trackedEntityAttribute); + + assertNumberOfChanges(2, changeLogs); + + assertAll( + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(0)), + () -> assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.get(1))); + } + + @Test + void shouldSortChangeLogsWhenOrderingByAttributeAsc() + throws ForbiddenException, NotFoundException, BadRequestException { + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder() + .orderBy("attribute", SortDirection.ASC) + .build(); + String trackedEntity = "QS6w44flWAf"; + String trackedEntityAttribute = "numericAttr"; + String updatedValue = "100"; + + updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); + + Page changeLogs = + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, params, defaultPageParams); + + assertNumberOfChanges(3, changeLogs.getItems()); + + assertAll( + () -> + assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.getItems().get(0)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(1)), + () -> assertCreate("toUpdate000", "summer day", changeLogs.getItems().get(2))); + } + + @Test + void shouldSortChangeLogsWhenOrderingByAttributeDesc() + throws ForbiddenException, NotFoundException, BadRequestException { + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder() + .orderBy("attribute", SortDirection.DESC) + .build(); + String trackedEntity = "QS6w44flWAf"; + String trackedEntityAttribute = "numericAttr"; + String updatedValue = "100"; + + updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); + + Page changeLogs = + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, params, defaultPageParams); + + assertNumberOfChanges(3, changeLogs.getItems()); + + assertAll( + () -> assertCreate("toUpdate000", "summer day", changeLogs.getItems().get(0)), + () -> + assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.getItems().get(1)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(2))); + } + + @Test + void shouldFilterChangeLogsWhenFilteringByUser() + throws ForbiddenException, NotFoundException, BadRequestException { + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder() + .filterBy("username", new QueryFilter(QueryOperator.EQ, importUser.getUsername())) + .build(); + + Page changeLogs = + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of("QS6w44flWAf"), null, params, defaultPageParams); + + Set changeLogUsers = + changeLogs.getItems().stream() + .map(cl -> cl.getCreatedBy().getUsername()) + .collect(Collectors.toSet()); + assertContainsOnly(List.of(importUser.getUsername()), changeLogUsers); + } + + @Test + void shouldFilterChangeLogsWhenFilteringByAttribute() + throws ForbiddenException, NotFoundException, BadRequestException { + String trackedEntityAttribute = "toUpdate000"; + TrackedEntityChangeLogOperationParams params = + TrackedEntityChangeLogOperationParams.builder() + .filterBy("attribute", new QueryFilter(QueryOperator.EQ, trackedEntityAttribute)) + .build(); + + Page changeLogs = + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of("dUE514NMOlo"), null, params, defaultPageParams); + + Set changeLogAttributes = + changeLogs.getItems().stream() + .map(cl -> cl.getTrackedEntityAttribute().getUid()) + .collect(Collectors.toSet()); + assertContainsOnly(List.of(trackedEntityAttribute), changeLogAttributes); + } + + private void updateAttributeValue( + String trackedEntity, String trackedEntityAttribute, String newValue) { + trackerObjects.getTrackedEntities().stream() + .filter(t -> t.getTrackedEntity().getValue().equalsIgnoreCase(trackedEntity)) + .findFirst() + .ifPresent( + t -> { + t.getAttributes().stream() + .filter( + tea -> + tea.getAttribute() + .getIdentifier() + .equalsIgnoreCase(trackedEntityAttribute)) + .findFirst() + .ifPresent(attribute -> attribute.setValue(newValue)); + + assertNoErrors( + trackerImportService.importTracker( + importParams, TrackerObjects.builder().trackedEntities(List.of(t)).build())); + }); + } + + private static void assertNumberOfChanges(int expected, List changeLogs) { + assertNotNull(changeLogs); + assertEquals( + expected, + changeLogs.size(), + String.format( + "Expected to find %s attributes in the tracked entity change log list, found %s instead: %s", + expected, changeLogs.size(), changeLogs)); + } + + private void assertCreate( + String trackedEntityAttribute, String currentValue, TrackedEntityChangeLog changeLog) { + assertAll( + () -> assertUser(importUser, changeLog), + () -> assertEquals("CREATE", changeLog.getChangeLogType().name()), + () -> assertChange(trackedEntityAttribute, null, currentValue, changeLog)); + } + + private void assertUpdate( + String trackedEntityAttribute, + String previousValue, + String currentValue, + TrackedEntityChangeLog changeLog) { + assertAll( + () -> assertUser(importUser, changeLog), + () -> assertEquals("UPDATE", changeLog.getChangeLogType().name()), + () -> assertChange(trackedEntityAttribute, previousValue, currentValue, changeLog)); + } + + private void assertChange( + String trackedEntityAttribute, + String previousValue, + String currentValue, + TrackedEntityChangeLog changeLog) { + assertEquals(trackedEntityAttribute, changeLog.getTrackedEntityAttribute().getUid()); + assertEquals(previousValue, changeLog.getPreviousValue()); + assertEquals(currentValue, changeLog.getCurrentValue()); + } + + private void assertUser(User user, TrackedEntityChangeLog changeLog) { + assertAll( + () -> assertEquals(user.getUsername(), changeLog.getCreatedBy().getUsername()), + () -> assertEquals(user.getFirstName(), changeLog.getCreatedBy().getFirstName()), + () -> assertEquals(user.getSurname(), changeLog.getCreatedBy().getSurname()), + () -> assertEquals(user.getUid(), changeLog.getCreatedBy().getUid())); + } + + private static List filterTrackedEntityAttribute( + Page changeLogs, String attribute) { + return changeLogs.getItems().stream() + .filter(cl -> cl.getTrackedEntityAttribute().getUid().equalsIgnoreCase(attribute)) + .toList(); + } +} diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogServiceTest.java index 0fb9bac9d533..68e6b444beab 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityChangeLogServiceTest.java @@ -162,12 +162,15 @@ void shouldFailWhenUserHasNoAccessToOrgUnitScope() { @Test void shouldReturnChangeLogsWhenTrackedEntityAttributeValueIsCreated() throws NotFoundException, ForbiddenException, BadRequestException { - Page changeLogs = - trackedEntityChangeLogService.getTrackedEntityChangeLog( - UID.of("QS6w44flWAf"), null, defaultOperationParams, defaultPageParams); - - assertNumberOfChanges(1, changeLogs.getItems()); - assertAll(() -> assertCreate("numericAttr", "88", changeLogs.getItems().get(0))); + String trackedEntityAttribute = "numericAttr"; + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of("QS6w44flWAf"), null, defaultOperationParams, defaultPageParams), + trackedEntityAttribute); + + assertNumberOfChanges(1, changeLogs); + assertAll(() -> assertCreate("numericAttr", "88", changeLogs.get(0))); } @Test @@ -177,14 +180,16 @@ void shouldReturnChangeLogsWhenTrackedEntityAttributeValueIsDeleted() String trackedEntityAttribute = "numericAttr"; updateAttributeValue(trackedEntity, trackedEntityAttribute, ""); - Page changeLogs = - trackedEntityChangeLogService.getTrackedEntityChangeLog( - UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams); + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams), + trackedEntityAttribute); - assertNumberOfChanges(2, changeLogs.getItems()); + assertNumberOfChanges(2, changeLogs); assertAll( - () -> assertDelete(trackedEntityAttribute, "88", changeLogs.getItems().get(0)), - () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(1))); + () -> assertDelete(trackedEntityAttribute, "88", changeLogs.get(0)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(1))); } @Test @@ -195,15 +200,16 @@ void shouldReturnChangeLogsWhenTrackedEntityAttributeValueIsUpdated() String updatedValue = "100"; updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); - Page changeLogs = - trackedEntityChangeLogService.getTrackedEntityChangeLog( - UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams); + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams), + trackedEntityAttribute); - assertNumberOfChanges(2, changeLogs.getItems()); + assertNumberOfChanges(2, changeLogs); assertAll( - () -> - assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.getItems().get(0)), - () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(1))); + () -> assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.get(0)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(1))); } @Test @@ -216,21 +222,19 @@ void shouldReturnChangeLogsWhenTrackedEntityAttributeValueIsUpdatedTwiceInARow() updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); updateAttributeValue(trackedEntity, trackedEntityAttribute, secondUpdatedValue); - Page changeLogs = - trackedEntityChangeLogService.getTrackedEntityChangeLog( - UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams); + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams), + trackedEntityAttribute); - assertNumberOfChanges(3, changeLogs.getItems()); + assertNumberOfChanges(3, changeLogs); assertAll( () -> assertUpdate( - trackedEntityAttribute, - updatedValue, - secondUpdatedValue, - changeLogs.getItems().get(0)), - () -> - assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.getItems().get(1)), - () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(2))); + trackedEntityAttribute, updatedValue, secondUpdatedValue, changeLogs.get(0)), + () -> assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.get(1)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(2))); } @Test @@ -242,16 +246,17 @@ void shouldReturnChangeLogsWhenTrackedEntityAttributeValueIsCreatedUpdatedAndDel updateAttributeValue(trackedEntity, trackedEntityAttribute, updatedValue); updateAttributeValue(trackedEntity, trackedEntityAttribute, ""); - Page changeLogs = - trackedEntityChangeLogService.getTrackedEntityChangeLog( - UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams); + List changeLogs = + filterTrackedEntityAttribute( + trackedEntityChangeLogService.getTrackedEntityChangeLog( + UID.of(trackedEntity), null, defaultOperationParams, defaultPageParams), + trackedEntityAttribute); - assertNumberOfChanges(3, changeLogs.getItems()); + assertNumberOfChanges(3, changeLogs); assertAll( - () -> assertDelete(trackedEntityAttribute, updatedValue, changeLogs.getItems().get(0)), - () -> - assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.getItems().get(1)), - () -> assertCreate(trackedEntityAttribute, "88", changeLogs.getItems().get(2))); + () -> assertDelete(trackedEntityAttribute, updatedValue, changeLogs.get(0)), + () -> assertUpdate(trackedEntityAttribute, "88", updatedValue, changeLogs.get(1)), + () -> assertCreate(trackedEntityAttribute, "88", changeLogs.get(2))); } @Test @@ -344,4 +349,11 @@ private void updateAttributeValue( importParams, TrackerObjects.builder().trackedEntities(List.of(t)).build())); }); } + + private static List filterTrackedEntityAttribute( + Page changeLogs, String attribute) { + return changeLogs.getItems().stream() + .filter(cl -> cl.getTrackedEntityAttribute().getUid().equalsIgnoreCase(attribute)) + .toList(); + } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportControllerPostgresTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesChangeLogsControllerTest.java similarity index 88% rename from dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportControllerPostgresTest.java rename to dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesChangeLogsControllerTest.java index 8dff7f95e871..5d28ecba8ff2 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportControllerPostgresTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesChangeLogsControllerTest.java @@ -77,7 +77,7 @@ import org.springframework.transaction.annotation.Transactional; @Transactional -class TrackedEntitiesExportControllerPostgresTest extends PostgresControllerIntegrationTestBase { +class TrackedEntitiesChangeLogsControllerTest extends PostgresControllerIntegrationTestBase { @Autowired private ObjectBundleService objectBundleService; @@ -108,19 +108,19 @@ void setUp() throws IOException { trackedEntity = manager.get(TrackedEntity.class, "IOR1AXXl24H"); JsonWebMessage importResponse = - POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload(2)) + POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload("numericAttr", 2)) .content(HttpStatus.OK) .as(JsonWebMessage.class); assertEquals(HttpStatus.OK.toString(), importResponse.getStatus()); importResponse = - POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload(3)) + POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload("numericAttr", 3)) .content(HttpStatus.OK) .as(JsonWebMessage.class); assertEquals(HttpStatus.OK.toString(), importResponse.getStatus()); importResponse = - POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload(4)) + POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload("numericAttr", 4)) .content(HttpStatus.OK) .as(JsonWebMessage.class); assertEquals(HttpStatus.OK.toString(), importResponse.getStatus()); @@ -137,9 +137,9 @@ void shouldGetTrackedEntityChangeLogInDescOrderByDefault() { assertNumberOfChanges(3, changeLogs); assertAll( - () -> assertUpdate(trackedEntityAttribute, "3", "4", changeLogs.get(0)), - () -> assertUpdate(trackedEntityAttribute, "2", "3", changeLogs.get(1)), - () -> assertCreate(trackedEntityAttribute, "2", changeLogs.get(2))); + () -> assertUpdate(trackedEntityAttribute.getUid(), "3", "4", changeLogs.get(0)), + () -> assertUpdate(trackedEntityAttribute.getUid(), "2", "3", changeLogs.get(1)), + () -> assertCreate(trackedEntityAttribute.getUid(), "2", changeLogs.get(2))); } @Test @@ -151,9 +151,28 @@ void shouldGetTrackedEntityChangeLogInAscOrder() { assertNumberOfChanges(3, changeLogs); assertAll( - () -> assertCreate(trackedEntityAttribute, "2", changeLogs.get(0)), - () -> assertUpdate(trackedEntityAttribute, "2", "3", changeLogs.get(1)), - () -> assertUpdate(trackedEntityAttribute, "3", "4", changeLogs.get(2))); + () -> assertCreate(trackedEntityAttribute.getUid(), "2", changeLogs.get(0)), + () -> assertUpdate(trackedEntityAttribute.getUid(), "2", "3", changeLogs.get(1)), + () -> assertUpdate(trackedEntityAttribute.getUid(), "3", "4", changeLogs.get(2))); + } + + @Test + void shouldGetEventChangeLogsWhenFilteringByAttribute() { + JsonWebMessage importResponse = + POST("/tracker?async=false&importStrategy=UPDATE", createJsonPayload("toUpdate000", 10)) + .content(HttpStatus.OK) + .as(JsonWebMessage.class); + assertEquals(HttpStatus.OK.toString(), importResponse.getStatus()); + + JsonList changeLogs = + GET( + "/tracker/trackedEntities/{id}/changeLogs?filter=attribute:eq:toUpdate000", + trackedEntity.getUid()) + .content(HttpStatus.OK) + .getList("changeLogs", JsonTrackedEntityChangeLog.class); + + assertNumberOfChanges(1, changeLogs); + assertAll(() -> assertCreate("toUpdate000", "10", changeLogs.get(0))); } @Test @@ -318,14 +337,14 @@ private Supplier errorMessage(String errorTitle, ValidationReport report }; } - private String createJsonPayload(int value) { + private String createJsonPayload(String attribute, int value) { return """ { "trackedEntities": [ { "attributes": [ { - "attribute": "numericAttr", + "attribute": "%s", "value": %d } ], @@ -336,7 +355,7 @@ private String createJsonPayload(int value) { ] } """ - .formatted(value); + .formatted(attribute, value); } private static void assertPagerLink(String actual, int page, int pageSize, String start) { @@ -369,7 +388,7 @@ private static void assertUser(JsonTrackedEntityChangeLog changeLog) { } private static void assertCreate( - TrackedEntityAttribute attribute, String currentValue, JsonTrackedEntityChangeLog actual) { + String attribute, String currentValue, JsonTrackedEntityChangeLog actual) { assertAll( () -> assertUser(actual), () -> assertEquals("CREATE", actual.getType()), @@ -377,7 +396,7 @@ private static void assertCreate( } private static void assertUpdate( - TrackedEntityAttribute attribute, + String attribute, String previousValue, String currentValue, JsonTrackedEntityChangeLog actual) { @@ -388,13 +407,12 @@ private static void assertUpdate( } private static void assertChange( - TrackedEntityAttribute attribute, + String attribute, String previousValue, String currentValue, JsonTrackedEntityChangeLog actual) { assertAll( - () -> - assertEquals(attribute.getUid(), actual.getChange().getAttributeValue().getAttribute()), + () -> assertEquals(attribute, actual.getChange().getAttributeValue().getAttribute()), () -> assertEquals(previousValue, actual.getChange().getAttributeValue().getPreviousValue()), () -> assertEquals(currentValue, actual.getChange().getAttributeValue().getCurrentValue())); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/RequestParamsValidator.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/RequestParamsValidator.java index d003bde90fba..c85d3fe6bcf9 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/RequestParamsValidator.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/RequestParamsValidator.java @@ -313,7 +313,7 @@ public static void validateFilter(String filter, Set>> sup for (Pair> filterField : supportedFields) { if (filterField.getKey().equalsIgnoreCase(split[0]) && filterField.getValue() == UID.class - && !CodeGenerator.isValidUid(filterField.getKey())) { + && !CodeGenerator.isValidUid(split[2])) { throw new BadRequestException( String.format( "Incorrect filter value provided as UID: %s. UID must be an alphanumeric string of 11 characters starting with a letter.", diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/ChangeLogRequestParamsMapper.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/ChangeLogRequestParamsMapper.java index a588816c483a..d9d1e7af6e1b 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/ChangeLogRequestParamsMapper.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/ChangeLogRequestParamsMapper.java @@ -27,11 +27,15 @@ */ package org.hisp.dhis.webapi.controller.tracker.export.trackedentity; +import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validateFilter; import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validateOrderParams; import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validatePaginationBounds; import java.util.List; import java.util.Set; +import org.apache.commons.lang3.tuple.Pair; +import org.hisp.dhis.common.QueryFilter; +import org.hisp.dhis.common.QueryOperator; import org.hisp.dhis.feedback.BadRequestException; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityChangeLogOperationParams; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityChangeLogOperationParams.TrackedEntityChangeLogOperationParamsBuilder; @@ -44,14 +48,18 @@ private ChangeLogRequestParamsMapper() { } static TrackedEntityChangeLogOperationParams map( - Set orderableFields, ChangeLogRequestParams requestParams) + Set orderableFields, + Set>> filterableFields, + ChangeLogRequestParams requestParams) throws BadRequestException { validatePaginationBounds(requestParams.getPage(), requestParams.getPageSize()); validateOrderParams(requestParams.getOrder(), orderableFields); + validateFilter(requestParams.getFilter(), filterableFields); TrackedEntityChangeLogOperationParamsBuilder builder = TrackedEntityChangeLogOperationParams.builder(); mapOrderParam(builder, requestParams.getOrder()); + mapFilterParam(builder, requestParams.getFilter()); return builder.build(); } @@ -63,4 +71,12 @@ private static void mapOrderParam( orders.forEach(order -> builder.orderBy(order.getField(), order.getDirection())); } + + private static void mapFilterParam( + TrackedEntityChangeLogOperationParamsBuilder builder, String filter) { + if (filter != null) { + String[] split = filter.split(":"); + builder.filterBy(split[0], new QueryFilter(QueryOperator.EQ, split[2])); + } + } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportController.java index 55bfb9ede269..e57977c33f59 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/trackedentity/TrackedEntitiesExportController.java @@ -362,7 +362,9 @@ ResponseEntity> getTrackedEntityAttributeChangeLog( TrackedEntityChangeLogOperationParams operationParams = ChangeLogRequestParamsMapper.map( - trackedEntityChangeLogService.getOrderableFields(), requestParams); + trackedEntityChangeLogService.getOrderableFields(), + trackedEntityChangeLogService.getFilterableFields(), + requestParams); PageParams pageParams = new PageParams(requestParams.getPage(), requestParams.getPageSize(), false);