Skip to content

Commit bdc99fd

Browse files
authored
fix: clear selection on files & uploads page after deleting (backport) (#2228)
* refactor: remove selected rows when deleting or adding elements * refactor: ensure unique asset IDs when adding new ones * refactor: remove unnecessary loading checks in mockStore function * test: add unit tests for TableActions component
1 parent 92c59cb commit bdc99fd

File tree

6 files changed

+167
-23
lines changed

6 files changed

+167
-23
lines changed

src/files-and-videos/files-page/FilesPage.test.jsx

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,6 @@ const mockStore = async (
7070
}
7171
renderComponent();
7272
await executeThunk(fetchAssets(courseId), store.dispatch);
73-
74-
// Finish loading the expected files into the data table before returning,
75-
// because loading new files can disrupt things like accessing file menus.
76-
if (status === RequestStatus.SUCCESSFUL) {
77-
const numFiles = skipNextPageFetch ? 13 : 15;
78-
await waitFor(() => {
79-
expect(screen.getByText(`Showing ${numFiles} of ${numFiles}`)).toBeInTheDocument();
80-
});
81-
}
8273
};
8374

8475
const emptyMockStore = async (status) => {

src/files-and-videos/files-page/data/slice.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const slice = createSlice({
2828
if (isEmpty(state.assetIds)) {
2929
state.assetIds = payload.assetIds;
3030
} else {
31-
state.assetIds = [...state.assetIds, ...payload.assetIds];
31+
state.assetIds = [...new Set([...state.assetIds, ...payload.assetIds])];
3232
}
3333
},
3434
setSortedAssetIds: (state, { payload }) => {

src/files-and-videos/generic/DeleteConfirmationModal.jsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { useContext } from 'react';
22
import PropTypes from 'prop-types';
33
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
44
import { getConfig } from '@edx/frontend-platform';
@@ -7,6 +7,7 @@ import {
77
AlertModal,
88
Button,
99
Collapsible,
10+
DataTableContext,
1011
Hyperlink,
1112
Truncate,
1213
} from '@openedx/paragon';
@@ -22,6 +23,13 @@ const DeleteConfirmationModal = ({
2223
// injected
2324
intl,
2425
}) => {
26+
const { clearSelection } = useContext(DataTableContext);
27+
28+
const handleConfirmDeletion = () => {
29+
handleBulkDelete();
30+
clearSelection();
31+
};
32+
2533
const firstSelectedRow = selectedRows[0]?.original;
2634
let activeContentRows = [];
2735
if (Array.isArray(selectedRows)) {
@@ -73,7 +81,7 @@ const DeleteConfirmationModal = ({
7381
<Button variant="tertiary" onClick={closeDeleteConfirmation}>
7482
{intl.formatMessage(messages.cancelButtonLabel)}
7583
</Button>
76-
<Button onClick={handleBulkDelete}>
84+
<Button onClick={handleConfirmDeletion}>
7785
{intl.formatMessage(messages.deleteFileButtonLabel)}
7886
</Button>
7987
</ActionRow>

src/files-and-videos/generic/FileTable.jsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,16 @@ const FileTable = ({
273273
setSelectedRows={setSelectedRows}
274274
fileType={fileType}
275275
/>
276+
277+
<DeleteConfirmationModal
278+
{...{
279+
isDeleteConfirmationOpen,
280+
closeDeleteConfirmation,
281+
handleBulkDelete,
282+
selectedRows,
283+
fileType,
284+
}}
285+
/>
276286
</DataTable>
277287
<FileInput key="generic-file-upload" fileInput={fileInputControl} supportedFileFormats={supportedFileFormats} />
278288
{!isEmpty(selectedRows) && (
@@ -286,15 +296,7 @@ const FileTable = ({
286296
sidebar={infoModalSidebar}
287297
/>
288298
)}
289-
<DeleteConfirmationModal
290-
{...{
291-
isDeleteConfirmationOpen,
292-
closeDeleteConfirmation,
293-
handleBulkDelete,
294-
selectedRows,
295-
fileType,
296-
}}
297-
/>
299+
298300
</div>
299301
);
300302
};

src/files-and-videos/generic/table-components/TableActions.jsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,18 @@ const TableActions = ({
2626
intl,
2727
}) => {
2828
const [isSortOpen, openSort, closeSort] = useToggle(false);
29-
const { state } = useContext(DataTableContext);
29+
const { state, clearSelection } = useContext(DataTableContext);
3030

3131
// This useEffect saves DataTable state so it can persist after table re-renders due to data reload.
3232
useEffect(() => {
3333
setInitialState(state);
3434
}, [state]);
3535

36+
const handleOpenFileSelector = () => {
37+
fileInputControl.click();
38+
clearSelection();
39+
};
40+
3641
return (
3742
<>
3843
<Button variant="outline-primary" onClick={openSort} iconBefore={Tune}>
@@ -71,7 +76,7 @@ const TableActions = ({
7176
</Dropdown.Item>
7277
</Dropdown.Menu>
7378
</Dropdown>
74-
<Button iconBefore={Add} onClick={fileInputControl.click}>
79+
<Button iconBefore={Add} onClick={handleOpenFileSelector}>
7580
{intl.formatMessage(messages.addFilesButtonLabel, { fileType })}
7681
</Button>
7782
<SortAndFilterModal {...{ isSortOpen, closeSort, handleSort }} />
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import React from 'react';
2+
import { screen, fireEvent } from '@testing-library/react';
3+
import { DataTableContext } from '@openedx/paragon';
4+
import { initializeMocks, render } from '../../../testUtils';
5+
import TableActions from './TableActions';
6+
import messages from '../messages';
7+
8+
const defaultProps = {
9+
selectedFlatRows: [],
10+
fileInputControl: { click: jest.fn() },
11+
handleOpenDeleteConfirmation: jest.fn(),
12+
handleBulkDownload: jest.fn(),
13+
encodingsDownloadUrl: null,
14+
handleSort: jest.fn(),
15+
fileType: 'video',
16+
setInitialState: jest.fn(),
17+
intl: {
18+
formatMessage: (msg, values) => msg.defaultMessage.replace('{fileType}', values?.fileType ?? ''),
19+
},
20+
};
21+
22+
const mockColumns = [
23+
{
24+
id: 'wrapperType',
25+
Header: 'Type',
26+
accessor: 'wrapperType',
27+
filter: 'includes',
28+
},
29+
];
30+
31+
const renderWithContext = (props = {}, contextOverrides = {}) => {
32+
const contextValue = {
33+
state: {
34+
selectedRowIds: {},
35+
filters: [],
36+
...contextOverrides.state,
37+
},
38+
clearSelection: jest.fn(),
39+
gotoPage: jest.fn(),
40+
setAllFilters: jest.fn(),
41+
columns: mockColumns,
42+
...contextOverrides,
43+
};
44+
45+
return render(
46+
<DataTableContext.Provider value={contextValue}>
47+
<TableActions {...defaultProps} {...props} />
48+
</DataTableContext.Provider>,
49+
);
50+
};
51+
52+
describe('TableActions', () => {
53+
beforeEach(() => {
54+
initializeMocks();
55+
jest.clearAllMocks();
56+
});
57+
58+
test('renders buttons and dropdown', () => {
59+
renderWithContext();
60+
61+
expect(screen.getByRole('button', { name: messages.sortButtonLabel.defaultMessage })).toBeInTheDocument();
62+
expect(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage })).toBeInTheDocument();
63+
expect(screen.getByRole('button', { name: messages.addFilesButtonLabel.defaultMessage.replace('{fileType}', 'video') })).toBeInTheDocument();
64+
});
65+
66+
test('disables bulk and delete actions if no rows selected', () => {
67+
renderWithContext();
68+
69+
fireEvent.click(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage }));
70+
71+
const downloadOption = screen.getByText(messages.downloadTitle.defaultMessage);
72+
const deleteButton = screen.getByTestId('open-delete-confirmation-button');
73+
74+
expect(downloadOption).toHaveAttribute('aria-disabled', 'true');
75+
expect(downloadOption).toHaveClass('disabled');
76+
77+
expect(deleteButton).toHaveAttribute('aria-disabled', 'true');
78+
expect(deleteButton).toHaveClass('disabled');
79+
});
80+
81+
test('enables bulk and delete actions when rows are selected', () => {
82+
renderWithContext({
83+
selectedFlatRows: [{ original: { id: '1', displayName: 'Video 1', wrapperType: 'video' } }],
84+
});
85+
86+
fireEvent.click(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage }));
87+
expect(screen.getByText(messages.downloadTitle.defaultMessage)).not.toBeDisabled();
88+
expect(screen.getByTestId('open-delete-confirmation-button')).not.toBeDisabled();
89+
});
90+
91+
test('calls file input click and clears selection when add button clicked', () => {
92+
const mockClick = jest.fn();
93+
const mockClear = jest.fn();
94+
95+
renderWithContext({ fileInputControl: { click: mockClick } }, {}, mockClear);
96+
fireEvent.click(screen.getByRole('button', { name: messages.addFilesButtonLabel.defaultMessage.replace('{fileType}', 'video') }));
97+
expect(mockClick).toHaveBeenCalled();
98+
});
99+
100+
test('opens sort modal when sort button clicked', () => {
101+
renderWithContext();
102+
fireEvent.click(screen.getByRole('button', { name: messages.sortButtonLabel.defaultMessage }));
103+
expect(screen.getByRole('dialog')).toBeInTheDocument();
104+
});
105+
106+
test('calls handleBulkDownload when selected and clicked', () => {
107+
const handleBulkDownload = jest.fn();
108+
renderWithContext({
109+
selectedFlatRows: [{ original: { id: '1', displayName: 'Video 1', wrapperType: 'video' } }],
110+
handleBulkDownload,
111+
});
112+
113+
fireEvent.click(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage }));
114+
fireEvent.click(screen.getByText(messages.downloadTitle.defaultMessage));
115+
expect(handleBulkDownload).toHaveBeenCalled();
116+
});
117+
118+
test('calls handleOpenDeleteConfirmation when clicked', () => {
119+
const handleOpenDeleteConfirmation = jest.fn();
120+
const selectedFlatRows = [{ original: { id: '1', displayName: 'Video 1', wrapperType: 'video' } }];
121+
renderWithContext({
122+
selectedFlatRows,
123+
handleOpenDeleteConfirmation,
124+
});
125+
126+
fireEvent.click(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage }));
127+
fireEvent.click(screen.getByTestId('open-delete-confirmation-button'));
128+
expect(handleOpenDeleteConfirmation).toHaveBeenCalledWith(selectedFlatRows);
129+
});
130+
131+
test('shows encoding download link when provided', () => {
132+
const encodingsDownloadUrl = '/some/path/to/encoding.zip';
133+
renderWithContext({ encodingsDownloadUrl });
134+
135+
fireEvent.click(screen.getByRole('button', { name: messages.actionsButtonLabel.defaultMessage }));
136+
expect(screen.getByRole('link', { name: messages.downloadEncodingsTitle.defaultMessage })).toHaveAttribute('href', expect.stringContaining(encodingsDownloadUrl));
137+
});
138+
});

0 commit comments

Comments
 (0)