Skip to content

Commit 614e704

Browse files
authored
Merge pull request #2582 from objectcomputing/bugfix-2579/error-transitioning-review-period-to-awaiting-review
Bugfix 2579/error transitioning review period to awaiting review
2 parents 63232f1 + d3bd559 commit 614e704

File tree

2 files changed

+43
-20
lines changed

2 files changed

+43
-20
lines changed

server/src/main/java/com/objectcomputing/checkins/services/reviews/ReviewPeriodServicesImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import jakarta.validation.constraints.NotNull;
1515
import org.slf4j.Logger;
1616
import org.slf4j.LoggerFactory;
17-
1817
import java.util.HashSet;
1918
import java.util.List;
2019
import java.util.Objects;
@@ -139,6 +138,7 @@ private void notifyRevieweeSupervisorsByReviewPeriod(UUID reviewPeriodId, String
139138
Set<ReviewAssignment> reviewAssignments = reviewAssignmentRepository.findByReviewPeriodId(reviewPeriodId);
140139
Set<UUID> revieweeIds = reviewAssignments.stream().map(ReviewAssignment::getRevieweeId).collect(Collectors.toSet());
141140
Set<UUID> supervisorIds = new HashSet<>(memberProfileRepository.findSupervisoridByIdIn(revieweeIds));
141+
supervisorIds.removeIf(Objects::isNull);
142142
if (supervisorIds.isEmpty()) {
143143
LOG.info(String.format("Supervisors not found for Reviewees %s", revieweeIds));
144144
return;

web-ui/src/components/reviews/TeamReviews.jsx

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ const TeamReviews = ({ onBack, periodId }) => {
139139
const [toDelete, setToDelete] = useState(null);
140140
const [unapproved, setUnapproved] = useState([]);
141141
const [validationMessage, setValidationMessage] = useState(null);
142+
const [confirmRevieweesWithNoSupervisorOpen, setConfirmRevieweesWithNoSupervisorOpen] = useState(false);
143+
const [confirmRevieweesWithNoSupervisorQuestion, setConfirmRevieweesWithNoSupervisorQuestionText] = useState('');
142144

143145
const loadedReviews = useRef(false);
144146
const loadingReviews = useRef(false);
@@ -652,25 +654,16 @@ const TeamReviews = ({ onBack, periodId }) => {
652654
setValidationMessage(msg);
653655
if (msg) return;
654656

655-
if (period.reviewStatus === ReviewStatus.PLANNING) {
656-
updateReviewPeriodStatus(ReviewStatus.AWAITING_APPROVAL);
657-
} else if (period.reviewStatus === ReviewStatus.AWAITING_APPROVAL) {
658-
const visibleIds = new Set(visibleTeamMembers().map(m => m.id));
659-
const unapproved = assignments.filter(
660-
a => !a.approved && visibleIds.has(a.revieweeId)
661-
);
662-
// logAssignments(unapproved); // for debugging
663-
setUnapproved(unapproved);
664-
setConfirmationText(
665-
unapproved.length === 0
666-
? 'Are you sure you want to launch the review period?'
667-
: unapproved.length === 1
668-
? 'There is one visible, unapproved review assignment. ' +
669-
'Would you like to approve it and launch this review period?'
670-
: `There are ${unapproved.length} visible, unapproved review assignments. ` +
671-
'Would you like to approve all of them and launch this review period?'
672-
);
673-
setConfirmationDialogOpen(true);
657+
const uniqueNamesWithNoSupervisor = [...new Set(
658+
visibleTeamMembers()
659+
.filter(member => member.supervisorid === null) // Filter by null supervisorid
660+
.map(member => member.name) // Map to the name property
661+
)].join(', ');
662+
if (uniqueNamesWithNoSupervisor.trim().length > 0) {
663+
setConfirmRevieweesWithNoSupervisorQuestionText(uniqueNamesWithNoSupervisor);
664+
setConfirmRevieweesWithNoSupervisorOpen(true);
665+
} else {
666+
return requestApprovalPost();
674667
}
675668
};
676669

@@ -784,6 +777,29 @@ const TeamReviews = ({ onBack, periodId }) => {
784777
onBack();
785778
};
786779

780+
const requestApprovalPost = async () => {
781+
if (period.reviewStatus === ReviewStatus.PLANNING) {
782+
updateReviewPeriodStatus(ReviewStatus.AWAITING_APPROVAL);
783+
} else if (period.reviewStatus === ReviewStatus.AWAITING_APPROVAL) {
784+
const visibleIds = new Set(visibleTeamMembers().map(m => m.id));
785+
const unapproved = assignments.filter(
786+
a => !a.approved && visibleIds.has(a.revieweeId)
787+
);
788+
// logAssignments(unapproved); // for debugging
789+
setUnapproved(unapproved);
790+
setConfirmationText(
791+
unapproved.length === 0
792+
? 'Are you sure you want to launch the review period?'
793+
: unapproved.length === 1
794+
? 'There is one visible, unapproved review assignment. ' +
795+
'Would you like to approve it and launch this review period?'
796+
: `There are ${unapproved.length} visible, unapproved review assignments. ` +
797+
'Would you like to approve all of them and launch this review period?'
798+
);
799+
setConfirmationDialogOpen(true);
800+
}
801+
}
802+
787803
const unapproveAll = () => {
788804
visibleTeamMembers().map(member => approveMember(member, false));
789805
};
@@ -1054,6 +1070,13 @@ const TeamReviews = ({ onBack, periodId }) => {
10541070
setOpen={setConfirmationDialogOpen}
10551071
title="Approve and Launch"
10561072
/>
1073+
<ConfirmationDialog
1074+
open={confirmRevieweesWithNoSupervisorOpen}
1075+
onYes={requestApprovalPost}
1076+
question={confirmRevieweesWithNoSupervisorQuestion}
1077+
setOpen={setConfirmRevieweesWithNoSupervisorOpen}
1078+
title="These reviewees have no supervisor. Continue?"
1079+
/>
10571080
</Root>
10581081
);
10591082
};

0 commit comments

Comments
 (0)