Skip to content

Commit 96bbb05

Browse files
committed
2024-11-19 - cleanup submit page for ext-recip
1 parent b36e527 commit 96bbb05

File tree

12 files changed

+27
-34
lines changed

12 files changed

+27
-34
lines changed

build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ tasks.named("sonarqube") {
88
}
99

1010
tasks.named("sonar") {
11+
dependsOn ":web-ui:yarn_run_coverage", ":server:jacocoTestReport"
1112
dependsOn ":web-ui-external-feedback:yarn_run_coverage", ":server:jacocoTestReport"
1213
}

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public HttpResponse<FeedbackAnswerResponseDTO> getById(UUID id) {
8282
@Get("/{?questionId,requestId}")
8383
@RequiredPermission(Permission.CAN_VIEW_FEEDBACK_ANSWER)
8484
public List<FeedbackAnswerResponseDTO> findByValues(@Nullable UUID questionId, @Nullable UUID requestId) {
85-
return feedbackAnswerServices.findByValues(questionId, requestId, null)
85+
return feedbackAnswerServices.findByValues(questionId, requestId)
8686
.stream()
8787
.map(this::fromEntity)
8888
.toList();

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerExternalRecipientController.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,11 @@ public HttpResponse<FeedbackAnswerResponseDTO> getById(UUID id) {
9090
*
9191
* @param questionId The attached {@link UUID} of the related question
9292
* @param requestId The attached {@link UUID} of the request that corresponds with the answer
93-
* @param externalRecipientId The attached {@link UUID} of the external-recipient that corresponds with the answer
9493
* @return {@link FeedbackAnswerResponseDTO}
9594
*/
96-
@Get("/{?questionId,requestId,externalRecipientId}")
97-
public List<FeedbackAnswerResponseDTO> findByValues(@Nullable UUID questionId, @Nullable UUID requestId, @Nullable UUID externalRecipientId) {
98-
return feedbackAnswerServices.findByValues(questionId, requestId, externalRecipientId)
95+
@Get("/{?questionId,requestId}")
96+
public List<FeedbackAnswerResponseDTO> findByValues(@Nullable UUID questionId, @Nullable UUID requestId) {
97+
return feedbackAnswerServices.findByValues(questionId, requestId)
9998
.stream()
10099
.map(this::fromEntity)
101100
.toList();

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerRepository.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,4 @@ public interface FeedbackAnswerRepository extends CrudRepository<FeedbackAnswer,
2727
"FROM feedback_answers WHERE (:questionId IS NULL OR question_id = :questionId) AND (request_id = :requestId)")
2828
List<FeedbackAnswer> getByQuestionIdAndRequestId(@Nullable String questionId, @Nullable String requestId);
2929

30-
@Query("SELECT FA.id, PGP_SYM_DECRYPT(cast(FA.answer as bytea), '${aes.key}') as answer, FA.question_id, FA.request_id, FA.sentiment " +
31-
"FROM feedback_answers FA JOIN feedback_requests FR on FA.request_id = FR.id " +
32-
"WHERE 1=1 " +
33-
"and (:questionId IS NULL OR question_id = :questionId) " +
34-
"AND (request_id = :requestId) " +
35-
"AND :externalRecipientId = FR.external_recipient_id " +
36-
"")
37-
List<FeedbackAnswer> getByQuestionIdAndRequestId(@Nullable String questionId, @Nullable String requestId, @NotNull String externalRecipientId);
38-
3930
}

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServices.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public interface FeedbackAnswerServices {
1414

1515
FeedbackAnswer getById(UUID id);
1616

17-
List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UUID requestId, @Nullable UUID externalRecipientId);
17+
List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UUID requestId);
1818

1919
boolean getIsPermitted(FeedbackRequest feedbackRequest);
20-
}
20+
}

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/FeedbackAnswerServicesImpl.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public FeedbackAnswer getById(UUID id) {
9898
}
9999

100100
@Override
101-
public List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UUID requestId, @Nullable UUID externalRecipientId) {
101+
public List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UUID requestId) {
102102
List<FeedbackAnswer> response = new ArrayList<>();
103103
FeedbackRequest feedbackRequest;
104104
MemberProfile currentUser;
@@ -123,11 +123,11 @@ public List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UU
123123
boolean isRequesteesSupervisor = requesteeId != null && currentUserId != null ? memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId())) : false;
124124
MemberProfile requestee = memberProfileServices.getById(requesteeId);
125125
final UUID requesteePDL = requestee.getPdlId();
126-
if (currentUserServices.isAdmin() || (currentUserId != null && currentUserId.equals(requesteePDL)) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || (recipientId != null && recipientId.equals(currentUserId))) {
126+
if (currentUserServices.isAdmin() || (currentUserId != null && currentUserId.equals(requesteePDL)) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || (recipientId != null && recipientId.equals(currentUserId)) || (currentUserId != null && feedbackRequestServices.selfRevieweeIsCurrentUserReviewee(feedbackRequest, currentUserId))) {
127127
response.addAll(feedbackAnswerRepository.getByQuestionIdAndRequestId(Util.nullSafeUUIDToString(questionId), Util.nullSafeUUIDToString(requestId)));
128128
return response;
129-
} else if (externalRecipientId != null) {
130-
response.addAll(feedbackAnswerRepository.getByQuestionIdAndRequestId(Util.nullSafeUUIDToString(questionId), Util.nullSafeUUIDToString(requestId), externalRecipientId.toString()));
129+
} else if (feedbackRequest.getExternalRecipientId() != null) {
130+
response.addAll(feedbackAnswerRepository.getByQuestionIdAndRequestId(Util.nullSafeUUIDToString(questionId), Util.nullSafeUUIDToString(requestId)));
131131
return response;
132132
}
133133

@@ -175,7 +175,11 @@ public boolean getIsPermitted(FeedbackRequest feedbackRequest) {
175175
}
176176
final UUID currentUserId = (currentUser != null) ? currentUserServices.getCurrentUser().getId() : null;
177177

178-
final boolean isAdmin = currentUserServices.isAdmin();
178+
// Admins can always get questions and answers.
179+
if (currentUser != null && currentUserServices.isAdmin()) {
180+
return true;
181+
}
182+
179183
final UUID requestCreatorId = feedbackRequest.getCreatorId();
180184
UUID requesteeId = feedbackRequest.getRequesteeId();
181185
MemberProfile requestee = memberProfileServices.getById(requesteeId);
@@ -190,6 +194,6 @@ public boolean getIsPermitted(FeedbackRequest feedbackRequest) {
190194

191195
final UUID requesteePDL = requestee.getPdlId();
192196

193-
return isAdmin || (currentUserId != null && currentUserId.equals(requesteePDL)) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || (recipientId != null && recipientId.equals(currentUserId)) || feedbackRequest.getExternalRecipientId()!= null;
197+
return (currentUserId != null && currentUserId.equals(requesteePDL)) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || (recipientId != null && recipientId.equals(currentUserId)) || feedbackRequest.getExternalRecipientId()!= null;
194198
}
195199
}

server/src/main/java/com/objectcomputing/checkins/services/feedback_answer/question_and_answer/QuestionAndAnswerServicesImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public List<Tuple> getAllQuestionsAndAnswers(UUID requestId) {
4646
throw new PermissionException(NOT_AUTHORIZED_MSG);
4747
}
4848
List<TemplateQuestion> templateQuestions = templateQuestionServices.findByFields(feedbackRequest.getTemplateId());
49-
List<FeedbackAnswer> answerList = feedbackAnswerServices.findByValues(null, requestId, null);
49+
List<FeedbackAnswer> answerList = feedbackAnswerServices.findByValues(null, requestId);
5050

5151
List<Tuple> returnerList = new ArrayList<>();
5252

@@ -86,7 +86,7 @@ public Tuple getQuestionAndAnswer(@Nullable UUID requestId, @Nullable UUID quest
8686
}
8787

8888
List<FeedbackAnswer> list;
89-
list = feedbackAnswerServices.findByValues(questionId, requestId, null);
89+
list = feedbackAnswerServices.findByValues(questionId, requestId);
9090

9191
FeedbackAnswer returnedAnswer;
9292
if (list.isEmpty()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void sendNewRequestEmail(FeedbackRequest storedRequest) {
159159
reviewerExternalRecipient = feedbackExternalRecipientServices.getById(recipientOrExternalRecipientId);
160160
reviewerFirstName = reviewerExternalRecipient.getFirstName();
161161
reviewerEmail = reviewerExternalRecipient.getEmail();
162-
urlFeedbackSubmit = "/externalFeedback/submitForExternalRecipient?request=";
162+
urlFeedbackSubmit = "/externalFeedback/submit?request=";
163163
} else {
164164
recipientOrExternalRecipientId = storedRequest.getRecipientId();
165165
reviewerMemberProfile = memberProfileServices.getById(recipientOrExternalRecipientId);

server/src/main/java/com/objectcomputing/checkins/services/feedback_template/FeedbackTemplate.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ public String toString() {
165165
public static boolean customEquals(Boolean a, Boolean b) {
166166
if (Boolean.FALSE.equals(a) && b == null) return true;
167167
if (a == null && Boolean.FALSE.equals(b)) return true;
168+
// Objects.equals returns true if both are null
168169
return Objects.equals(a, b);
169170
}
170171

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,11 @@ public boolean deleteIsPermitted(UUID templateCreatorId) {
153153
return createIsPermitted(templateCreatorId);
154154
}
155155

156-
public boolean getIsPermitted(UUID templateQuestionId) {
156+
public boolean getIsPermitted(UUID templateId) {
157157
UUID currentUserId;
158158
MemberProfile currentUser;
159159

160-
final Optional<TemplateQuestion> templateQuestion = templateQuestionRepository.findById(templateQuestionId);
161-
final Optional<FeedbackTemplate> feedbackTemplate = (templateQuestion.isPresent()) ? feedbackTemplateRepo.findById(templateQuestion.get().getTemplateId()) : null;
160+
final Optional<FeedbackTemplate> feedbackTemplate = feedbackTemplateRepo.findById(templateId);
162161

163162
try {
164163
currentUser = currentUserServices.getCurrentUser();
@@ -168,7 +167,7 @@ public boolean getIsPermitted(UUID templateQuestionId) {
168167
currentUserId = null;
169168
}
170169

171-
return (currentUserId != null || (feedbackTemplate.isPresent() && feedbackTemplate.get().getIsForExternalRecipient() != null && feedbackTemplate.get().getIsForExternalRecipient() == true));
170+
return (currentUserId != null || (feedbackTemplate != null && feedbackTemplate.isPresent() && feedbackTemplate.get().getIsForExternalRecipient() != null && feedbackTemplate.get().getIsForExternalRecipient() == true));
172171
}
173172

174173
}

0 commit comments

Comments
 (0)