Skip to content

Commit

Permalink
Merge pull request #2758 from objectcomputing/bugfix-2753/reviewers-m…
Browse files Browse the repository at this point in the history
…ust-see-reviewees-self-review

Allow reviewers to see the self-reviews of their reviewees for a review period.
  • Loading branch information
mkimberlin authored Nov 13, 2024
2 parents 319e965 + 75f2a2a commit e920319
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 38 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.objectcomputing.checkins.services.feedback_answer;

import com.objectcomputing.checkins.services.feedback_request.FeedbackRequest;
import io.micronaut.core.annotation.Nullable;

import java.util.List;
Expand All @@ -14,4 +15,6 @@ public interface FeedbackAnswerServices {
FeedbackAnswer getById(UUID id);

List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UUID requestId);

boolean getIsPermitted(FeedbackRequest feedbackRequest);
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public List<FeedbackAnswer> findByValues(@Nullable UUID questionId, @Nullable UU
boolean isRequesteesSupervisor = requesteeId != null ? memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId())) : false;
MemberProfile requestee = memberProfileServices.getById(requesteeId);
final UUID requesteePDL = requestee.getPdlId();
if (currentUserServices.isAdmin() || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || recipientId.equals(currentUserId)) {
if (currentUserServices.isAdmin() || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || recipientId.equals(currentUserId) || feedbackRequestServices.selfRevieweeIsCurrentUserReviewee(feedbackRequest, currentUserId)) {
response.addAll(feedbackAnswerRepository.getByQuestionIdAndRequestId(Util.nullSafeUUIDToString(questionId), Util.nullSafeUUIDToString(requestId)));
return response;
}
Expand Down Expand Up @@ -145,15 +145,36 @@ public boolean updateIsPermitted(FeedbackRequest feedbackRequest) {
}

public boolean getIsPermitted(FeedbackRequest feedbackRequest) {
final boolean isAdmin = currentUserServices.isAdmin();
final UUID requestCreatorId = feedbackRequest.getCreatorId();
UUID requesteeId = feedbackRequest.getRequesteeId();
MemberProfile requestee = memberProfileServices.getById(requesteeId);
// Admins can always get questions and answers.
if (currentUserServices.isAdmin()) {
return true;
}

// See if the current user is the requestee's supervisor.
final UUID requesteeId = feedbackRequest.getRequesteeId();
final UUID currentUserId = currentUserServices.getCurrentUser().getId();
if (requesteeId != null &&
memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId()))) {
return true;
}

// See if the current user is the requestee's PDL.
final MemberProfile requestee = memberProfileServices.getById(requesteeId);
if (currentUserId.equals(requestee.getPdlId())) {
return true;
}

// See if the current user is the request creator or the recipient of
// the request.
final UUID requestCreatorId = feedbackRequest.getCreatorId();
final UUID recipientId = feedbackRequest.getRecipientId();
boolean isRequesteesSupervisor = requesteeId != null ? memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId())) : false;
final UUID requesteePDL = requestee.getPdlId();
if (requestCreatorId.equals(currentUserId) ||
recipientId.equals(currentUserId)) {
return true;
}


return isAdmin || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || recipientId.equals(currentUserId);
return feedbackRequestServices.selfRevieweeIsCurrentUserReviewee(
feedbackRequest, currentUserId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public QuestionAndAnswerServicesImpl(FeedbackAnswerServices feedbackAnswerServic
@Override
public List<Tuple> getAllQuestionsAndAnswers(UUID requestId) {
FeedbackRequest feedbackRequest = feedbackRequestServices.getById(requestId);
if (!getIsPermitted(feedbackRequest)) {
if (!feedbackAnswerServices.getIsPermitted(feedbackRequest)) {
throw new PermissionException(NOT_AUTHORIZED_MSG);
}
List<TemplateQuestion> templateQuestions = templateQuestionServices.findByFields(feedbackRequest.getTemplateId());
Expand Down Expand Up @@ -77,7 +77,7 @@ public Tuple getQuestionAndAnswer(@Nullable UUID requestId, @Nullable UUID quest
TemplateQuestion question = new TemplateQuestion();
FeedbackRequest feedbackRequest = feedbackRequestServices.getById(requestId);

if (!getIsPermitted(feedbackRequest)) {
if (!feedbackAnswerServices.getIsPermitted(feedbackRequest)) {
throw new PermissionException(NOT_AUTHORIZED_MSG);
}

Expand All @@ -101,18 +101,4 @@ public Tuple getQuestionAndAnswer(@Nullable UUID requestId, @Nullable UUID quest
}
return new Tuple(question, returnedAnswer);
}

public boolean getIsPermitted(FeedbackRequest feedbackRequest) {
final boolean isAdmin = currentUserServices.isAdmin();
final UUID requestCreatorId = feedbackRequest.getCreatorId();
UUID requesteeId = feedbackRequest.getRequesteeId();
MemberProfile requestee = memberProfileServices.getById(requesteeId);
final UUID currentUserId = currentUserServices.getCurrentUser().getId();
final UUID recipientId = feedbackRequest.getRecipientId();
boolean isRequesteesSupervisor = requesteeId != null ? memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId())) : false;
final UUID requesteePDL = requestee.getPdlId();

return isAdmin || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || requestCreatorId.equals(currentUserId) || recipientId.equals(currentUserId);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ public interface FeedbackRequestServices {
FeedbackRequest getById(UUID id);

List<FeedbackRequest> findByValues(UUID creatorId, UUID requesteeId, UUID recipientId, LocalDate oldestDate, UUID reviewPeriodId, UUID templateId, List<UUID> requesteeIds);
}

boolean selfRevieweeIsCurrentUserReviewee(FeedbackRequest request,
UUID currentUserId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,8 @@ public FeedbackRequest getById(UUID id) {
if (feedbackReq.isEmpty()) {
throw new NotFoundException("No feedback req with id " + id);
}
final LocalDate sendDate = feedbackReq.get().getSendDate();
final UUID requesteeId = feedbackReq.get().getRequesteeId();
final UUID recipientId = feedbackReq.get().getRecipientId();
if (!getIsPermitted(requesteeId, recipientId, sendDate)) {

if (!getIsPermitted(feedbackReq.get())) {
throw new PermissionException(NOT_AUTHORIZED_MSG);
}

Expand Down Expand Up @@ -303,9 +301,12 @@ public List<FeedbackRequest> findByValues(UUID creatorId, UUID requesteeId, UUID
if (currentUserServices.isAdmin()) {
visible = true;
} else if (request != null) {
if (currentUserId.equals(request.getCreatorId())) visible = true;
if (isSupervisor(request.getRequesteeId(), currentUserId)) visible = true;
if (currentUserId.equals(request.getRecipientId())) visible = true;
if (currentUserId.equals(request.getCreatorId()) ||
isSupervisor(request.getRequesteeId(), currentUserId) ||
currentUserId.equals(request.getRecipientId()) ||
selfRevieweeIsCurrentUserReviewee(request, currentUserId)) {
visible = true;
}
}
return visible;
}).toList();
Expand All @@ -318,6 +319,23 @@ private boolean isSupervisor(UUID requesteeId, UUID currentUserId) {
&& memberProfileServices.getSupervisorsForId(requesteeId).stream().anyMatch(profile -> currentUserId.equals(profile.getId()));
}

public boolean selfRevieweeIsCurrentUserReviewee(FeedbackRequest request,
UUID currentUserId) {
// If we are looking at a self-review request, see if there is a review
// request in the same review period that is assigned to the current
// user and the requestee is the same as the self-review request. If
// so, this user is allowed to see the self-review request.
if (request.getRecipientId().equals(request.getRequesteeId())) {
List<FeedbackRequest> other = feedbackReqRepository.findByValues(
null, request.getRecipientId().toString(),
currentUserId.toString(), null,
Util.nullSafeUUIDToString(request.getReviewPeriodId()),
null);
return (other.size() == 1);
}
return false;
}

private boolean createIsPermitted(UUID requesteeId) {
final boolean isAdmin = currentUserServices.isAdmin();
final UUID currentUserId = currentUserServices.getCurrentUser().getId();
Expand All @@ -329,16 +347,21 @@ private boolean createIsPermitted(UUID requesteeId) {
return isAdmin || currentUserId.equals(requesteePDL) || isRequesteesSupervisor || currentUserId.equals(requesteeId);
}

private boolean getIsPermitted(UUID requesteeId, UUID recipientId, LocalDate sendDate) {
LocalDate today = LocalDate.now();
private boolean getIsPermitted(FeedbackRequest feedbackReq) {
final LocalDate sendDate = feedbackReq.getSendDate();
final UUID requesteeId = feedbackReq.getRequesteeId();
final UUID recipientId = feedbackReq.getRecipientId();
final LocalDate today = LocalDate.now();
final UUID currentUserId = currentUserServices.getCurrentUser().getId();

// The recipient can only access the feedback request after it has been sent
if (sendDate.isAfter(today) && currentUserId.equals(recipientId)) {
throw new PermissionException("You are not permitted to access this request before the send date.");
}

return createIsPermitted(requesteeId) || currentUserId.equals(recipientId);
return createIsPermitted(requesteeId) ||
currentUserId.equals(recipientId) ||
selfRevieweeIsCurrentUserReviewee(feedbackReq, currentUserId);
}

private boolean updateDueDateIsPermitted(FeedbackRequest feedbackRequest) {
Expand Down
7 changes: 4 additions & 3 deletions web-ui/src/components/reviews/TeamReviews.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
} from '../../context/actions';
import { AppContext } from '../../context/AppContext';
import {
selectIsAdmin,
selectCsrfToken,
selectCurrentMembers,
selectCurrentUser,
Expand Down Expand Up @@ -125,7 +126,7 @@ const ReviewStatus = {
const TeamReviews = ({ onBack, periodId }) => {
const { state, dispatch } = useContext(AppContext);
const location = useLocation();

const isAdmin = selectIsAdmin(state);
const [openMode, setOpenMode] = useState(false);
const [approvalState, setApprovalState] = useState(false);
const [assignments, setAssignments] = useState([]);
Expand Down Expand Up @@ -796,7 +797,7 @@ const TeamReviews = ({ onBack, periodId }) => {
const url = getReviewerURL(request, selfReviewRequest);

return (url ?
<Link to={url}>
<Link key={member?.id} to={url}>
<Chip
key={reviewer.id}
label={statusLabel}
Expand Down Expand Up @@ -840,7 +841,7 @@ const TeamReviews = ({ onBack, periodId }) => {
const manages = recipientProfile.supervisorid == currentUser?.id;
const request = getReviewRequest(member, currentUser);
const isReviewer = request?.recipientId == currentUser?.id;
if (manages || isReviewer) {
if (isAdmin || manages || isReviewer) {
const selfReviewRequest = getSelfReviewRequest(member);
return (
<Chip
Expand Down

0 comments on commit e920319

Please sign in to comment.