From 28caa82ad134925a3bfcb4d89de35894c6a26e9a Mon Sep 17 00:00:00 2001 From: David Leek Date: Mon, 30 Jun 2025 08:51:51 +0200 Subject: [PATCH] feat(changerequests): add requested approvers to overview (#10232) --- .../ChangeRequestOverview.tsx | 17 +- .../ChangeRequestRequestedApprovers.tsx | 372 ++++++++++++++++++ .../ChangeRequestReviewer.tsx | 23 +- .../DraftChangeRequestActions.tsx | 2 +- .../EnvironmentChangeRequest.tsx | 4 +- .../useChangeRequestApi.ts | 10 +- .../useAvailableChangeRequestReviewers.ts | 2 +- .../useRequestedApprovers.ts | 48 +++ 8 files changed, 466 insertions(+), 12 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestRequestedApprovers/ChangeRequestRequestedApprovers.tsx create mode 100644 frontend/src/hooks/api/getters/useRequestedApprovers/useRequestedApprovers.ts diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx index 6f2ae5534b..c1e24c886e 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx @@ -34,6 +34,8 @@ import { ScheduleChangeRequestDialog } from './ChangeRequestScheduledDialogs/Sch import type { PlausibleChangeRequestState } from '../changeRequest.types'; import { useNavigate } from 'react-router-dom'; import { useActionableChangeRequests } from 'hooks/api/getters/useActionableChangeRequests/useActionableChangeRequests'; +import { useUiFlag } from 'hooks/useUiFlag.ts'; +import { ChangeRequestRequestedApprovers } from './ChangeRequestRequestedApprovers/ChangeRequestRequestedApprovers.tsx'; const StyledAsideBox = styled(Box)(({ theme }) => ({ width: '30%', @@ -106,6 +108,7 @@ export const ChangeRequestOverview: FC = () => { useChangeRequestsEnabled(projectId); const [disabled, setDisabled] = useState(false); const navigate = useNavigate(); + const approversEnabled = useUiFlag('changeRequestApproverEmails'); if (!changeRequest) { return null; @@ -288,7 +291,19 @@ export const ChangeRequestOverview: FC = () => { - + + } + elseShow={ + + } + /> diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestRequestedApprovers/ChangeRequestRequestedApprovers.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestRequestedApprovers/ChangeRequestRequestedApprovers.tsx new file mode 100644 index 0000000000..e11899a8b2 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestRequestedApprovers/ChangeRequestRequestedApprovers.tsx @@ -0,0 +1,372 @@ +import { + Box, + Paper, + styled, + IconButton, + useTheme, + type AutocompleteChangeReason, + type FilterOptionsState, + Checkbox, + TextField, + Button, + Typography, +} from '@mui/material'; +import { + type ReviewerSchema, + useRequestedApprovers, +} from 'hooks/api/getters/useRequestedApprovers/useRequestedApprovers'; +import { useState, type FC } from 'react'; +import type { ChangeRequestType } from '../../changeRequest.types'; +import { + ChangeRequestApprover, + ChangeRequestPending, + ChangeRequestRejector, +} from '../ChangeRequestReviewers/ChangeRequestReviewer.js'; +import Add from '@mui/icons-material/Add'; +import KeyboardArrowUp from '@mui/icons-material/KeyboardArrowUp'; +import AutocompleteVirtual from 'component/common/AutocompleteVirtual/AutcompleteVirtual.js'; +import { + type AvailableReviewerSchema, + useAvailableChangeRequestReviewers, +} from 'hooks/api/getters/useAvailableChangeRequestReviewers/useAvailableChangeRequestReviewers.js'; +import { caseInsensitiveSearch } from 'utils/search.js'; +import CheckBoxOutlineBlankIcon from '@mui/icons-material/CheckBoxOutlineBlank'; +import CheckBoxIcon from '@mui/icons-material/CheckBox'; +import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi.js'; + +export const StyledSpan = styled('span')(({ theme }) => ({ + display: 'flex', + alignItems: 'center', + marginRight: theme.spacing(1), + fontSize: theme.fontSizes.bodySize, +})); + +const StyledBox = styled(Box)(({ theme }) => ({ + display: 'flex', + flexDirection: 'row', + width: '100%', + '& > div': { width: '100%' }, + justifyContent: 'space-between', + marginBottom: theme.spacing(2), + marginRight: theme.spacing(-2), +})); + +const StyledIconButton = styled(IconButton)(({ theme }) => ({ + marginRight: theme.spacing(-1), +})); + +const StrechedLi = styled('li')({ width: '100%' }); + +const StyledOption = styled('div')(({ theme }) => ({ + display: 'flex', + flexDirection: 'column', + '& > span:first-of-type': { + color: theme.palette.text.secondary, + }, +})); + +const StyledTags = styled('div')(({ theme }) => ({ + paddingLeft: theme.spacing(1), +})); + +const renderOption = ( + props: React.HTMLAttributes, + option: AvailableReviewerSchema, + { selected }: { selected: boolean }, +) => ( + + } + checkedIcon={} + style={{ marginRight: 8 }} + checked={selected} + /> + + {option.name || option.username} + + {option.name && option.username + ? option.username + : option.email} + + + +); + +const renderTags = (value: AvailableReviewerSchema[]) => ( + + {value.length > 1 + ? `${value.length} reviewers` + : value[0].name || value[0].username || value[0].email} + +); + +export const ChangeRequestReviewersHeader: FC<{ + canShowAddReviewers: boolean; + showAddReviewers: boolean; + actualApprovals: number; + minApprovals: number; + setShowAddReviewers: React.Dispatch>; +}> = ({ + canShowAddReviewers, + showAddReviewers, + actualApprovals, + minApprovals, + setShowAddReviewers, +}) => { + return ( + <> + + + Reviewers + + ({actualApprovals}/{minApprovals} required) + + + {canShowAddReviewers && + (showAddReviewers ? ( + { + setShowAddReviewers(false); + }} + > + + + ) : ( + { + setShowAddReviewers(true); + }} + > + + + ))} + + + ); +}; + +export const ChangeRequestAddRequestedApprovers: FC<{ + changeRequest: Pick; + saveClicked: (reviewers: AvailableReviewerSchema[]) => void; + existingReviewers: Pick[]; +}> = ({ changeRequest, saveClicked, existingReviewers }) => { + const theme = useTheme(); + const [reviewers, setReviewers] = useState([]); + const { reviewers: fetchedReviewers, loading: isLoading } = + useAvailableChangeRequestReviewers( + changeRequest.project, + changeRequest.environment, + ); + const availableReviewers = fetchedReviewers.filter( + (reviewer) => + !existingReviewers.some((existing) => existing.id === reviewer.id), + ); + const autoCompleteChange = ( + event: React.SyntheticEvent, + newValue: AvailableReviewerSchema[], + reason: AutocompleteChangeReason, + ) => { + if ( + event.type === 'keydown' && + (event as React.KeyboardEvent).key === 'Backspace' && + reason === 'removeOption' + ) { + return; + } + setReviewers(newValue); + }; + + const filterOptions = ( + options: AvailableReviewerSchema[], + { inputValue }: FilterOptionsState, + ) => + options.filter( + ({ name, username, email }) => + caseInsensitiveSearch(inputValue, email) || + caseInsensitiveSearch(inputValue, name) || + caseInsensitiveSearch(inputValue, username), + ); + + return ( + + option.id === value.id} + getOptionLabel={(option: AvailableReviewerSchema) => + option.email || option.name || option.username || '' + } + renderInput={(params) => ( + + )} + renderTags={(value) => renderTags(value)} + noOptionsText={isLoading ? 'Loading…' : 'No options'} + /> + + + ); +}; + +export const ChangeRequestRequestedApprovers: FC<{ + changeRequest: Pick< + ChangeRequestType, + | 'id' + | 'project' + | 'approvals' + | 'rejections' + | 'state' + | 'minApprovals' + | 'environment' + >; +}> = ({ changeRequest }) => { + const [showAddReviewers, setShowAddReviewers] = useState(false); + const { reviewers: requestedReviewers, refetchReviewers } = + useRequestedApprovers(changeRequest.project, changeRequest.id); + const { updateRequestedApprovers } = useChangeRequestApi(); + const canShowAddReviewers = + (changeRequest.state === 'Draft' || + changeRequest.state === 'In review') && + changeRequest.minApprovals > 0; + + let reviewers = requestedReviewers.map((reviewer) => { + const approver = changeRequest.approvals.find( + (approval) => approval.createdBy.id === reviewer.id, + ); + const rejector = changeRequest.rejections.find( + (rejection) => rejection.createdBy.id === reviewer.id, + ); + + return { + id: reviewer.id, + name: reviewer.username || reviewer.name || 'Unknown user', + imageUrl: reviewer.imageUrl, + status: approver ? 'approved' : rejector ? 'rejected' : 'pending', + }; + }); + + reviewers = [ + ...reviewers, + ...changeRequest.approvals + .filter( + (approval) => + !reviewers.find( + (r) => r.name === approval.createdBy.username, + ), + ) + .map((approval) => ({ + id: approval.createdBy.id, + name: approval.createdBy.username || 'Unknown user', + imageUrl: approval.createdBy.imageUrl, + status: 'approved', + })), + ...changeRequest.rejections + .filter( + (rejection) => + !reviewers.find( + (r) => r.name === rejection.createdBy.username, + ), + ) + .map((rejection) => ({ + id: rejection.createdBy.id, + name: rejection.createdBy.username || 'Unknown user', + imageUrl: rejection.createdBy.imageUrl, + status: 'rejected', + })), + ]; + + const saveClicked = async ( + selectedReviewers: AvailableReviewerSchema[], + ) => { + if (selectedReviewers.length > 0) { + const tosend = [ + ...reviewers.map((reviewer) => reviewer.id), + ...selectedReviewers.map((reviewer) => reviewer.id), + ]; + await updateRequestedApprovers( + changeRequest.project, + changeRequest.id, + tosend, + ); + } + refetchReviewers(); + setShowAddReviewers(false); + }; + + return ( + ({ + marginTop: theme.spacing(2), + padding: theme.spacing(4), + paddingTop: theme.spacing(2), + borderRadius: (theme) => `${theme.shape.borderRadiusLarge}px`, + })} + > + + {canShowAddReviewers && showAddReviewers && ( + + )} + {reviewers.map((reviewer) => ( + <> + {reviewer.status === 'approved' && ( + + )} + {reviewer.status === 'rejected' && ( + + )} + {reviewer.status === 'pending' && ( + + )} + + ))} + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewer.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewer.tsx index 8c20bdbea9..c98794dce5 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewer.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewer.tsx @@ -3,6 +3,7 @@ import type { FC } from 'react'; import { StyledAvatar } from '../ChangeRequestHeader/ChangeRequestHeader.styles'; import CheckCircle from '@mui/icons-material/CheckCircle'; import Cancel from '@mui/icons-material/Cancel'; +import Pending from '@mui/icons-material/Pending'; interface IChangeRequestReviewerProps { name?: string; imageUrl?: string; @@ -26,6 +27,11 @@ export const StyledErrorIcon = styled(Cancel)(({ theme }) => ({ marginLeft: 'auto', })); +export const StyledPendingIcon = styled(Pending)(({ theme }) => ({ + color: theme.palette.neutral.main, + marginLeft: 'auto', +})); + export const ReviewerName = styled(Typography)({ maxWidth: '170px', textOverflow: 'ellipsis', @@ -39,7 +45,7 @@ export const ChangeRequestApprover: FC = ({ imageUrl, }) => { return ( - + {name} @@ -52,10 +58,23 @@ export const ChangeRequestRejector: FC = ({ imageUrl, }) => { return ( - + {name} ); }; + +export const ChangeRequestPending: FC = ({ + name, + imageUrl, +}) => { + return ( + + + {name} + + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/DraftChangeRequestActions/DraftChangeRequestActions.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/DraftChangeRequestActions/DraftChangeRequestActions.tsx index da1bb88b26..155c535058 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/DraftChangeRequestActions/DraftChangeRequestActions.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/DraftChangeRequestActions/DraftChangeRequestActions.tsx @@ -74,7 +74,7 @@ const renderOption = ( const renderTags = (value: AvailableReviewerSchema[]) => ( {value.length > 1 - ? `${value.length} users selected` + ? `${value.length} reviewers` : value[0].name || value[0].username || value[0].email} ); diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx index 3d48ecae1b..d57f103a38 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx @@ -74,7 +74,7 @@ export const EnvironmentChangeRequest: FC<{ const [commentText, setCommentText] = useState(''); const { user } = useAuthUser(); const [title, setTitle] = useState(environmentChangeRequest.title); - const { changeState, updateRequestedReviewers } = useChangeRequestApi(); + const { changeState, updateRequestedApprovers } = useChangeRequestApi(); const [reviewers, setReviewers] = useState([]); const [disabled, setDisabled] = useState(false); @@ -83,7 +83,7 @@ export const EnvironmentChangeRequest: FC<{ setDisabled(true); try { if (reviewers && reviewers.length > 0) { - await updateRequestedReviewers( + await updateRequestedApprovers( project, environmentChangeRequest.id, reviewers.map((reviewer) => reviewer.id), diff --git a/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts b/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts index a8cd0a0485..8863024ef6 100644 --- a/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts +++ b/frontend/src/hooks/api/actions/useChangeRequestApi/useChangeRequestApi.ts @@ -190,17 +190,17 @@ export const useChangeRequestApi = () => { return makeRequest(req.caller, req.id); }; - const updateRequestedReviewers = async ( + const updateRequestedApprovers = async ( project: string, changeRequestId: number, - reviewers: string[], + reviewers: number[], ) => { trackEvent('change_request', { props: { - eventType: 'reviewers updated', + eventType: 'approvers updated', }, }); - const path = `api/admin/projects/${project}/change-requests/${changeRequestId}/reviewers`; + const path = `api/admin/projects/${project}/change-requests/${changeRequestId}/approvers`; const req = createRequest(path, { method: 'PUT', body: JSON.stringify({ reviewers }), @@ -217,7 +217,7 @@ export const useChangeRequestApi = () => { discardDraft, addComment, updateTitle, - updateRequestedReviewers, + updateRequestedApprovers, errors, loading, }; diff --git a/frontend/src/hooks/api/getters/useAvailableChangeRequestReviewers/useAvailableChangeRequestReviewers.ts b/frontend/src/hooks/api/getters/useAvailableChangeRequestReviewers/useAvailableChangeRequestReviewers.ts index 00c83131d0..347c76db36 100644 --- a/frontend/src/hooks/api/getters/useAvailableChangeRequestReviewers/useAvailableChangeRequestReviewers.ts +++ b/frontend/src/hooks/api/getters/useAvailableChangeRequestReviewers/useAvailableChangeRequestReviewers.ts @@ -5,7 +5,7 @@ import handleErrorResponses from '../httpErrorResponseHandler.js'; // TODO: These will likely be created by Orval next time it is run export interface AvailableReviewerSchema { - id: string; + id: number; name?: string; email: string; username?: string; diff --git a/frontend/src/hooks/api/getters/useRequestedApprovers/useRequestedApprovers.ts b/frontend/src/hooks/api/getters/useRequestedApprovers/useRequestedApprovers.ts new file mode 100644 index 0000000000..be6488f098 --- /dev/null +++ b/frontend/src/hooks/api/getters/useRequestedApprovers/useRequestedApprovers.ts @@ -0,0 +1,48 @@ +import useSWR from 'swr'; +import { useMemo } from 'react'; +import { formatApiPath } from 'utils/formatPath'; +import handleErrorResponses from '../httpErrorResponseHandler.js'; + +// TODO: These will likely be created by Orval next time it is run +export interface ReviewerSchema { + id: number; + name?: string; + email: string; + username?: string; + imageUrl?: string; +} + +export interface IReviewersResponse { + reviewers: ReviewerSchema[]; + refetchReviewers: () => void; + loading: boolean; + error?: Error; +} + +export const useRequestedApprovers = ( + project: string, + changeRequestId: number, +): IReviewersResponse => { + const { data, error, mutate } = useSWR( + formatApiPath( + `api/admin/projects/${project}/change-requests/${changeRequestId}/approvers`, + ), + fetcher, + ); + + return useMemo( + () => ({ + reviewers: data?.reviewers || [], + loading: !error && !data, + refetchReviewers: () => mutate(), + error, + }), + [data, error, mutate], + ); +}; + +const fetcher = (path: string) => { + return fetch(path) + .then(handleErrorResponses('Requested Approvers')) + .then((res) => res.json()); +};