Skip to content

Commit

Permalink
feat: add duplicate file validation for asset upload (#885)
Browse files Browse the repository at this point in the history 
* feat: add duplicate file validation for asset upload

* fix: modal only appearing once

* feat: add tests for overwrite modal

* fix: input not allowing second upload of same file

* fix: default pageSize for asset details
  • Loading branch information
@KristinAoki
KristinAoki authored Mar 13, 2024
1 parent a88a88e commit 9740974
Show file tree
Hide file tree
Showing 12 changed files with 318 additions and 50 deletions.
67 changes: 67 additions & 0 deletions src/files-and-videos/files-page/FileValidationModal.jsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React, { useEffect } from 'react';
import PropTypes from 'prop-types';
import { useSelector } from 'react-redux';
import { injectIntl, FormattedMessage, intlShape } from '@edx/frontend-platform/i18n';
import {
ActionRow,
Button,
ModalDialog,
useToggle,
} from '@openedx/paragon';
import { isEmpty } from 'lodash';

import messages from './messages';

const FileValidationModal = ({
handleFileOverwrite,
// injected
intl,
}) => {
const [isOpen, open, close] = useToggle();

const { duplicateFiles } = useSelector(state => state.assets);

useEffect(() => {
if (!isEmpty(duplicateFiles)) {
open();
}
}, [duplicateFiles]);

return (
<ModalDialog
title={intl.formatMessage(messages.overwriteModalTitle)}
isOpen={isOpen}
onClose={close}
>
<ModalDialog.Header>
<ModalDialog.Title>
<FormattedMessage {...messages.overwriteModalTitle} />
</ModalDialog.Title>
</ModalDialog.Header>
<ModalDialog.Body>
<FormattedMessage {...messages.overwriteConfirmMessage} />
<ul className="mt-2">
{Object.keys(duplicateFiles).map(file => <li>{file}</li>)}
</ul>
</ModalDialog.Body>
<ModalDialog.Footer>
<ActionRow>
<ModalDialog.CloseButton variant="tertiary">
<FormattedMessage {...messages.cancelOverwriteButtonLabel} />
</ModalDialog.CloseButton>
<Button onClick={() => handleFileOverwrite(close, duplicateFiles)}>
<FormattedMessage {...messages.confirmOverwriteButtonLabel} />
</Button>
</ActionRow>
</ModalDialog.Footer>
</ModalDialog>
);
};

FileValidationModal.propTypes = {
handleFileOverwrite: PropTypes.func.isRequired,
// injected
intl: intlShape.isRequired,
};

export default injectIntl(FileValidationModal);
50 changes: 31 additions & 19 deletions src/files-and-videos/files-page/FilesPage.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getUsagePaths,
resetErrors,
updateAssetOrder,
validateAssetFiles,
} from './data/thunks';
import messages from './messages';
import FilesPageProvider from './FilesPageProvider';
Expand All @@ -30,6 +31,7 @@ import {
import { getFileSizeToClosestByte } from '../../utils';
import FileThumbnail from './FileThumbnail';
import FileInfoModalSidebar from './FileInfoModalSidebar';
import FileValidationModal from './FileValidationModal';

const FilesPage = ({
courseId,
Expand All @@ -55,9 +57,16 @@ const FilesPage = ({
} = useSelector(state => state.assets);

const handleErrorReset = (error) => dispatch(resetErrors(error));
const handleAddFile = (file) => dispatch(addAssetFile(courseId, file));
const handleDeleteFile = (id) => dispatch(deleteAssetFile(courseId, id));
const handleDownloadFile = (selectedRows) => dispatch(fetchAssetDownload({ selectedRows, courseId }));
const handleAddFile = (files) => {
handleErrorReset({ errorType: 'add' });
dispatch(validateAssetFiles(courseId, files));
};
const handleFileOverwrite = (close, files) => {
Object.values(files).forEach(file => dispatch(addAssetFile(courseId, file, true)));
close();
};
const handleLockFile = (fileId, locked) => {
handleErrorReset({ errorType: 'lock' });
dispatch(updateAssetLock({ courseId, assetId: fileId, locked }));
Expand Down Expand Up @@ -183,24 +192,27 @@ const FilesPage = ({
<FormattedMessage {...messages.heading} />
</div>
{loadingStatus !== RequestStatus.FAILED && (
<FileTable
{...{
courseId,
data,
handleAddFile,
handleDeleteFile,
handleDownloadFile,
handleLockFile,
handleUsagePaths,
handleErrorReset,
handleFileOrder,
tableColumns,
maxFileSize,
thumbnailPreview,
infoModalSidebar,
files: assets,
}}
/>
<>
<FileTable
{...{
courseId,
data,
handleAddFile,
handleDeleteFile,
handleDownloadFile,
handleLockFile,
handleUsagePaths,
handleErrorReset,
handleFileOrder,
tableColumns,
maxFileSize,
thumbnailPreview,
infoModalSidebar,
files: assets,
}}
/>
<FileValidationModal {...{ handleFileOverwrite }} />
</>
)}
</Container>
</FilesPageProvider>
Expand Down
134 changes: 120 additions & 14 deletions src/files-and-videos/files-page/FilesPage.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import userEvent from '@testing-library/user-event';
import ReactDOM from 'react-dom';
import { saveAs } from 'file-saver';

import { initializeMockApp } from '@edx/frontend-platform';
import { camelCaseObject, initializeMockApp } from '@edx/frontend-platform';
import MockAdapter from 'axios-mock-adapter';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { AppProvider } from '@edx/frontend-platform/react';
Expand All @@ -36,9 +36,12 @@ import {
deleteAssetFile,
updateAssetLock,
getUsagePaths,
validateAssetFiles,
} from './data/thunks';
import { getAssetsUrl } from './data/api';
import messages from '../generic/messages';
import filesPageMessages from './messages';
import { updateFileValues } from './data/utils';

let axiosMock;
let store;
Expand Down Expand Up @@ -124,12 +127,13 @@ describe('FilesAndUploads', () => {
await emptyMockStore(RequestStatus.SUCCESSFUL);
const dropzone = screen.getByTestId('files-dropzone');
await act(async () => {
axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] });
axiosMock.onPost(getAssetsUrl(courseId)).reply(204, generateNewAssetApiResponse());
Object.defineProperty(dropzone, 'files', {
value: [file],
});
fireEvent.drop(dropzone);
await executeThunk(addAssetFile(courseId, file, 0), store.dispatch);
await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch);
});
const addStatus = store.getState().assets.addingStatus;
expect(addStatus).toEqual(RequestStatus.SUCCESSFUL);
Expand Down Expand Up @@ -191,19 +195,106 @@ describe('FilesAndUploads', () => {
});

describe('table actions', () => {
it('should upload a single file', async () => {
await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onPost(getAssetsUrl(courseId)).reply(200, generateNewAssetApiResponse());
let addFilesButton;
await waitFor(() => {
addFilesButton = screen.getByLabelText('file-input');
describe('upload a single file', () => {
it('should upload without duplication modal', async () => {
await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] });
axiosMock.onPost(getAssetsUrl(courseId)).reply(200, generateNewAssetApiResponse());
let addFilesButton;
await waitFor(() => {
addFilesButton = screen.getByLabelText('file-input');
});
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch);
});
const addStatus = store.getState().assets.addingStatus;
expect(addStatus).toEqual(RequestStatus.SUCCESSFUL);
});
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(addAssetFile(courseId, file, 1), store.dispatch);

it('should show duplicate file modal', async () => {
file = new File(['(⌐□_□)'], 'mOckID6', { type: 'image/png' });

await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(
`${getAssetsUrl(courseId)}?display_name=mOckID6&page_size=1`,
).reply(200, { assets: [{ display_name: 'mOckID6' }] });
let addFilesButton;
await waitFor(() => {
addFilesButton = screen.getByLabelText('file-input');
});
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch);
});
expect(screen.getByText(filesPageMessages.overwriteConfirmMessage.defaultMessage)).toBeVisible();
});

it('should overwrite duplicate file', async () => {
file = new File(['(⌐□_□)'], 'mOckID6', { type: 'image/png' });

await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(
`${getAssetsUrl(courseId)}?display_name=mOckID6&page_size=1`,
).reply(200, { assets: [{ display_name: 'mOckID6' }] });
const { asset: newDefaultAssetResponse } = generateNewAssetApiResponse();
const responseData = {
asset: {
...newDefaultAssetResponse, id: 'mOckID6',
},
};

axiosMock.onPost(getAssetsUrl(courseId)).reply(200, responseData);
let addFilesButton;
await waitFor(() => {
addFilesButton = screen.getByLabelText('file-input');
});
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch);
});

const overwriteButton = screen.getByText(filesPageMessages.confirmOverwriteButtonLabel.defaultMessage);
await act(async () => {
fireEvent.click(overwriteButton);
});

const assetData = store.getState().models.assets.mOckID6;
const { asset: responseAssetData } = responseData;
const [defaultData] = updateFileValues([camelCaseObject(responseAssetData)]);

expect(screen.queryByText(filesPageMessages.overwriteConfirmMessage.defaultMessage)).toBeNull();
expect(assetData).toEqual(defaultData);
});

it('should keep original file', async () => {
file = new File(['(⌐□_□)'], 'mOckID6', { type: 'image/png' });

await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(
`${getAssetsUrl(courseId)}?display_name=mOckID6&page_size=1`,
).reply(200, { assets: [{ display_name: 'mOckID6' }] });
let addFilesButton;
await waitFor(() => {
addFilesButton = screen.getByLabelText('file-input');
});
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch);
});

const cancelButton = screen.getByText(filesPageMessages.cancelOverwriteButtonLabel.defaultMessage);
await act(async () => {
fireEvent.click(cancelButton);
});

const assetData = store.getState().models.assets.mOckID6;
const defaultAssets = generateFetchAssetApiResponse().assets;
const [defaultData] = updateFileValues([defaultAssets[4]]);

expect(screen.queryByText(filesPageMessages.overwriteConfirmMessage.defaultMessage)).toBeNull();
expect(assetData).toEqual(defaultData);
});
const addStatus = store.getState().assets.addingStatus;
expect(addStatus).toEqual(RequestStatus.SUCCESSFUL);
});

it('should have disabled action buttons', async () => {
Expand Down Expand Up @@ -503,7 +594,7 @@ describe('FilesAndUploads', () => {
it('invalid file size should show error', async () => {
const errorMessage = 'File download.png exceeds maximum size of 20 MB.';
await mockStore(RequestStatus.SUCCESSFUL);

axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] });
axiosMock.onPost(getAssetsUrl(courseId)).reply(413, { error: errorMessage });
const addFilesButton = screen.getByLabelText('file-input');
await act(async () => {
Expand All @@ -515,8 +606,23 @@ describe('FilesAndUploads', () => {
expect(screen.getByText('Error')).toBeVisible();
});

it('404 validation should show error', async () => {
await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(404);
const addFilesButton = screen.getByLabelText('file-input');
await act(async () => {
userEvent.upload(addFilesButton, file);
await executeThunk(addAssetFile(courseId, file, 1), store.dispatch);
});
const addStatus = store.getState().assets.addingStatus;
expect(addStatus).toEqual(RequestStatus.FAILED);

expect(screen.getByText('Error')).toBeVisible();
});

it('404 upload should show error', async () => {
await mockStore(RequestStatus.SUCCESSFUL);
axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] });
axiosMock.onPost(getAssetsUrl(courseId)).reply(404);
const addFilesButton = screen.getByLabelText('file-input');
await act(async () => {
Expand Down
13 changes: 13 additions & 0 deletions src/files-and-videos/files-page/data/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ export async function getAssets(courseId, page) {
return camelCaseObject(data);
}

/**
* Fetches the course custom pages for provided course
* @param {string} courseId
* @returns {Promise<[{}]>}
*/
export async function getAssetDetails({ courseId, filenames, fileCount }) {
const params = new URLSearchParams(filenames.map(filename => ['display_name', filename]));
params.append('page_size', fileCount);
const { data } = await getAuthenticatedHttpClient()
.get(`${getAssetsUrl(courseId)}?${params}`);
return camelCaseObject(data);
}

/**
* Fetch asset file.
* @param {blockId} courseId Course ID for the course to operate on
Expand Down
5 changes: 5 additions & 0 deletions src/files-and-videos/files-page/data/slice.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const slice = createSlice({
initialState: {
assetIds: [],
loadingStatus: RequestStatus.IN_PROGRESS,
duplicateFiles: [],
updatingStatus: '',
addingStatus: '',
deletingStatus: '',
Expand Down Expand Up @@ -64,6 +65,9 @@ const slice = createSlice({
addAssetSuccess: (state, { payload }) => {
state.assetIds = [payload.assetId, ...state.assetIds];
},
updateDuplicateFiles: (state, { payload }) => {
state.duplicateFiles = payload.files;
},
updateErrors: (state, { payload }) => {
const { error, message } = payload;
if (error === 'loading') {
Expand All @@ -89,6 +93,7 @@ export const {
updateErrors,
clearErrors,
updateEditStatus,
updateDuplicateFiles,
} = slice.actions;

export const {
Expand Down
Loading

0 comments on commit 9740974

Please sign in to comment.