1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-04-24 01:18:01 +02:00

fix: generalize multi action button (#6294)

This PR moves the CR specific logic out of the MultiActionButton and
generalises so that we can re-use it across the application. The CR
specific logic is moved into:

* ApplyButton.tsx
* ReviewButton.tsx

This fixes a bug where multi action button would be disabled if you
tried to apply an approved change request that you had created yourself.
This commit is contained in:
Fredrik Strand Oseberg 2024-02-21 14:37:35 +01:00 committed by GitHub
parent ac183e76f8
commit 0ccfc29e26
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 138 additions and 64 deletions

View File

@ -2,34 +2,45 @@ import { FC } from 'react';
import CheckBox from '@mui/icons-material/Check'; import CheckBox from '@mui/icons-material/Check';
import Today from '@mui/icons-material/Today'; import Today from '@mui/icons-material/Today';
import { MultiActionButton } from '../MultiActionButton/MultiActionButton';
import { APPLY_CHANGE_REQUEST } from 'component/providers/AccessProvider/permissions'; import { APPLY_CHANGE_REQUEST } from 'component/providers/AccessProvider/permissions';
import { MultiActionButton } from 'component/common/MultiActionButton/MultiActionButton';
import { useChangeRequest } from 'hooks/api/getters/useChangeRequest/useChangeRequest';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
export const ApplyButton: FC<{ export const ApplyButton: FC<{
disabled: boolean; disabled: boolean;
onSchedule: () => void; onSchedule: () => void;
onApply: () => void; onApply: () => void;
variant?: 'create' | 'update'; variant?: 'create' | 'update';
}> = ({ disabled, onSchedule, onApply, variant = 'create', children }) => ( }> = ({ disabled, onSchedule, onApply, variant = 'create', children }) => {
<MultiActionButton const projectId = useRequiredPathParam('projectId');
permission={APPLY_CHANGE_REQUEST} const id = useRequiredPathParam('id');
disabled={disabled} const { data } = useChangeRequest(projectId, id);
actions={[
{ return (
label: 'Apply changes', <MultiActionButton
onSelect: onApply, permission={APPLY_CHANGE_REQUEST}
icon: <CheckBox fontSize='small' />, disabled={disabled}
}, actions={[
{ {
label: label: 'Apply changes',
variant === 'create' onSelect: onApply,
? 'Schedule changes' icon: <CheckBox fontSize='small' />,
: 'Update schedule', },
onSelect: onSchedule, {
icon: <Today fontSize='small' />, label:
}, variant === 'create'
]} ? 'Schedule changes'
> : 'Update schedule',
{children} onSelect: onSchedule,
</MultiActionButton> icon: <Today fontSize='small' />,
); },
]}
environmentId={data?.environment}
projectId={projectId}
ariaLabel='apply or schedule changes'
>
{children}
</MultiActionButton>
);
};

View File

@ -126,11 +126,17 @@ const changeRequestConfig = () =>
'get', 'get',
); );
const setupChangeRequest = (featureName: string, state: ChangeRequestState) => { const setupChangeRequest = (
pendingChangeRequest(mockChangeRequest(featureName, state)); featureName: string,
changeRequest(mockChangeRequest(featureName, state)); state: ChangeRequestState,
overrides: Partial<Omit<ChangeRequestType, 'state' | 'schedule'>> = {},
) => {
pendingChangeRequest({
...mockChangeRequest(featureName, state),
...overrides,
});
changeRequest({ ...mockChangeRequest(featureName, state), ...overrides });
}; };
const uiConfig = () => { const uiConfig = () => {
testServerRoute(server, '/api/admin/ui-config', { testServerRoute(server, '/api/admin/ui-config', {
versionInfo: { versionInfo: {
@ -249,6 +255,44 @@ test('should show a reschedule dialog when change request is scheduled and updat
await screen.findByRole('dialog', { name: 'Update schedule' }); await screen.findByRole('dialog', { name: 'Update schedule' });
}); });
test('should be allowed to apply your own change request if it is approved', async () => {
setupChangeRequest(featureName, 'Approved', {
createdBy: {
id: 17,
imageUrl:
'https://gravatar.com/avatar/21232f297a57a5a743894a0e4a801fc3?size=42&default=retro',
},
});
render(<Component />, {
route: '/projects/default/change-requests/1',
permissions: [
{
permission: APPLY_CHANGE_REQUEST,
project: 'default',
environment: 'production',
},
],
});
const applyOrScheduleButton = await screen.findByText(
'Apply or schedule changes',
);
await waitFor(() => expect(applyOrScheduleButton).toBeEnabled(), {
timeout: 3000,
});
fireEvent.click(applyOrScheduleButton);
const scheduleChangesButton = await screen.findByRole('menuitem', {
name: 'Schedule changes',
});
fireEvent.click(scheduleChangesButton);
await screen.findByRole('dialog', { name: 'Schedule changes' });
});
test('should show an apply dialog when change request is scheduled and apply is selected', async () => { test('should show an apply dialog when change request is scheduled and apply is selected', async () => {
setupChangeRequest(featureName, 'Scheduled'); setupChangeRequest(featureName, 'Scheduled');

View File

@ -1,31 +1,49 @@
import { FC } from 'react'; import { FC, useContext } from 'react';
import CheckBox from '@mui/icons-material/Check'; import CheckBox from '@mui/icons-material/Check';
import Clear from '@mui/icons-material/Clear'; import Clear from '@mui/icons-material/Clear';
import { MultiActionButton } from '../MultiActionButton/MultiActionButton'; import { MultiActionButton } from 'component/common/MultiActionButton/MultiActionButton';
import { APPROVE_CHANGE_REQUEST } from 'component/providers/AccessProvider/permissions'; import { APPROVE_CHANGE_REQUEST } from 'component/providers/AccessProvider/permissions';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
import { useAuthUser } from 'hooks/api/getters/useAuth/useAuthUser';
import AccessContext from 'contexts/AccessContext';
import { useChangeRequest } from 'hooks/api/getters/useChangeRequest/useChangeRequest';
export const ReviewButton: FC<{ export const ReviewButton: FC<{
disabled: boolean; disabled: boolean;
onReject: () => void; onReject: () => void;
onApprove: () => void; onApprove: () => void;
}> = ({ disabled, onReject, onApprove, children }) => ( }> = ({ disabled, onReject, onApprove, children }) => {
<MultiActionButton const { isAdmin } = useContext(AccessContext);
permission={APPROVE_CHANGE_REQUEST} const projectId = useRequiredPathParam('projectId');
disabled={disabled} const id = useRequiredPathParam('id');
actions={[ const { user } = useAuthUser();
{ const { data } = useChangeRequest(projectId, id);
label: 'Approve',
onSelect: onApprove, const approverIsCreator = data?.createdBy.id === user?.id;
icon: <CheckBox fontSize='small' />, const disableApprove = disabled || (approverIsCreator && !isAdmin);
},
{ return (
label: 'Reject', <MultiActionButton
onSelect: onReject, permission={APPROVE_CHANGE_REQUEST}
icon: <Clear fontSize='small' />, disabled={disableApprove}
}, actions={[
]} {
> label: 'Approve',
{children} onSelect: onApprove,
</MultiActionButton> icon: <CheckBox fontSize='small' />,
); },
{
label: 'Reject',
onSelect: onReject,
icon: <Clear fontSize='small' />,
},
]}
environmentId={data?.environment}
projectId={projectId}
ariaLabel='review or reject changes'
>
{children}
</MultiActionButton>
);
};

View File

@ -15,8 +15,6 @@ import {
import ArrowDropDownIcon from '@mui/icons-material/ArrowDropDown'; import ArrowDropDownIcon from '@mui/icons-material/ArrowDropDown';
import PermissionButton from 'component/common/PermissionButton/PermissionButton'; import PermissionButton from 'component/common/PermissionButton/PermissionButton';
import { useAuthUser } from 'hooks/api/getters/useAuth/useAuthUser';
import AccessContext from 'contexts/AccessContext';
type Action = { type Action = {
label: string; label: string;
@ -28,13 +26,18 @@ export const MultiActionButton: FC<{
disabled: boolean; disabled: boolean;
actions: Action[]; actions: Action[];
permission: string; permission: string;
}> = ({ disabled, children, actions, permission }) => { projectId?: string;
const { isAdmin } = useContext(AccessContext); environmentId?: string;
const projectId = useRequiredPathParam('projectId'); ariaLabel?: string;
const id = useRequiredPathParam('id'); }> = ({
const { user } = useAuthUser(); disabled,
const { data } = useChangeRequest(projectId, id); children,
actions,
permission,
projectId,
ariaLabel,
environmentId,
}) => {
const [open, setOpen] = React.useState(false); const [open, setOpen] = React.useState(false);
const anchorRef = React.useRef<HTMLButtonElement>(null); const anchorRef = React.useRef<HTMLButtonElement>(null);
@ -57,19 +60,17 @@ export const MultiActionButton: FC<{
<React.Fragment> <React.Fragment>
<PermissionButton <PermissionButton
variant='contained' variant='contained'
disabled={ disabled={disabled}
disabled || (data?.createdBy.id === user?.id && !isAdmin)
}
aria-controls={open ? 'review-options-menu' : undefined} aria-controls={open ? 'review-options-menu' : undefined}
aria-expanded={open ? 'true' : undefined} aria-expanded={open ? 'true' : undefined}
aria-label='review changes' aria-label={ariaLabel}
aria-haspopup='menu' aria-haspopup='menu'
onClick={onToggle} onClick={onToggle}
ref={anchorRef} ref={anchorRef}
endIcon={<ArrowDropDownIcon />} endIcon={<ArrowDropDownIcon />}
permission={permission} permission={permission}
projectId={projectId} projectId={projectId}
environmentId={data?.environment} environmentId={environmentId}
> >
{children} {children}
</PermissionButton> </PermissionButton>