From 7e591bfce48009c972422adc9c0fe3b5585ba26e Mon Sep 17 00:00:00 2001 From: Enrico Date: Fri, 17 Jan 2025 10:22:39 -0300 Subject: [PATCH] chore: align fetching a single vs multiple enrollments [DHIS2-18791] --- .../enrollment/DefaultEnrollmentService.java | 124 +++++++++--------- .../enrollment/EnrollmentOperationParams.java | 4 +- .../export/enrollment/EnrollmentService.java | 3 + .../RelationshipsExportControllerTest.java | 1 + 4 files changed, 70 insertions(+), 62 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java index 6e912e995a7f..74a9c9ba6281 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/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 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 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 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 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 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 getRelationshipItems( UserDetails user, Enrollment enrollment, boolean includeDeleted) { Set relationshipItems = new HashSet<>(); @@ -225,24 +222,28 @@ private Set getTrackedEntityAttributeValues( return attributeValues; } + @Nonnull @Override public List getEnrollments(@Nonnull Set uids) throws ForbiddenException { - List enrollments = enrollmentStore.getByUid(UID.toValueList(uids)); - UserDetails user = getCurrentUserDetails(); - List 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 getEnrollments(@Nonnull EnrollmentOperationParams params) throws ForbiddenException, BadRequestException { @@ -255,6 +256,7 @@ public List getEnrollments(@Nonnull EnrollmentOperationParams params queryParams.getOrganisationUnitMode()); } + @Nonnull @Override public Page getEnrollments( @Nonnull EnrollmentOperationParams params, PageParams pageParams) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java index 92d80e5dd037..b56f99ae085c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java @@ -71,7 +71,9 @@ public class EnrollmentOperationParams { @Builder.Default private final Set 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/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java index 06051c96f20c..75bf70a6af4c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java +++ b/dhis-2/dhis-services/dhis-service-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 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 getEnrollments(@Nonnull Set uids) throws ForbiddenException; /** diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java index 9261a496081b..fbcdd131e6d1 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java @@ -493,6 +493,7 @@ void getRelationshipsByEnrollmentWithNotes() { @Test void getRelationshipsByEnrollmentNotFound() { + switchContextToUser(user); assertStartsWith( "Enrollment with id Hq3Kc6HK4OZ", GET("/tracker/relationships?enrollment=Hq3Kc6HK4OZ")