Skip to content
Merged
13 changes: 6 additions & 7 deletions web-ui/src/components/menu/Menu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
selectHasAnniversaryReportPermission,
selectHasBirthdayReportPermission,
selectHasCheckinsReportPermission,
selectHasPulseReportPermission,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was renamed to selectHasViewPulseReportPermission for consistency with other permissions.

selectHasSkillsReportPermission,
selectHasTeamSkillsReportPermission
selectHasTeamSkillsReportPermission,
selectHasViewPulseReportPermission
} from '../../context/selectors';
import { UPDATE_TOAST } from '../../context/actions';

Expand Down Expand Up @@ -123,11 +123,10 @@ function Menu({ children }) {
links.push(['/checkins-reports', 'Check-ins']);
}

//TODO: Skipping the permission check during testing
// because this permission has not been implemented yet.
// if (selectHasPulseReportPermission(state)) {
links.push(['/pulse-reports', 'Pulses']);
// }
// TODO: Uncomment this check after PR #2429 is merged.
//if (selectHasViewPulseReportPermission(state)) {
links.push(['/pulse-reports', 'Pulses']);
//}

if (selectHasSkillsReportPermission(state)) {
links.push(['/skills-reports', 'Skills']);
Expand Down
131 changes: 74 additions & 57 deletions web-ui/src/components/reviews/TeamReviews.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import PropTypes from 'prop-types';
import queryString from 'query-string';
import React, {
useEffect,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetized imported items.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My OCD thanks you for this!

useContext,
useCallback,
useContext,
useEffect,
useRef,
useState,
useRef
} from 'react';
import { useLocation, useHistory } from 'react-router-dom';

Expand Down Expand Up @@ -65,6 +65,9 @@ import {
selectCurrentMembers,
selectCurrentUser,
selectCurrentUserSubordinates,
selectHasDeleteReviewPeriodPermission,
selectHasLaunchReviewPeriodPermission,
selectHasUpdateReviewPeriodPermission,
selectIsAdmin,
selectReviewPeriod,
selectSupervisors,
Expand Down Expand Up @@ -127,6 +130,7 @@ const TeamReviews = ({ onBack, periodId }) => {

const [approvalMode, setApprovalMode] = useState(false);
const [assignments, setAssignments] = useState([]);
const [canUpdate, setCanUpdate] = useState(false);
const [confirmApproveAllOpen, setConfirmApproveAllOpen] = useState(false);
const [confirmationDialogOpen, setConfirmationDialogOpen] = useState(false);
const [confirmationText, setConfirmationText] = useState('');
Expand Down Expand Up @@ -174,6 +178,7 @@ const TeamReviews = ({ onBack, periodId }) => {
isManager && period.reviewStatus === ReviewStatus.AWAITING_APPROVAL
);
}
setCanUpdate(selectHasUpdateReviewPeriodPermission(state));
}, [state]);

useEffect(() => {
Expand Down Expand Up @@ -735,7 +740,7 @@ const TeamReviews = ({ onBack, periodId }) => {
key={reviewer.id}
label={reviewer.name}
variant="outlined"
onDelete={() => deleteReviewer(member, reviewer)}
onDelete={canUpdate ? () => deleteReviewer(member, reviewer) : null}
Copy link
Collaborator Author

@mvolkmann mvolkmann May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of UI elements are not either removed or disabled based on the values of canUpdate and canLaunch.

style={{
backgroundColor: reviewer.approved
? 'var(--checkins-palette-action-green)'
Expand All @@ -753,7 +758,8 @@ const TeamReviews = ({ onBack, periodId }) => {
case ReviewStatus.PLANNING:
return <Button onClick={requestApproval}>Request Approval</Button>;
case ReviewStatus.AWAITING_APPROVAL:
return <Button onClick={requestApproval}>Launch Review</Button>;
return selectHasLaunchReviewPeriodPermission(state) ?
<Button onClick={requestApproval}>Launch Review</Button> : null;
default:
return null;
}
Expand Down Expand Up @@ -843,37 +849,43 @@ const TeamReviews = ({ onBack, periodId }) => {
<Typography variant="h4">{period?.name ?? ''} Team Reviews</Typography>
{period && isAdmin && (
<div>
<Tooltip
title={
period.reviewStatus === ReviewStatus.OPEN
? 'Archive'
: 'Unarchive'
}
>
<IconButton
onClick={toggleReviewPeriod}
aria-label={
{canUpdate && (
<Tooltip
title={
period.reviewStatus === ReviewStatus.OPEN
? 'Archive'
: 'Unarchive'
}
>
{period.reviewStatus === ReviewStatus.OPEN ? (
<Archive />
) : (
<Unarchive />
)}
</IconButton>
</Tooltip>
<Tooltip title="Delete">
<IconButton
onClick={confirmDelete}
edge="end"
aria-label="Delete"
>
<Delete />
</IconButton>
</Tooltip>
<IconButton
onClick={toggleReviewPeriod}
aria-label={
period.reviewStatus === ReviewStatus.OPEN
? 'Archive'
: 'Unarchive'
}
>
{period.reviewStatus === ReviewStatus.OPEN ? (
<Archive />
) : (
<Unarchive />
)}
</IconButton>
</Tooltip>
)}

{selectHasDeleteReviewPeriodPermission(state) && (
<Tooltip title="Delete">
<IconButton
onClick={confirmDelete}
edge="end"
aria-label="Delete"
>
<Delete />
</IconButton>
</Tooltip>
)}

{approvalMode && (
<FormControlLabel
control={
Expand All @@ -896,20 +908,20 @@ const TeamReviews = ({ onBack, periodId }) => {
date={period.launchDate}
setDate={val => handleLaunchDateChange(val, period)}
label="Launch Date"
disabled={!isAdmin}
disabled={!canUpdate}
open={period?.reviewStatus === ReviewStatus?.PLANNING}
/>
<DatePickerField
date={period.selfReviewCloseDate}
setDate={val => handleSelfReviewDateChange(val, period)}
label="Self-Review Date"
disabled={!isAdmin}
disabled={!canUpdate}
/>
<DatePickerField
date={period.closeDate}
setDate={val => handleCloseDateChange(val, period)}
label="Close Date"
disabled={!isAdmin}
disabled={!canUpdate}
/>
</div>
{approvalButton()}
Expand Down Expand Up @@ -938,22 +950,25 @@ const TeamReviews = ({ onBack, periodId }) => {
)
}}
/>
<div>
<Button onClick={() => setConfirmApproveAllOpen(true)}>
Approve All
</Button>
<Button onClick={unapproveAll}>Unapprove All</Button>
</div>
{canUpdate && (
<div>
<Button onClick={() => setConfirmApproveAllOpen(true)}>
Approve All
</Button>
<Button onClick={unapproveAll}>Unapprove All</Button>
</div>
)}
</div>
)}

{/* TODO: Only render this if the user has a specific permission. */}
<MemberSelector
className="team-skill-member-selector"
exportable
onChange={updateTeamMembers}
selected={teamMembers}
/>
{canUpdate && (
<MemberSelector
className="team-skill-member-selector"
exportable
onChange={updateTeamMembers}
selected={teamMembers}
/>
)}

<List dense role="list">
{visibleTeamMembers().map(member => (
Expand All @@ -970,13 +985,17 @@ const TeamReviews = ({ onBack, periodId }) => {
<div className="chip-row">
<Typography>Reviewers:</Typography>
{renderReviewers(member)}
<IconButton
aria-label="Edit Reviewers"
onClick={() => editReviewers(member)}
>
<AddCircle />
</IconButton>
{approvalMode && (

{canUpdate && (
<IconButton
aria-label="Edit Reviewers"
onClick={() => editReviewers(member)}
>
<AddCircle />
</IconButton>
)}

{canUpdate && approvalMode && (
<Button onClick={() => toggleApproval(member)}>
{isMemberApproved(member) ? 'Unapprove' : 'Approve'}
</Button>
Expand Down Expand Up @@ -1020,9 +1039,7 @@ const TeamReviews = ({ onBack, periodId }) => {
</DialogContent>
<DialogActions>
<Button onClick={handleConfirmDeleteClose}>No</Button>
<Button onClick={deleteReviewPeriod} autoFocus>
Yes
</Button>
<Button onClick={deleteReviewPeriod} autoFocus>Yes</Button>
</DialogActions>
</Dialog>
<Dialog
Expand Down
5 changes: 3 additions & 2 deletions web-ui/src/components/reviews/periods/ReviewPeriods.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
import {
selectCsrfToken,
selectCurrentUserId,
selectHasCreateReviewPeriodPermission,
selectReviewPeriod,
selectReviewPeriods,
selectUserProfile
Expand Down Expand Up @@ -340,7 +341,7 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
<Root>
<div className={classes.headerContainer}>
<Typography variant="h4">Review Periods</Typography>
{isAdmin ? (
{selectHasCreateReviewPeriodPermission(state) && (
<Button
onClick={handleOpen}
className={classes.actionButtons}
Expand All @@ -349,7 +350,7 @@ const ReviewPeriods = ({ onPeriodSelected, mode }) => {
>
Add Review Period
</Button>
) : null}
)}
</div>
<List sx={{ width: '100%', bgcolor: 'background.paper' }}>
{loading ? (
Expand Down
Loading