chore: align fetching a single vs multiple enrollments [DHIS2-18791] (#…

…19705) * chore: Use EventService in EnrollmentService [DHIS2-18791] * chore: align fetching a single vs multiple enrollments [DHIS2-18791] * Temporary fix for tests * Clean up tests
dhis2 · Jan 21, 2025 · 1b51f1d · 1b51f1d
1 parent cf6eac0
commit 1b51f1d
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 98 deletions.
diff --git a/...acker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java b/...acker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java
@@ -76,29 +76,68 @@ class DefaultEnrollmentService implements EnrollmentService {
 
   private final EnrollmentOperationParamsMapper paramsMapper;
 
+  @Nonnull
   @Override
   public Enrollment getEnrollment(@Nonnull UID uid) throws ForbiddenException, NotFoundException {
     return getEnrollment(uid, EnrollmentParams.FALSE, false);
   }
 
+  @Nonnull
   @Override
   public Enrollment getEnrollment(
       @Nonnull UID uid, @Nonnull EnrollmentParams params, boolean includeDeleted)
       throws NotFoundException, ForbiddenException {
-    UserDetails currentUser = getCurrentUserDetails();
-    Enrollment enrollment = enrollmentStore.getByUid(uid.getValue());
+    Page<Enrollment> enrollments;
+    try {
+      EnrollmentOperationParams operationParams =
+          EnrollmentOperationParams.builder()
+              .enrollments(Set.of(uid))
+              .enrollmentParams(params)
+              .includeDeleted(includeDeleted)
+              .build();
+      enrollments = getEnrollments(operationParams, new PageParams(1, 1, false));
+    } catch (BadRequestException e) {
+      throw new IllegalArgumentException(
+          "this must be a bug in how the EnrollmentOperationParams are built");
+    }
 
-    if (enrollment == null) {
+    if (enrollments.getItems().isEmpty()) {
       throw new NotFoundException(Enrollment.class, uid);
     }
 
-    List<String> errors = trackerAccessManager.canRead(currentUser, enrollment, false);
+    return enrollments.getItems().get(0);
+  }
 
-    if (!errors.isEmpty()) {
-      throw new ForbiddenException(errors.toString());
+  @Override
+  public RelationshipItem getEnrollmentInRelationshipItem(
+      @Nonnull UID uid, boolean includeDeleted) {
+    Enrollment enrollment;
+    try {
+      enrollment = getEnrollment(uid);
+    } catch (NotFoundException | ForbiddenException e) {
+      // enrollments are not shown in relationships if the user has no access to them
+      return null;
     }
 
-    return getEnrollment(enrollment, params, includeDeleted, currentUser);
+    RelationshipItem relationshipItem = new RelationshipItem();
+    relationshipItem.setEnrollment(enrollment);
+    return relationshipItem;
+  }
+
+  private Set<Event> getEvents(
+      Enrollment enrollment, EventParams eventParams, boolean includeDeleted) {
+    EventOperationParams eventOperationParams =
+        EventOperationParams.builder()
+            .enrollments(Set.of(UID.of(enrollment)))
+            .eventParams(eventParams)
+            .includeDeleted(includeDeleted)
+            .build();
+    try {
+      return Set.copyOf(eventService.getEvents(eventOperationParams));
+    } catch (BadRequestException | ForbiddenException e) {
+      throw new IllegalArgumentException(
+          "this must be a bug in how the EventOperationParams are built");
+    }
   }
 
   private Enrollment getEnrollment(
@@ -151,48 +190,6 @@ private Enrollment getEnrollment(
     return result;
   }
 
-  @Override
-  public RelationshipItem getEnrollmentInRelationshipItem(@Nonnull UID uid, boolean includeDeleted)
-      throws NotFoundException {
-
-    RelationshipItem relationshipItem = new RelationshipItem();
-    Enrollment enrollment = enrollmentStore.getByUid(uid.getValue());
-
-    if (enrollment == null) {
-      throw new NotFoundException(Enrollment.class, uid);
-    }
-
-    UserDetails currentUser = getCurrentUserDetails();
-    List<String> errors = trackerAccessManager.canRead(currentUser, enrollment, false);
-    if (!errors.isEmpty()) {
-      return null;
-    }
-
-    relationshipItem.setEnrollment(
-        getEnrollment(
-            enrollment,
-            EnrollmentParams.FALSE.withIncludeAttributes(true),
-            includeDeleted,
-            currentUser));
-    return relationshipItem;
-  }
-
-  private Set<Event> getEvents(
-      Enrollment enrollment, EventParams eventParams, boolean includeDeleted) {
-    EventOperationParams eventOperationParams =
-        EventOperationParams.builder()
-            .enrollments(Set.of(UID.of(enrollment)))
-            .eventParams(eventParams)
-            .includeDeleted(includeDeleted)
-            .build();
-    try {
-      return Set.copyOf(eventService.getEvents(eventOperationParams));
-    } catch (BadRequestException | ForbiddenException e) {
-      throw new IllegalArgumentException(
-          "this must be a bug in how the EventOperationParams are built");
-    }
-  }
-
   private Set<RelationshipItem> getRelationshipItems(
       UserDetails user, Enrollment enrollment, boolean includeDeleted) {
     Set<RelationshipItem> relationshipItems = new HashSet<>();
@@ -225,24 +222,28 @@ private Set<TrackedEntityAttributeValue> getTrackedEntityAttributeValues(
     return attributeValues;
   }
 
+  @Nonnull
   @Override
   public List<Enrollment> getEnrollments(@Nonnull Set<UID> uids) throws ForbiddenException {
-    List<Enrollment> enrollments = enrollmentStore.getByUid(UID.toValueList(uids));
-    UserDetails user = getCurrentUserDetails();
-    List<String> errors =
-        enrollments.stream()
-            .flatMap(e -> trackerAccessManager.canRead(user, e, false).stream())
-            .toList();
-
-    if (!errors.isEmpty()) {
-      throw new ForbiddenException(errors.toString());
+    EnrollmentQueryParams queryParams;
+    try {
+      queryParams =
+          paramsMapper.map(
+              EnrollmentOperationParams.builder().enrollments(uids).build(),
+              getCurrentUserDetails());
+    } catch (BadRequestException e) {
+      throw new IllegalArgumentException(
+          "this must be a bug in how the EventOperationParams are built");
     }
 
-    return enrollments.stream()
-        .map(e -> getEnrollment(e, EnrollmentParams.FALSE, false, user))
-        .toList();
+    return getEnrollments(
+        new ArrayList<>(enrollmentStore.getEnrollments(queryParams)),
+        EnrollmentParams.FALSE,
+        false,
+        queryParams.getOrganisationUnitMode());
   }
 
+  @Nonnull
   @Override
   public List<Enrollment> getEnrollments(@Nonnull EnrollmentOperationParams params)
       throws ForbiddenException, BadRequestException {
@@ -255,6 +256,7 @@ public List<Enrollment> getEnrollments(@Nonnull EnrollmentOperationParams params
         queryParams.getOrganisationUnitMode());
   }
 
+  @Nonnull
   @Override
   public Page<Enrollment> getEnrollments(
       @Nonnull EnrollmentOperationParams params, PageParams pageParams)

diff --git a/...cker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java b/...cker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java
@@ -71,7 +71,9 @@ public class EnrollmentOperationParams {
   @Builder.Default private final Set<UID> orgUnits = new HashSet<>();
 
   /** Selection mode for the specified organisation units. */
-  private final OrganisationUnitSelectionMode orgUnitMode;
+  @Builder.Default
+  private final OrganisationUnitSelectionMode orgUnitMode =
+      OrganisationUnitSelectionMode.ACCESSIBLE;
 
   /** Enrollments must be enrolled into this program. */
   private final UID program;

diff --git a/...vice-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java b/...vice-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java
@@ -40,8 +40,10 @@
 import org.hisp.dhis.tracker.export.PageParams;
 
 public interface EnrollmentService {
+  @Nonnull
   Enrollment getEnrollment(UID uid) throws ForbiddenException, NotFoundException;
 
+  @Nonnull
   Enrollment getEnrollment(UID uid, EnrollmentParams params, boolean includeDeleted)
       throws NotFoundException, ForbiddenException;
 
@@ -62,6 +64,7 @@ Page<Enrollment> getEnrollments(EnrollmentOperationParams params, PageParams pag
    * Get event matching given {@code UID} under the privileges the user in the context. This method
    * does not get the events relationships.
    */
+  @Nonnull
   List<Enrollment> getEnrollments(@Nonnull Set<UID> uids) throws ForbiddenException;
 
   /**

diff --git a/...ervice-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java b/...ervice-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java
@@ -125,10 +125,6 @@ public class EventOperationParams {
    */
   private List<Order> order;
 
-  private boolean includeAttributes;
-
-  private boolean includeAllDataElements;
-
   @Builder.Default private Set<UID> events = new HashSet<>();
 
   /** Data element filters per data element UID. */

diff --git a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java
@@ -2683,7 +2683,7 @@ protected void injectSecurityContextUser(User user) {
     }
 
     user = userService.getUser(user.getUid());
-    UserDetails userDetails = userService.createUserDetails(user);
+    UserDetails userDetails = UserDetails.fromUser(user);
 
     injectSecurityContext(userDetails);
   }

diff --git a/...t/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java b/...t/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java
@@ -27,11 +27,13 @@
  */
 package org.hisp.dhis.tracker.deduplication;
 
+import static org.hisp.dhis.security.Authorities.ALL;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.List;
+import java.util.Set;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.common.UID;
 import org.hisp.dhis.dbms.DbmsManager;
@@ -52,6 +54,7 @@
 import org.hisp.dhis.tracker.export.enrollment.EnrollmentService;
 import org.hisp.dhis.tracker.imports.bundle.persister.TrackerObjectDeletionService;
 import org.hisp.dhis.tracker.trackedentityattributevalue.TrackedEntityAttributeValueService;
+import org.hisp.dhis.user.User;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.annotation.Transactional;
@@ -145,6 +148,8 @@ void shouldDeleteRelationShips() throws NotFoundException {
   void shouldDeleteEnrollments() throws ForbiddenException, NotFoundException {
     OrganisationUnit ou = createOrganisationUnit("OU_A");
     organisationUnitService.addOrganisationUnit(ou);
+    User user = createAndAddUser(false, "user", Set.of(ou), Set.of(ou), ALL.toString());
+    injectSecurityContextUser(user);
     TrackedEntity original = createTrackedEntity(ou);
     TrackedEntity duplicate = createTrackedEntity(ou);
     TrackedEntity control1 = createTrackedEntity(ou);