Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@cnasikas
cnasikas committed Oct 4, 2022
1 parent 727410f commit d96956b
Show file tree
Hide file tree
Showing 11 changed files with 353 additions and 344 deletions.
11 changes: 8 additions & 3 deletions x-pack/plugins/cases/public/components/actions/delete/use_delete_action.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,19 @@ import { useDeleteCases } from '../../../containers/use_delete_cases';

import * as i18n from './translations';
import { UseActionProps } from '../types';
import { useCasesContext } from '../../cases_context/use_cases_context';

const getDeleteActionTitle = (totalCases: number): string =>
totalCases > 1 ? i18n.BULK_ACTION_DELETE_LABEL : i18n.DELETE_ACTION_LABEL;

export const useDeleteAction = ({ onAction, onActionSuccess, isDisabled }: UseActionProps) => {
const euiTheme = useEuiTheme();
const { permissions } = useCasesContext();
const [isModalVisible, setIsModalVisible] = useState<boolean>(false);
const [caseToBeDeleted, setCaseToBeDeleted] = useState<Case[]>([]);
const canDelete = permissions.delete;
const isActionDisabled = isDisabled || !canDelete;

const onCloseModal = useCallback(() => setIsModalVisible(false), []);
const openModal = useCallback(
(selectedCases: Case[]) => {
Expand All @@ -43,20 +48,20 @@ export const useDeleteAction = ({ onAction, onActionSuccess, isDisabled }: UseAc
);
}, [deleteCases, onActionSuccess, onCloseModal, caseToBeDeleted]);

const color = isDisabled ? euiTheme.euiTheme.colors.disabled : 'danger';
const color = isActionDisabled ? euiTheme.euiTheme.colors.disabled : 'danger';

const getAction = (selectedCases: Case[]) => {
return {
name: <EuiTextColor color={color}>{getDeleteActionTitle(selectedCases.length)}</EuiTextColor>,
onClick: () => openModal(selectedCases),
disabled: isDisabled,
disabled: isActionDisabled,
'data-test-subj': 'cases-bulk-action-delete',
icon: <EuiIcon type="trash" size="m" color={color} />,
key: 'cases-bulk-action-delete',
};
};

return { getAction, isModalVisible, onConfirmDeletion, onCloseModal };
return { getAction, isModalVisible, onConfirmDeletion, onCloseModal, canDelete };
};

export type UseDeleteAction = ReturnType<typeof useDeleteAction>;
20 changes: 11 additions & 9 deletions x-pack/plugins/cases/public/components/actions/status/use_status_action.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Case, CaseStatuses } from '../../../../common';
import * as i18n from './translations';
import { UseActionProps } from '../types';
import { statuses } from '../../status';
import { useCasesContext } from '../../cases_context/use_cases_context';

const getStatusToasterMessage = (status: CaseStatuses, cases: Case[]): string => {
const totalCases = cases.length;
Expand All @@ -33,11 +34,8 @@ interface UseStatusActionProps extends UseActionProps {
selectedStatus?: CaseStatuses;
}

const getCasesWithChanges = (cases: Case[], status: CaseStatuses): Case[] =>
cases.filter((theCase) => theCase.status !== status);

const disableStatus = (cases: Case[], status: CaseStatuses) =>
getCasesWithChanges(cases, status).length === 0;
const shouldDisableStatus = (cases: Case[], status: CaseStatuses) =>
cases.every((theCase) => theCase.status === status);

export const useStatusAction = ({
onAction,
Expand All @@ -46,6 +44,9 @@ export const useStatusAction = ({
selectedStatus,
}: UseStatusActionProps) => {
const { mutate: updateCases } = useUpdateCases();
const { permissions } = useCasesContext();
const canUpdateStatus = permissions.update;
const isActionDisabled = isDisabled || !canUpdateStatus;

const handleUpdateCaseStatus = useCallback(
(selectedCases: Case[], status: CaseStatuses) => {
Expand Down Expand Up @@ -76,30 +77,31 @@ export const useStatusAction = ({
name: statuses[CaseStatuses.open].label,
icon: getStatusIcon(CaseStatuses.open),
onClick: () => handleUpdateCaseStatus(selectedCases, CaseStatuses.open),
disabled: isDisabled || disableStatus(selectedCases, CaseStatuses.open),
disabled: isActionDisabled || shouldDisableStatus(selectedCases, CaseStatuses.open),
'data-test-subj': 'cases-bulk-action-status-open',
key: 'cases-bulk-action-status-open',
},
{
name: statuses[CaseStatuses['in-progress']].label,
icon: getStatusIcon(CaseStatuses['in-progress']),
onClick: () => handleUpdateCaseStatus(selectedCases, CaseStatuses['in-progress']),
disabled: isDisabled || disableStatus(selectedCases, CaseStatuses['in-progress']),
disabled:
isActionDisabled || shouldDisableStatus(selectedCases, CaseStatuses['in-progress']),
'data-test-subj': 'cases-bulk-action-status-in-progress',
key: 'cases-bulk-action-status-in-progress',
},
{
name: statuses[CaseStatuses.closed].label,
icon: getStatusIcon(CaseStatuses.closed),
onClick: () => handleUpdateCaseStatus(selectedCases, CaseStatuses.closed),
disabled: isDisabled || disableStatus(selectedCases, CaseStatuses.closed),
disabled: isActionDisabled || shouldDisableStatus(selectedCases, CaseStatuses.closed),
'data-test-subj': 'cases-bulk-action-status-closed',
key: 'cases-bulk-status-action',
},
];
};

return { getActions };
return { getActions, canUpdateStatus };
};

export type UseStatusAction = ReturnType<typeof useStatusAction>;
76 changes: 23 additions & 53 deletions x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
import { useGetCasesMockState, connectorsMock } from '../../containers/mock';

import { StatusAll } from '../../../common/ui/types';
import { CaseSeverity, CaseStatuses } from '../../../common/api';
import { CaseStatuses } from '../../../common/api';
import { SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { getEmptyTagValue } from '../empty_value';
import { useKibana } from '../../common/lib/kibana';
Expand Down Expand Up @@ -360,60 +360,21 @@ describe('AllCasesListGeneric', () => {
});

it('should call onRowClick when clicking a case with modal=true', async () => {
const theCase = defaultGetCases.data.cases[0];

const wrapper = mount(
<TestProviders>
<AllCasesList isSelectorView={true} onRowClick={onRowClick} />
</TestProviders>
);

wrapper.find('[data-test-subj="cases-table-row-select-1"]').first().simulate('click');
wrapper
.find(`[data-test-subj="cases-table-row-select-${theCase.id}"]`)
.first()
.simulate('click');

await waitFor(() => {
expect(onRowClick).toHaveBeenCalledWith({
assignees: [{ uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }],
closedAt: null,
closedBy: null,
comments: [],
connector: { fields: null, id: '123', name: 'My Connector', type: '.jira' },
createdAt: '2020-02-19T23:06:33.798Z',
createdBy: {
email: '[email protected]',
fullName: 'Leslie Knope',
username: 'lknope',
},
description: 'Security banana Issue',
severity: CaseSeverity.LOW,
duration: null,
externalService: {
connectorId: '123',
connectorName: 'connector name',
externalId: 'external_id',
externalTitle: 'external title',
externalUrl: 'basicPush.com',
pushedAt: '2020-02-20T15:02:57.995Z',
pushedBy: {
email: '[email protected]',
fullName: 'Leslie Knope',
username: 'lknope',
},
},
id: '1',
owner: SECURITY_SOLUTION_OWNER,
status: 'open',
tags: ['coke', 'pepsi'],
title: 'Another horrible breach!!',
totalAlerts: 0,
totalComment: 0,
updatedAt: '2020-02-20T15:02:57.995Z',
updatedBy: {
email: '[email protected]',
fullName: 'Leslie Knope',
username: 'lknope',
},
version: 'WzQ3LDFd',
settings: {
syncAlerts: true,
},
});
expect(onRowClick).toHaveBeenCalledWith(theCase);
});
});

Expand Down Expand Up @@ -746,6 +707,10 @@ describe('AllCasesListGeneric', () => {
it('Renders bulk action', async () => {
const result = appMockRenderer.render(<AllCasesList />);

act(() => {
userEvent.click(result.getByTestId('checkboxSelectAll'));
});

act(() => {
userEvent.click(result.getByText('Bulk actions'));
});
Expand All @@ -760,10 +725,9 @@ describe('AllCasesListGeneric', () => {
'Bulk update status: %s',
async (status) => {
const result = appMockRenderer.render(<AllCasesList />);
const theCase = useGetCasesMockState.data.cases[0];

act(() => {
userEvent.click(result.getByTestId(`checkboxSelectRow-${theCase.id}`));
userEvent.click(result.getByTestId('checkboxSelectAll'));
});

act(() => {
Expand All @@ -787,7 +751,11 @@ describe('AllCasesListGeneric', () => {
await waitForComponentToUpdate();

expect(updateCasesSpy).toBeCalledWith(
[{ id: theCase.id, version: theCase.version, status }],
useGetCasesMockState.data.cases.map(({ id, version }) => ({
id,
version,
status,
})),
expect.anything()
);
}
Expand Down Expand Up @@ -857,7 +825,9 @@ describe('AllCasesListGeneric', () => {

it.each(statusTests)('update the status of a case: %s', async (status) => {
const res = appMockRenderer.render(<AllCasesList />);
const theCase = defaultGetCases.data.cases[0];
const openCase = useGetCasesMockState.data.cases[0];
const inProgressCase = useGetCasesMockState.data.cases[1];
const theCase = status === CaseStatuses.open ? inProgressCase : openCase;

await waitFor(() => {
expect(res.getByTestId(`case-action-popover-button-${theCase.id}`)).toBeInTheDocument();
Expand Down Expand Up @@ -887,7 +857,7 @@ describe('AllCasesListGeneric', () => {

await waitFor(() => {
expect(updateCasesSpy).toHaveBeenCalledWith(
[{ id: 'basic-case-id', status, version: 'WzQ3LDFd' }],
[{ id: theCase.id, status, version: theCase.version }],
expect.anything()
);
});
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/public/components/all_cases/table.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const CasesTable: FunctionComponent<CasesTableProps> = ({
) : (
<Div data-test-subj={isCasesLoading ? 'cases-table-loading' : null}>
<CasesTableUtilityBar
isSelectorView={isSelectorView}
totalCases={data.total ?? 0}
selectedCases={selectedCases}
deselectCases={deselectCases}
Expand Down
27 changes: 19 additions & 8 deletions x-pack/plugins/cases/public/components/all_cases/use_actions.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
noDeleteCasesPermissions,
onlyDeleteCasesPermission,
allCasesPermissions,
readCasesPermissions,
} from '../../common/mock';

jest.mock('../../containers/api');
Expand Down Expand Up @@ -51,7 +52,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

expect(res.getByTestId(`case-action-popover-${basicCase.id}`)).toBeInTheDocument();
Expand All @@ -62,14 +63,15 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
userEvent.click(res.getByTestId(`case-action-popover-button-${basicCase.id}`));
});

await waitFor(() => {
expect(res.getByText('Actions')).toBeInTheDocument();
expect(res.getByTestId(`case-action-status-panel-${basicCase.id}`)).toBeInTheDocument();
expect(res.getByTestId('cases-bulk-action-delete')).toBeInTheDocument();
});
Expand All @@ -82,7 +84,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand Down Expand Up @@ -120,7 +122,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand Down Expand Up @@ -155,7 +157,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand Down Expand Up @@ -193,7 +195,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand All @@ -213,7 +215,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand All @@ -233,7 +235,7 @@ describe('useActions', () => {
wrapper: appMockRender.AppWrapper,
});

const comp = result.current.actions.render(basicCase) as React.ReactElement;
const comp = result.current.actions!.render(basicCase) as React.ReactElement;
const res = appMockRender.render(comp);

act(() => {
Expand All @@ -246,5 +248,14 @@ describe('useActions', () => {
expect(res.queryByTestId(`actions-separator-${basicCase.id}`)).toBeFalsy();
});
});

it('returns null if the user does not have update or delete permissions', async () => {
appMockRender = createAppMockRenderer({ permissions: readCasesPermissions() });
const { result } = renderHook(() => useActions(), {
wrapper: appMockRender.AppWrapper,
});

expect(result.current.actions).toBe(null);
});
});
});
Loading

0 comments on commit d96956b

Please sign in to comment.