From 848555c8b29add53150aec39cc5d72f5db223225 Mon Sep 17 00:00:00 2001 From: Luciano Belotto Date: Wed, 23 Oct 2024 14:14:05 -0400 Subject: [PATCH] 2024-10-23 - feedback - external reviewer - server-side --- .../FeedbackExternalRecipientController.java | 10 +- .../FeedbackRequestController.java | 6 +- .../FeedbackRequestControllerTest.java | 136 +++++++++++++++++- .../fixture/FeedbackRequestFixture.java | 6 + 4 files changed, 145 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_external_recipient/FeedbackExternalRecipientController.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_external_recipient/FeedbackExternalRecipientController.java index ce5e41664..a9156ab6c 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_external_recipient/FeedbackExternalRecipientController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_external_recipient/FeedbackExternalRecipientController.java @@ -1,5 +1,6 @@ package com.objectcomputing.checkins.services.feedback_external_recipient; +import com.objectcomputing.checkins.exceptions.BadArgException; import com.objectcomputing.checkins.services.feedback_request.*; import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.permissions.RequiredPermission; @@ -39,9 +40,12 @@ public FeedbackExternalRecipientController(FeedbackRequestServices feedbackReque this.feedbackExternalRecipientServices = feedbackExternalRecipientServices; } - @Get("/{?externalRecipientId}") - public List findByValues(@Nullable UUID externalRecipientId) { - return feedbackReqServices.findByValues(null, null, null, null, null, null, null, null) + @Get("/{?creatorId,requesteeId,recipientId,oldestDate,reviewPeriodId,templateId,externalRecipientId,requesteeIds}") + public List findByValues(@Nullable UUID creatorId, @Nullable UUID requesteeId, @Nullable UUID recipientId, @Nullable @Format("yyyy-MM-dd") LocalDate oldestDate, @Nullable UUID reviewPeriodId, @Nullable UUID templateId, @Nullable UUID externalRecipientId, @Nullable List requesteeIds) { + if (externalRecipientId == null) { + throw new BadArgException("Missing required parameter: externalRecipientId"); + } + return feedbackReqServices.findByValues(creatorId, requesteeId, recipientId, oldestDate, reviewPeriodId, templateId, externalRecipientId, requesteeIds) .stream() .map(this::fromEntity) .toList(); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java index b24f34265..663a5e066 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestController.java @@ -123,9 +123,9 @@ public HttpResponse getById(UUID id) { * @return list of {@link FeedbackRequestResponseDTO} */ @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_REQUEST) - @Get("/{?creatorId,requesteeId,recipientId,oldestDate,reviewPeriodId,templateId,requesteeIds}") - public List findByValues(@Nullable UUID creatorId, @Nullable UUID requesteeId, @Nullable UUID recipientId, @Nullable @Format("yyyy-MM-dd") LocalDate oldestDate, @Nullable UUID reviewPeriodId, @Nullable UUID templateId, @Nullable List requesteeIds) { - return feedbackReqServices.findByValues(creatorId, requesteeId, recipientId, oldestDate, reviewPeriodId, templateId, null, requesteeIds) + @Get("/{?creatorId,requesteeId,recipientId,oldestDate,reviewPeriodId,templateId,externalRecipientId,requesteeIds}") + public List findByValues(@Nullable UUID creatorId, @Nullable UUID requesteeId, @Nullable UUID recipientId, @Nullable @Format("yyyy-MM-dd") LocalDate oldestDate, @Nullable UUID reviewPeriodId, @Nullable UUID templateId, @Nullable UUID externalRecipientId, @Nullable List requesteeIds) { + return feedbackReqServices.findByValues(creatorId, requesteeId, recipientId, oldestDate, reviewPeriodId, templateId, externalRecipientId, requesteeIds) .stream() .map(this::fromEntity) .toList(); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java index d4cd95024..601151347 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java @@ -1226,13 +1226,11 @@ void testGetByCreatorRecipientIdPermittedToExternalRecipients() { MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile); - MemberProfile recipient = createADefaultRecipient(); final FeedbackExternalRecipient externalRecipient = createADefaultFeedbackExternalRecipient(); FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, requestee, externalRecipient); //get feedback request final HttpRequest request = HttpRequest.GET(String.format("/?externalRecipientId=%s", feedbackRequest.getExternalRecipientId())); - //.basicAuth(recipient.getWorkEmail(), RoleType.Constants.MEMBER_ROLE); final HttpResponse response = clientExternalRecipient.toBlocking().exchange(request, FeedbackRequestResponseDTO.class); // recipient must be able to get the feedback request @@ -1242,7 +1240,7 @@ void testGetByCreatorRecipientIdPermittedToExternalRecipients() { } @Test - void testGetByCreatorRequesteeIdMultiplePermitted() { + void testGetByCreatorRequesteeIdMultiplePermittedToRecipients() { //create two employee-PDL relationships MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); @@ -1270,7 +1268,35 @@ void testGetByCreatorRequesteeIdMultiplePermitted() { } @Test - void testGetLastThreeMonthsByCreator() { + void testGetByCreatorRequesteeIdMultiplePermittedToExternalRecipients() { + //create two employee-PDL relationships + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); + MemberProfile pdlMemberProfileTwo = createASecondDefaultMemberProfile(); + assignPdlRole(pdlMemberProfileTwo); + final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient(); + final FeedbackExternalRecipient externalRecipient02 = createASecondDefaultFeedbackExternalRecipient(); + + //create two sample feedback requests by the same PDL and same requestee + final FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01); + final FeedbackRequest feedbackReqTwo = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02); + + //search for feedback requests by a specific creator, requestee + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s&requesteeId=%s", feedbackReq.getCreatorId(), feedbackReq.getRequesteeId())) + .basicAuth(pdlMemberProfile.getWorkEmail(), RoleType.Constants.PDL_ROLE); + final HttpResponse> response = client.toBlocking() + .exchange(request, Argument.listOf(FeedbackRequestResponseDTO.class)); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertEquals(2, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackReq, response.getBody().get().get(0)); + assertResponseEqualsEntity(feedbackReqTwo, response.getBody().get().get(1)); + } + + @Test + void testGetLastThreeMonthsByCreatorToRecipients() { //create two employee-PDL relationships MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); @@ -1303,7 +1329,40 @@ void testGetLastThreeMonthsByCreator() { } @Test - void testGetLastThreeMonthsByCreatorRequesteeId() { + void testGetLastThreeMonthsByCreatorToExternalRecipients() { + //create two employee-PDL relationships + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); + MemberProfile pdlMemberProfileTwo = createASecondDefaultMemberProfile(); + assignPdlRole(pdlMemberProfileTwo); + final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient(); + final FeedbackExternalRecipient externalRecipient02 = createASecondDefaultFeedbackExternalRecipient(); + + LocalDate now = LocalDate.now(); + LocalDate oldestDate = now.minusMonths(3); + LocalDate withinLastFewMonths = now.minusMonths(2); + LocalDate outOfRange = now.minusMonths(10); + + // create sample feedback requests with different send dates + final FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01, now); + final FeedbackRequest feedbackReqTwo = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02, withinLastFewMonths); + saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02, outOfRange); + + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s&oldestDate=%s", pdlMemberProfile.getId(), oldestDate)) + .basicAuth(pdlMemberProfile.getWorkEmail(), RoleType.Constants.PDL_ROLE); + final HttpResponse> response = client.toBlocking() + .exchange(request, Argument.listOf(FeedbackRequestResponseDTO.class)); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertEquals(2, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackReq, response.getBody().get().get(0)); + assertResponseEqualsEntity(feedbackReqTwo, response.getBody().get().get(1)); + } + + @Test + void testGetLastThreeMonthsByCreatorRequesteeIdToRecipients() { MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); @@ -1333,7 +1392,37 @@ void testGetLastThreeMonthsByCreatorRequesteeId() { } @Test - void testGetLastThreeMonthsByRequesteeId() { + void testGetLastThreeMonthsByCreatorRequesteeIdToExternalRecipients() { + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); + final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient(); + final FeedbackExternalRecipient externalRecipient02 = createASecondDefaultFeedbackExternalRecipient(); + + LocalDate now = LocalDate.now(); + LocalDate oldestDate = now.minusMonths(3); + LocalDate withinLastFewMonths = now.minusMonths(2); + LocalDate outOfRange = now.minusMonths(10); + + // create sample feedback requests with different send dates + final FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01, now); + final FeedbackRequest feedbackReqTwo = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02, withinLastFewMonths); + saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02, outOfRange); + + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s&requesteeId=%s&oldestDate=%s", pdlMemberProfile.getId(), memberOne.getId(), oldestDate)) + .basicAuth(pdlMemberProfile.getWorkEmail(), RoleType.Constants.PDL_ROLE); + final HttpResponse> response = client.toBlocking() + .exchange(request, Argument.listOf(FeedbackRequestResponseDTO.class)); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertEquals(2, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackReq, response.getBody().get().get(0)); + assertResponseEqualsEntity(feedbackReqTwo, response.getBody().get().get(1)); + } + + @Test + void testGetLastThreeMonthsByRequesteeIdToRecipients() { //create two employee-PDL relationships MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); @@ -1366,7 +1455,40 @@ void testGetLastThreeMonthsByRequesteeId() { } @Test - void testGetEveryAllTimeAdmin() { + void testGetLastThreeMonthsByRequesteeIdToExternalRecipients() { + //create two employee-PDL relationships + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); + MemberProfile pdlMemberProfileTwo = createASecondDefaultMemberProfile(); + assignPdlRole(pdlMemberProfileTwo); + MemberProfile memberTwo = createASecondDefaultMemberProfileForPdl(pdlMemberProfileTwo); + final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient(); + final FeedbackExternalRecipient externalRecipient02 = createASecondDefaultFeedbackExternalRecipient(); + + LocalDate now = LocalDate.now(); + LocalDate oldestDate = now.minusMonths(3); + LocalDate withinLastFewMonths = now.minusMonths(2); + + // create sample feedback requests with different send dates and different requestees + final FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01, now); + saveFeedbackRequest(pdlMemberProfile, memberTwo, externalRecipient02, withinLastFewMonths); + final FeedbackRequest feedbackReqThree = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient02, withinLastFewMonths); + + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s&requesteeId=%s&oldestDate=%s", feedbackReq.getCreatorId(), feedbackReq.getRequesteeId(), oldestDate)) + .basicAuth(pdlMemberProfile.getWorkEmail(), RoleType.Constants.PDL_ROLE); + final HttpResponse> response = client.toBlocking() + .exchange(request, Argument.listOf(FeedbackRequestResponseDTO.class)); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertEquals(2, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackReq, response.getBody().get().get(0)); + assertResponseEqualsEntity(feedbackReqThree, response.getBody().get().get(1)); + } + + @Test + void testGetEveryAllTimeAdminToRecipients() { MemberProfile admin = createADefaultMemberProfile(); assignAdminRole(admin); MemberProfile pdlMemberProfile = createASecondDefaultMemberProfile(); diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackRequestFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackRequestFixture.java index 0a0995a00..9f5b75076 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackRequestFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackRequestFixture.java @@ -184,6 +184,12 @@ default FeedbackRequest saveFeedbackRequest(MemberProfile creator, MemberProfile return getFeedbackRequestRepository().save(feedbackRequest); } + default FeedbackRequest saveFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient feedbackExternalRecipient, LocalDate sendDate) { + final FeedbackRequest feedbackRequest = createFeedbackRequest(creator, requestee, feedbackExternalRecipient); + feedbackRequest.setSendDate(sendDate); + return getFeedbackRequestRepository().save(feedbackRequest); + } + default FeedbackRequest saveFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient, ReviewPeriod reviewPeriod) { final FeedbackRequest feedbackRequest = createFeedbackRequest(creator, requestee, externalRecipient, reviewPeriod); return saveSampleFeedbackRequest(creator, requestee, externalRecipient, feedbackRequest.getTemplateId(), reviewPeriod);