Skip to content

Commit 7e2785d

Browse files
Luch76Luch76
committed
2024-11-19 - Correction to test-cases
1 parent 8560502 commit 7e2785d

File tree

4 files changed

+48
-25
lines changed

4 files changed

+48
-25
lines changed

‎server/src/main/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestServicesImpl.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ public boolean selfRevieweeIsCurrentUserReviewee(FeedbackRequest request, UUID c
415415
// request in the same review period that is assigned to the current
416416
// user and the requestee is the same as the self-review request. If
417417
// so, this user is allowed to see the self-review request.
418-
if (request.getRecipientId().equals(request.getRequesteeId())) {
418+
if (request.getRecipientId() != null && request.getRecipientId().equals(request.getRequesteeId())) {
419419
List<FeedbackRequest> other = feedbackReqRepository.findByValues(
420420
null, request.getRecipientId().toString(),
421421
currentUserId.toString(), null,
@@ -455,9 +455,16 @@ private boolean getIsPermitted(FeedbackRequest feedbackReq) {
455455
private boolean getIsPermittedForExternalRecipient(FeedbackRequest feedbackReq) {
456456
final LocalDate sendDate = feedbackReq.getSendDate();
457457
final LocalDate today = LocalDate.now();
458+
MemberProfile currentUser;
459+
try {
460+
currentUser = currentUserServices.getCurrentUser();
461+
} catch (NotFoundException notFoundException) {
462+
currentUser = null;
463+
}
458464

459465
// The recipient can only access the feedback request after it has been sent
460-
if (sendDate.isAfter(today)) {
466+
// Since request is for an external recipient, any logged in user can access it as long as they have feedback-reques-id
467+
if (sendDate.isAfter(today) && currentUser == null) {
461468
throw new PermissionException("You are not permitted to access this request before the send date.");
462469
}
463470
return true;

‎server/src/main/java/com/objectcomputing/checkins/services/feedback_template/template_question/TemplateQuestionServicesImpl.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,14 @@ public void delete(UUID id) {
116116
@Override
117117
public TemplateQuestion getById(UUID id) {
118118
final Optional<TemplateQuestion> templateQuestion = templateQuestionRepository.findById(id);
119-
if (!getIsPermitted(id)) {
120-
throw new PermissionException(NOT_AUTHORIZED_MSG);
119+
if (templateQuestion == null || templateQuestion.isEmpty()) {
120+
throw new NotFoundException("No feedback question with ID " + id);
121121
}
122+
final Optional<FeedbackTemplate> feedbackTemplateOptional = feedbackTemplateRepo.findById(templateQuestion.get().getTemplateId());
123+
FeedbackTemplate feedbackTemplate = feedbackTemplateOptional != null && feedbackTemplateOptional.isPresent() ? feedbackTemplateOptional.get() : null;
122124

123-
if (templateQuestion.isEmpty()) {
124-
throw new NotFoundException("No feedback question with ID " + id);
125+
if (!getIsPermitted(feedbackTemplate)) {
126+
throw new PermissionException(NOT_AUTHORIZED_MSG);
125127
}
126128

127129
return templateQuestion.get();
@@ -131,10 +133,14 @@ public TemplateQuestion getById(UUID id) {
131133
public List<TemplateQuestion> findByFields(UUID templateId) {
132134
if (templateId == null) {
133135
throw new BadArgException("Cannot find template questions for null template ID");
134-
} else if (!getIsPermitted(templateId)) {
135-
throw new PermissionException(NOT_AUTHORIZED_MSG);
136136
}
137137

138+
final Optional<FeedbackTemplate> feedbackTemplateOptional = feedbackTemplateRepo.findById(templateId);
139+
FeedbackTemplate feedbackTemplate = feedbackTemplateOptional != null && feedbackTemplateOptional.isPresent() ? feedbackTemplateOptional.get() : null;
140+
141+
if (!getIsPermitted(feedbackTemplate)) {
142+
throw new PermissionException(NOT_AUTHORIZED_MSG);
143+
}
138144
return new ArrayList<>(templateQuestionRepository.findByTemplateId(Util.nullSafeUUIDToString(templateId)));
139145
}
140146

@@ -153,22 +159,17 @@ public boolean deleteIsPermitted(UUID templateCreatorId) {
153159
return createIsPermitted(templateCreatorId);
154160
}
155161

156-
public boolean getIsPermitted(UUID templateId) {
162+
public boolean getIsPermitted(FeedbackTemplate feedbackTemplate) {
157163
UUID currentUserId;
158164
MemberProfile currentUser;
159165

160-
final Optional<FeedbackTemplate> feedbackTemplateOptional = feedbackTemplateRepo.findById(templateId);
161-
FeedbackTemplate feedbackTemplate;
162-
feedbackTemplate = feedbackTemplateOptional.orElse(null);
163-
164166
try {
165167
currentUser = currentUserServices.getCurrentUser();
166168
currentUserId = currentUser.getId();
167169
} catch (NotFoundException e) {
168170
currentUser = null;
169171
currentUserId = null;
170172
}
171-
172173
if (currentUserId != null) return true;
173174

174175
boolean templateForExternalRecipient = (feedbackTemplate != null && feedbackTemplate.getIsForExternalRecipient() == true);

‎server/src/test/java/com/objectcomputing/checkins/services/feedback_request/FeedbackRequestControllerTest.java‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -803,12 +803,21 @@ void testGetFeedbackRequestByUnassignedPdlToExternalRecipient() {
803803
FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, requestee, externalRecipient);
804804

805805
//get feedback request
806-
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
807-
.basicAuth(unrelatedPdl.getWorkEmail(), RoleType.Constants.PDL_ROLE);
806+
final HttpRequest<?> requestHttp = HttpRequest.GET(String.format("%s", feedbackRequest.getId())).basicAuth(unrelatedPdl.getWorkEmail(), RoleType.Constants.PDL_ROLE);
807+
808+
// Access by un-assigned PDL is allowed since the recipient is an external recipient
809+
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(requestHttp, FeedbackRequestResponseDTO.class);
810+
811+
assertEquals(HttpStatus.OK, response.getStatus());
812+
assertTrue(response.getBody().isPresent());
813+
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
814+
815+
/*
808816
final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () ->
809-
client.toBlocking().exchange(request, Map.class));
817+
client.toBlocking().exchange(requestHttp, Map.class));
810818
811819
assertUnauthorized(responseException);
820+
*/
812821
}
813822

814823
@Test
@@ -840,13 +849,20 @@ void testGetFeedbackRequestByRequesteeToExternalRecipient() {
840849
FeedbackRequest feedbackRequest = saveFeedbackRequest(pdlMemberProfile, employeeMemberProfile, externalRecipient);
841850

842851
//get feedback request
843-
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
844-
.basicAuth(memberProfile2.getWorkEmail(), RoleType.Constants.MEMBER_ROLE);
845-
final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () ->
846-
client.toBlocking().exchange(request, Map.class));
852+
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId())).basicAuth(memberProfile2.getWorkEmail(), RoleType.Constants.MEMBER_ROLE);
853+
// Access by requestee is allowed since the recipient is an external recipient
854+
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
855+
856+
assertEquals(HttpStatus.OK, response.getStatus());
857+
assertTrue(response.getBody().isPresent());
858+
assertResponseEqualsEntity(feedbackRequest, response.getBody().get());
847859

860+
/*
861+
Older:
862+
final HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().exchange(request, Map.class));
848863
// requestee should not be able to get the feedback request about them
849864
assertUnauthorized(responseException);
865+
*/
850866
}
851867

852868
@Test
@@ -2353,8 +2369,7 @@ void testRecipientGetBeforeSendDateAsAdminAuthorizedToExternalRecipient() {
23532369
getFeedbackRequestRepository().save(feedbackRequest);
23542370

23552371
//get feedback request
2356-
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId()))
2357-
.basicAuth(adminUser.getWorkEmail(), RoleType.Constants.ADMIN_ROLE);
2372+
final HttpRequest<?> request = HttpRequest.GET(String.format("%s", feedbackRequest.getId())).basicAuth(adminUser.getWorkEmail(), RoleType.Constants.ADMIN_ROLE);
23582373
final HttpResponse<FeedbackRequestResponseDTO> response = client.toBlocking().exchange(request, FeedbackRequestResponseDTO.class);
23592374

23602375
// the sendDate must be before the sent date unless its an admin

‎web-ui-external-feedback/src/api/api.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ const successResult = { id: 123, name: 'test result' };
66

77
const server = setupServer(
88
...[
9-
http.get('http://localhost:8080/fail', () => {
9+
http.get('http://localhost:8080/services/feedback/requests/external/recipients/fail', () => {
1010
return HttpResponse.json(
1111
{ message: 'Internal Server PROBLEM' },
1212
{ status: 500 }
1313
);
1414
}),
15-
http.get('http://localhost:8080/success', () => {
15+
http.get('http://localhost:8080/services/feedback/requests/external/recipients/success', () => {
1616
return HttpResponse.json(successResult);
1717
})
1818
]

0 commit comments

Comments
 (0)