From 2eb6270053fe7e0190ceb462529f35577b6f6e36 Mon Sep 17 00:00:00 2001 From: Luciano Belotto Date: Tue, 22 Oct 2024 15:41:34 -0400 Subject: [PATCH] 2024-10-22 - feedback - external reviewer - server-side --- .../FeedbackRequestController.java | 1 + .../FeedbackRequestServicesImpl.java | 14 +- .../FeedbackRequestControllerTest.java | 122 ++++++++++++++++-- .../FeedbackExternalRecipientFixture.java | 24 +++- .../fixture/FeedbackRequestFixture.java | 57 +++++++- 5 files changed, 195 insertions(+), 23 deletions(-) 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 17cc313de..f12963405 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 @@ -96,6 +96,7 @@ public void delete(@NotNull UUID id) { * @param id {@link UUID} ID of the request * @return {@link FeedbackRequestResponseDTO} */ + //@Secured(SecurityRule.IS_ANONYMOUS) @Get("/{id}") @RequiredPermission(Permission.CAN_VIEW_FEEDBACK_REQUEST) public HttpResponse getById(UUID id) { diff --git a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java index 63a14fd1c..bab75a4b0 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java @@ -330,10 +330,16 @@ public FeedbackRequest getById(UUID id) { } final LocalDate sendDate = feedbackReq.get().getSendDate(); final UUID requesteeId = feedbackReq.get().getRequesteeId(); - final UUID recipientId; - recipientId = feedbackReq.get().getRecipientId() != null ? feedbackReq.get().getRecipientId() : feedbackReq.get().getExternalRecipientId(); - if (!getIsPermitted(requesteeId, recipientId, sendDate)) { - throw new PermissionException(NOT_AUTHORIZED_MSG); + final UUID recipientId = feedbackReq.get().getRecipientId(); + final UUID externalRecipientId = feedbackReq.get().getExternalRecipientId(); + if (recipientId != null) { + if (!getIsPermitted(requesteeId, recipientId, sendDate)) { + throw new PermissionException(NOT_AUTHORIZED_MSG); + } + } else { + if (externalRecipientId == null) { + throw new PermissionException(NOT_AUTHORIZED_MSG); + } } return feedbackReq.get(); 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 77a19aaa3..812a56fde 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 @@ -801,10 +801,11 @@ void testGetFeedbackRequestByUnassignedPdlToExternalRecipient() { //get feedback request final HttpRequest request = HttpRequest.GET(String.format("%s", feedbackRequest.getId())) .basicAuth(unrelatedPdl.getWorkEmail(), RoleType.Constants.PDL_ROLE); - final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> - client.toBlocking().exchange(request, Map.class)); + final HttpResponse response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class); - assertUnauthorized(responseException); + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertResponseEqualsEntity(feedbackRequest, response.getBody().get()); } @Test @@ -838,11 +839,11 @@ void testGetFeedbackRequestByRequesteeToExternalRecipient() { //get feedback request final HttpRequest request = HttpRequest.GET(String.format("%s", feedbackRequest.getId())) .basicAuth(memberProfile2.getWorkEmail(), RoleType.Constants.MEMBER_ROLE); - final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> - client.toBlocking().exchange(request, Map.class)); + final HttpResponse response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class); - // requestee should not be able to get the feedback request about them - assertUnauthorized(responseException); + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertResponseEqualsEntity(feedbackRequest, response.getBody().get()); } @Test @@ -884,7 +885,7 @@ void testGetFeedbackRequestByExternalRecipient() { } @Test - void testGetFeedbackRequestByReviewPeriodId() { + void testGetFeedbackRequestByReviewPeriodIdToRecipient() { ReviewPeriod reviewPeriod = createADefaultReviewPeriod(); MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignAdminRole(pdlMemberProfile); @@ -906,7 +907,28 @@ void testGetFeedbackRequestByReviewPeriodId() { } @Test - void testGetFeedbackRequestByUnrelatedUser() { + void testGetFeedbackRequestByReviewPeriodIdToExternalRecipient() { + ReviewPeriod reviewPeriod = createADefaultReviewPeriod(); + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignAdminRole(pdlMemberProfile); + MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile); + final FeedbackExternalRecipient externalRecipient = createADefaultFeedbackExternalRecipient(); + FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, requestee, externalRecipient, reviewPeriod); + + //search for feedback requests by a specific creator + final HttpRequest request = HttpRequest.GET(String.format("/?reviewPeriodId=%s", reviewPeriod.getId())) + .basicAuth(pdlMemberProfile.getWorkEmail(), RoleType.Constants.ADMIN_ROLE); + final HttpResponse> response = client.toBlocking() + .exchange(request, Argument.listOf(FeedbackRequestResponseDTO.class)); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertEquals(1, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackRequest, response.getBody().get().get(0)); + } + + @Test + void testGetFeedbackRequestByUnrelatedUserToRecipient() { MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile); @@ -924,7 +946,26 @@ void testGetFeedbackRequestByUnrelatedUser() { } @Test - void testGetByCreatorIdPermitted() { + void testGetFeedbackRequestByUnrelatedUserToExternalRecipient() { + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile requestee = createADefaultMemberProfileForPdl(pdlMemberProfile); + final FeedbackExternalRecipient externalRecipient = createADefaultFeedbackExternalRecipient(); + MemberProfile unrelatedUser = createAnUnrelatedUser(); + FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, requestee, externalRecipient); + + //get feedback request + final HttpRequest request = HttpRequest.GET(String.format("%s", feedbackRequest.getId())) + .basicAuth(unrelatedUser.getWorkEmail(), RoleType.Constants.MEMBER_ROLE); + final HttpResponse response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class); + + assertEquals(HttpStatus.OK, response.getStatus()); + assertTrue(response.getBody().isPresent()); + assertResponseEqualsEntity(feedbackRequest, response.getBody().get()); + } + + @Test + void testGetByCreatorIdPermittedToRecipients() { //create two employee-PDL relationships MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); @@ -953,7 +994,36 @@ void testGetByCreatorIdPermitted() { } @Test - void testGetByCreatorIdPermittedMultipleReqs() { + void testGetByCreatorIdPermittedToExternalRecipients() { + //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(); + + // Create a feedback request from a PDL + FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01); + // Create a feedback request by a different PDL + saveFeedbackRequest(pdlMemberProfileTwo, memberTwo, externalRecipient02); + + //search for feedback requests by a specific creator + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s", feedbackReq.getCreatorId())) + .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(1, response.getBody().get().size()); + assertResponseEqualsEntity(feedbackReq, response.getBody().get().get(0)); + } + + @Test + void testGetByCreatorIdPermittedMultipleReqsToRecipients() { //create two employee-PDL relationships MemberProfile pdlMemberProfile = createADefaultMemberProfile(); assignPdlRole(pdlMemberProfile); @@ -984,6 +1054,36 @@ void testGetByCreatorIdPermittedMultipleReqs() { assertResponseEqualsEntity(feedbackReqTwo, response.getBody().get().get(1)); } + @Test + void testGetByCreatorIdPermittedMultipleReqsToExternalRecipients() { + //create two employee-PDL relationships + MemberProfile pdlMemberProfile = createADefaultMemberProfile(); + assignPdlRole(pdlMemberProfile); + MemberProfile memberOne = createADefaultMemberProfileForPdl(pdlMemberProfile); + MemberProfile pdlMemberProfileTwo = createASecondDefaultMemberProfile(); + assignPdlRole(pdlMemberProfileTwo); + MemberProfile memberTwo = createASecondDefaultMemberProfileForPdl(pdlMemberProfileTwo); + MemberProfile memberThree = createAThirdDefaultMemberProfileForPdl(pdlMemberProfileTwo); + final FeedbackExternalRecipient externalRecipient01 = createADefaultFeedbackExternalRecipient(); + + // Create two sample feedback requests by the same PDL + FeedbackRequest feedbackReq = saveFeedbackRequest(pdlMemberProfile, memberOne, externalRecipient01); + FeedbackRequest feedbackReqTwo = saveFeedbackRequest(pdlMemberProfile, memberTwo, externalRecipient01); + // Create a feedback request by a different PDL + saveFeedbackRequest(pdlMemberProfileTwo, memberThree, externalRecipient01); + + final HttpRequest request = HttpRequest.GET(String.format("/?creatorId=%s", feedbackReq.getCreatorId())) + .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 testGetByCreatorRequesteeIdPermitted() { //create two employee-PDL relationships diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackExternalRecipientFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackExternalRecipientFixture.java index 402dca8ba..eaa2883cc 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackExternalRecipientFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/FeedbackExternalRecipientFixture.java @@ -9,7 +9,7 @@ public interface FeedbackExternalRecipientFixture extends RepositoryFixture { default FeedbackExternalRecipient createADefaultFeedbackExternalRecipient() { - String email = "externalRecipient@example.com"; + String email = "externalRecipient01@example.com"; String firstName = "External-01"; String lastName = "Recipient"; String companyName = "Company Name"; @@ -19,4 +19,26 @@ default FeedbackExternalRecipient createADefaultFeedbackExternalRecipient() { )); } + default FeedbackExternalRecipient createASecondDefaultFeedbackExternalRecipient() { + String email = "externalRecipient02@example.com"; + String firstName = "External-02"; + String lastName = "Recipient"; + String companyName = "Company Name"; + + return getFeedbackExternalRecipientRepository().save(new FeedbackExternalRecipient( + email, firstName, lastName, companyName + )); + } + + default FeedbackExternalRecipient createAThirdDefaultFeedbackExternalRecipient() { + String email = "externalRecipient03@example.com"; + String firstName = "External-03"; + String lastName = "Recipient"; + String companyName = "Company Name"; + + return getFeedbackExternalRecipientRepository().save(new FeedbackExternalRecipient( + email, firstName, lastName, companyName + )); + } + } 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 e6979fbc9..0a0995a00 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 @@ -20,7 +20,7 @@ public interface FeedbackRequestFixture extends RepositoryFixture, FeedbackTempl * @param templateId The UUID of the FeedbackTemplate * @return The created {@link FeedbackRequest} */ - default FeedbackRequest createSampleFeedbackRequestWithRecipient(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId) { + default FeedbackRequest createSampleFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId) { LocalDate testDate = LocalDate.of(2010, 10, 8); return new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, "pending", null, null, null); } @@ -47,9 +47,23 @@ default FeedbackRequest createSampleFeedbackRequestWithExternalRecipient(MemberP * @param reviewPeriod the {@link ReviewPeriod} that this feedback request is associated with * @return The created {@link FeedbackRequest} */ - default FeedbackRequest createSampleFeedbackRequestWithRecipient(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId, ReviewPeriod reviewPeriod) { + default FeedbackRequest createSampleFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId, ReviewPeriod reviewPeriod) { LocalDate testDate = LocalDate.of(2010, 10, 8); - return new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, "pending", null, reviewPeriod.getId(), null); /** TODO Luch reciplientg **/ + return new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, "pending", null, reviewPeriod.getId(), null); + } + + /** + * Creates a sample feedback request + * @param creator The {@link MemberProfile} of the creator of the feedback request + * @param requestee The {@link MemberProfile} of the requestee of the feedback request + * @param externalRecipient The {@link FeedbackExternalRecipient} of the external-recipient giving feedback + * @param templateId The UUID of the FeedbackTemplate + * @param reviewPeriod the {@link ReviewPeriod} that this feedback request is associated with + * @return The created {@link FeedbackRequest} + */ + default FeedbackRequest createSampleFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient, UUID templateId, ReviewPeriod reviewPeriod) { + LocalDate testDate = LocalDate.of(2010, 10, 8); + return new FeedbackRequest(creator.getId(), requestee.getId(), null, templateId, testDate, null, "pending", null, reviewPeriod.getId(), externalRecipient.getId()); } /** @@ -88,12 +102,30 @@ default FeedbackRequest saveSampleFeedbackRequest(MemberProfile creator, MemberP */ default FeedbackRequest saveSampleFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId, ReviewPeriod reviewPeriod) { LocalDate testDate = LocalDate.of(2010, 10, 8); - return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, "pending", null, reviewPeriod.getId(), null)); /** TODO Luch recipient **/ + return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, "pending", null, reviewPeriod.getId(), null)); + } + + /** + * Saves a sample feedback request + * @param creator The {@link MemberProfile} of the creator of the feedback request + * @param externalRecipient The {@link FeedbackExternalRecipient} of the external-recipient giving feedback + * @param requestee The {@link MemberProfile} of the requestee of the feedback request + * @param templateId The UUID of the FeedbackTemplate + * @return The saved {@link FeedbackRequest} + */ + default FeedbackRequest saveSampleFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient, UUID templateId, ReviewPeriod reviewPeriod) { + LocalDate testDate = LocalDate.of(2010, 10, 8); + return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), null, templateId, testDate, null, "pending", null, reviewPeriod.getId(), externalRecipient.getId())); } default FeedbackRequest saveSampleFeedbackRequestWithStatus(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, UUID templateId, String status) { LocalDate testDate = LocalDate.of(2010, 10, 8); - return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, status, null, null, null)); /** TODO Luch recipient **/ + return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), recipient.getId(), templateId, testDate, null, status, null, null, null)); + } + + default FeedbackRequest saveSampleFeedbackRequestWithStatus(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient, UUID templateId, String status) { + LocalDate testDate = LocalDate.of(2010, 10, 8); + return getFeedbackRequestRepository().save(new FeedbackRequest(creator.getId(), requestee.getId(), null, templateId, testDate, null, status, null, null, externalRecipient.getId())); } default MemberProfile createADefaultRecipient() { @@ -115,7 +147,7 @@ default MemberProfile createASecondDefaultRecipient() { default FeedbackRequest createFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient) { FeedbackTemplate template = createFeedbackTemplate(creator.getId()); getFeedbackTemplateRepository().save(template); - return createSampleFeedbackRequestWithRecipient(creator, requestee, recipient, template.getId()); + return createSampleFeedbackRequest(creator, requestee, recipient, template.getId()); } default FeedbackRequest createFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient) { @@ -127,7 +159,13 @@ default FeedbackRequest createFeedbackRequest(MemberProfile creator, MemberProfi default FeedbackRequest createFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient, ReviewPeriod reviewPeriod) { FeedbackTemplate template = createAnotherFeedbackTemplate(creator.getId()); getFeedbackTemplateRepository().save(template); - return createSampleFeedbackRequestWithRecipient(creator, requestee, recipient, template.getId(), reviewPeriod); + return createSampleFeedbackRequest(creator, requestee, recipient, template.getId(), reviewPeriod); + } + + default FeedbackRequest createFeedbackRequest(MemberProfile creator, MemberProfile requestee, FeedbackExternalRecipient externalRecipient, ReviewPeriod reviewPeriod) { + FeedbackTemplate template = createAnotherFeedbackTemplate(creator.getId()); + getFeedbackTemplateRepository().save(template); + return createSampleFeedbackRequest(creator, requestee, externalRecipient, template.getId(), reviewPeriod); } default FeedbackRequest saveFeedbackRequest(MemberProfile creator, MemberProfile requestee, MemberProfile recipient) { @@ -146,6 +184,11 @@ default FeedbackRequest saveFeedbackRequest(MemberProfile creator, MemberProfile 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); + } + default List getFeedbackRequests(MemberProfile recipient) { return getFeedbackRequestRepository() .findByValues(null, null, recipient.getId().toString(), null, null, null, null);