Skip to content

Commit

Permalink
Fixing fallout from migration to paginated entity table component. (#…
Browse files Browse the repository at this point in the history
…19639)

* Fix refetching dashboards after deletion.

* Allow supplying a placeholder for the search form.

* Adding test case.

* Adding default prop.

* Adjusting test.
  • Loading branch information
dennisoelkers authored Jun 16, 2024
1 parent 8a81246 commit 0151825
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,24 @@ const SearchRow = styled.div`
align-items: center;
`;

type EntityDataTableProps = React.ComponentProps<typeof EntityDataTable>;

type Props<T> = {
actionsCellWidth?: React.ComponentProps<typeof EntityDataTable>['actionsCellWidth'],
bulkSelection?: React.ComponentProps<typeof EntityDataTable>['bulkSelection'],
columnRenderers: React.ComponentProps<typeof EntityDataTable>['columnRenderers'],
columnsOrder: React.ComponentProps<typeof EntityDataTable>['columnsOrder'],
entityActions: React.ComponentProps<typeof EntityDataTable>['entityActions'],
expandedSectionsRenderer?: React.ComponentProps<typeof EntityDataTable>['expandedSectionsRenderer'],
actionsCellWidth?: EntityDataTableProps['actionsCellWidth'],
additionalAttributes?: Array<Attribute>,
bulkSelection?: EntityDataTableProps['bulkSelection'],
columnRenderers: EntityDataTableProps['columnRenderers'],
columnsOrder: EntityDataTableProps['columnsOrder'],
entityActions: EntityDataTableProps['entityActions'],
entityAttributesAreCamelCase: boolean,
expandedSectionsRenderer?: EntityDataTableProps['expandedSectionsRenderer'],
fetchEntities: (options: SearchParams) => Promise<PaginatedResponse<T>>,
filterValueRenderers?: React.ComponentProps<typeof EntityFilters>['filterValueRenderers'],
humanName: string,
keyFn: (options: SearchParams) => Array<unknown>,
queryHelpComponent?: React.ReactNode,
searchPlaceholder?: string,
tableLayout: Parameters<typeof useTableLayout>[0],
additionalAttributes?: Array<Attribute>,
entityAttributesAreCamelCase: boolean,
topRightCol?: React.ReactNode,
}

Expand All @@ -75,7 +78,7 @@ const PaginatedEntityTable = <T extends EntityBase>({
actionsCellWidth, columnsOrder, entityActions, tableLayout, fetchEntities, keyFn,
humanName, columnRenderers, queryHelpComponent, filterValueRenderers,
expandedSectionsRenderer, bulkSelection, additionalAttributes,
entityAttributesAreCamelCase, topRightCol,
entityAttributesAreCamelCase, topRightCol, searchPlaceholder,
}: Props<T>) => {
const [urlQueryFilters, setUrlQueryFilters] = useUrlQueryFilters();
const [query, setQuery] = useQueryParam('query', StringParam);
Expand Down Expand Up @@ -135,6 +138,7 @@ const PaginatedEntityTable = <T extends EntityBase>({
<SearchForm onSearch={onSearch}
onReset={onSearchReset}
query={query}
placeholder={searchPlaceholder ?? `Search for ${humanName}`}
queryHelpComponent={queryHelpComponent}>
<div style={{ marginBottom: 5 }}>
<EntityFilters attributes={attributes}
Expand Down Expand Up @@ -179,6 +183,7 @@ PaginatedEntityTable.defaultProps = {
filterValueRenderers: undefined,
queryHelpComponent: undefined,
topRightCol: undefined,
searchPlaceholder: undefined,
};

export default PaginatedEntityTable;
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ const ProfilesList = () => {
keyFn={keyFn}
entityAttributesAreCamelCase
expandedSectionsRenderer={expandedSectionsRenderer}
columnRenderers={customColumnRenderers} />
columnRenderers={customColumnRenderers}
searchPlaceholder="Search for profile name" />
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import Immutable from 'immutable';

import { asMock } from 'helpers/mocking';
import { ViewManagementActions } from 'views/stores/ViewManagementStore';
import DashboardActions from 'views/components/dashboard/DashboardsOverview/DashboardActions';
import OriginalDashboardActions from 'views/components/dashboard/DashboardsOverview/DashboardActions';
import { simpleView } from 'views/test/ViewFixtures';
import useCurrentUser from 'hooks/useCurrentUser';
import { adminUser } from 'fixtures/users';
import useSelectedEntities from 'components/common/EntityDataTable/hooks/useSelectedEntities';
import type { ContextValue } from 'components/common/PaginatedEntityTable/TableFetchContext';
import TableFetchContext from 'components/common/PaginatedEntityTable/TableFetchContext';

jest.mock('hooks/useCurrentUser');
jest.mock('components/common/EntityDataTable/hooks/useSelectedEntities');
Expand All @@ -38,6 +40,28 @@ jest.mock('views/stores/ViewManagementStore', () => ({
},
}));

const mockSearchParams = {
page: 1,
pageSize: 10,
query: '',
sort: {
attributeId: 'name',
direction: 'asc',
},
} as const;

const mockContextValue = { searchParams: mockSearchParams, refetch: jest.fn(), attributes: [] };

const DashboardActions = ({ contextValue, ...props }: React.ComponentProps<typeof OriginalDashboardActions> & { contextValue?: ContextValue }) => (
<TableFetchContext.Provider value={contextValue ?? mockContextValue}>
<OriginalDashboardActions {...props} />
</TableFetchContext.Provider>
);

DashboardActions.defaultProps = {
contextValue: undefined,
};

describe('DashboardActions', () => {
let oldWindowConfirm;

Expand Down Expand Up @@ -71,7 +95,7 @@ describe('DashboardActions', () => {
it('does not delete dashboard when user clicks cancel', async () => {
asMock(window.confirm).mockReturnValue(false);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await clickDashboardAction('Delete');

Expand All @@ -83,7 +107,7 @@ describe('DashboardActions', () => {
it('deletes dashboard when user confirms deletion', async () => {
asMock(window.confirm).mockReturnValue(true);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await clickDashboardAction('Delete');

Expand All @@ -96,7 +120,7 @@ describe('DashboardActions', () => {
const currentUser = adminUser.toBuilder().permissions(Immutable.List([`view:read:${simpleDashboard.id}`])).build();
asMock(useCurrentUser).mockReturnValue(currentUser);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await screen.findByRole('button', { name: /share/i });

Expand All @@ -123,7 +147,7 @@ describe('DashboardActions', () => {
});

it('triggers hook when deleting dashboard', async () => {
render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await clickDashboardAction('Delete');

Expand All @@ -132,10 +156,24 @@ describe('DashboardActions', () => {
expect(deletingDashboard).toHaveBeenCalledWith(simpleDashboard);
});

it('refreshes dashboard list after deletion', async () => {
const contextValue = {
...mockContextValue,
refetch: jest.fn(),
};
render(<DashboardActions dashboard={simpleDashboard} contextValue={contextValue} />);

await clickDashboardAction('Delete');

await waitFor(() => expect(ViewManagementActions.delete).toHaveBeenCalledWith(expect.objectContaining({ id: 'foo' })));

expect(contextValue.refetch).toHaveBeenCalled();
});

it('does not delete dashboard when hook returns false', async () => {
asMock(deletingDashboard).mockResolvedValue(false);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await clickDashboardAction('Delete');

Expand All @@ -148,7 +186,7 @@ describe('DashboardActions', () => {
asMock(deletingDashboard).mockReturnValue(null);
asMock(window.confirm).mockReturnValue(true);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

await clickDashboardAction('Delete');

Expand All @@ -164,7 +202,7 @@ describe('DashboardActions', () => {
asMock(deletingDashboard).mockImplementation(() => { throw error; });
asMock(window.confirm).mockReturnValue(true);

render(<DashboardActions dashboard={simpleDashboard} refetchDashboards={() => Promise.resolve()} />);
render(<DashboardActions dashboard={simpleDashboard} />);

/* eslint-disable no-console */
const oldConsoleTrace = console.trace;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type FetchError from 'logic/errors/FetchError';
import { isAnyPermitted } from 'util/PermissionsMixin';
import useCurrentUser from 'hooks/useCurrentUser';
import { MoreActions } from 'components/common/EntityDataTable';
import { useTableFetchContext } from 'components/common/PaginatedEntityTable';

// eslint-disable-next-line no-alert
const defaultDashboardDeletionHook = async (view: View) => window.confirm(`Are you sure you want to delete "${view.title}"?`);
Expand All @@ -44,7 +45,6 @@ const _extractErrorMessage = (error: FetchError) => ((error

type Props = {
dashboard: View,
refetchDashboards: () => void,
isEvidenceModal?: boolean,
}

Expand Down Expand Up @@ -107,16 +107,17 @@ DashboardDeleteAction.defaultProps = {
isEvidenceModal: false,
};

const DashboardActions = ({ dashboard, refetchDashboards, isEvidenceModal }: Props) => {
const DashboardActions = ({ dashboard, isEvidenceModal }: Props) => {
const [showShareModal, setShowShareModal] = useState(false);
const { actions: pluggableActions, actionModals: pluggableActionModals } = usePluggableDashboardActions(dashboard);
const currentUser = useCurrentUser();
const { refetch } = useTableFetchContext();

const moreActions = [
pluggableActions.length ? pluggableActions : null,
pluggableActions.length && !isEvidenceModal ? <MenuItem divider key="divider" /> : null,
isAnyPermitted(currentUser.permissions, [`view:edit:${dashboard.id}`, 'view:edit'])
? <DashboardDeleteAction dashboard={dashboard} refetchDashboards={refetchDashboards} key="delete-action" isEvidenceModal={isEvidenceModal} />
? <DashboardDeleteAction dashboard={dashboard} refetchDashboards={refetch} key="delete-action" isEvidenceModal={isEvidenceModal} />
: null,
].filter(Boolean);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe('DashboardsOverview', () => {

render(<SUT />);

const searchInput = await screen.findByPlaceholderText('Enter search query...');
const searchInput = await screen.findByPlaceholderText('Search for dashboards');

expect(searchInput).toHaveValue('example query');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const DashboardsOverview = ({ isEvidenceModal }: Props) => {
const customColumnRenderers = useColumnRenderers();

const renderDashboardActions = useCallback((dashboard: View) => (
<DashboardActions dashboard={dashboard} refetchDashboards={() => {}} isEvidenceModal={isEvidenceModal} />
<DashboardActions dashboard={dashboard} isEvidenceModal={isEvidenceModal} />
), [isEvidenceModal]);

return (
Expand Down

0 comments on commit 0151825

Please sign in to comment.