feat: add duplicate file validation for asset upload (#885)

* 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
This commit is contained in:
Kristin Aoki
2024-03-13 12:09:00 -04:00
committed by GitHub
parent a88a88e9af
commit 9740974bbd
12 changed files with 318 additions and 50 deletions

View File

@@ -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);

View File

@@ -16,6 +16,7 @@ import {
getUsagePaths,
resetErrors,
updateAssetOrder,
validateAssetFiles,
} from './data/thunks';
import messages from './messages';
import FilesPageProvider from './FilesPageProvider';
@@ -30,6 +31,7 @@ import {
import { getFileSizeToClosestByte } from '../../utils';
import FileThumbnail from './FileThumbnail';
import FileInfoModalSidebar from './FileInfoModalSidebar';
import FileValidationModal from './FileValidationModal';
const FilesPage = ({
courseId,
@@ -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 }));
@@ -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>

View File

@@ -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';
@@ -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;
@@ -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);
@@ -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 () => {
@@ -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 () => {
@@ -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 () => {

View File

@@ -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

View File

@@ -9,6 +9,7 @@ const slice = createSlice({
initialState: {
assetIds: [],
loadingStatus: RequestStatus.IN_PROGRESS,
duplicateFiles: [],
updatingStatus: '',
addingStatus: '',
deletingStatus: '',
@@ -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') {
@@ -89,6 +93,7 @@ export const {
updateErrors,
clearErrors,
updateEditStatus,
updateDuplicateFiles,
} = slice.actions;
export const {

View File

@@ -15,6 +15,7 @@ import {
deleteAsset,
updateLockStatus,
getDownload,
getAssetDetails,
} from './api';
import {
setAssetIds,
@@ -25,11 +26,12 @@ import {
updateErrors,
clearErrors,
updateEditStatus,
updateDuplicateFiles,
} from './slice';
import { updateFileValues } from './utils';
import { getUploadConflicts, updateFileValues } from './utils';
export function fetchAddtionalAsstets(courseId, totalCount) {
export function fetchAdditionalAssets(courseId, totalCount) {
return async (dispatch) => {
let remainingAssetCount = totalCount;
let page = 1;
@@ -66,7 +68,7 @@ export function fetchAssets(courseId) {
assetIds: assets.map(asset => asset.id),
}));
if (totalCount > 50) {
dispatch(fetchAddtionalAsstets(courseId, totalCount - 50));
dispatch(fetchAdditionalAssets(courseId, totalCount - 50));
}
dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL }));
} catch (error) {
@@ -104,7 +106,7 @@ export function deleteAssetFile(courseId, id) {
};
}
export function addAssetFile(courseId, file) {
export function addAssetFile(courseId, file, isOverwrite) {
return async (dispatch) => {
dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.IN_PROGRESS }));
@@ -115,9 +117,11 @@ export function addAssetFile(courseId, file) {
modelType: 'assets',
model: { ...parsedAssets },
}));
dispatch(addAssetSuccess({
assetId: asset.id,
}));
if (!isOverwrite) {
dispatch(addAssetSuccess({
assetId: asset.id,
}));
}
dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.SUCCESSFUL }));
} catch (error) {
if (error.response && error.response.status === 413) {
@@ -131,6 +135,30 @@ export function addAssetFile(courseId, file) {
};
}
export function validateAssetFiles(courseId, files) {
return async (dispatch) => {
dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.IN_PROGRESS }));
dispatch(updateDuplicateFiles({ files: {} }));
try {
const filenames = [];
files.forEach(file => filenames.push(file.name));
await getAssetDetails({ courseId, filenames, fileCount: filenames.length }).then(({ assets }) => {
const [conflicts, newFiles] = getUploadConflicts(files, assets);
if (!isEmpty(newFiles)) {
newFiles.forEach(file => dispatch(addAssetFile(courseId, file)));
}
if (!isEmpty(conflicts)) {
dispatch(updateDuplicateFiles({ files: conflicts }));
}
});
} catch (error) {
files.forEach(file => dispatch(updateErrors({ error: 'add', message: `Failed to validate ${file.name}.` })));
dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED }));
}
};
}
export function updateAssetLock({ assetId, courseId, locked }) {
return async (dispatch) => {
dispatch(updateEditStatus({ editType: 'lock', status: RequestStatus.IN_PROGRESS }));

View File

@@ -57,3 +57,17 @@ export const getSrc = ({ thumbnail, wrapperType, externalUrl }) => {
return InsertDriveFile;
}
};
export const getUploadConflicts = (filesToUpload, assets) => {
const filesFound = assets.map(item => item.displayName);
const conflicts = {};
const newFiles = [];
filesToUpload.forEach(file => {
if (filesFound.includes(file.name)) {
conflicts[file.name] = file;
} else {
newFiles.push(file);
}
});
return [conflicts, newFiles];
};

View File

@@ -22,6 +22,7 @@ export const initialState = {
usageMetrics: [],
loading: '',
},
duplicateFiles: {},
},
models: {
assets: {
@@ -101,9 +102,9 @@ export const generateFetchAssetApiResponse = () => ({
displayName: 'mOckID6',
locked: false,
externalUrl: 'static_tab_1',
portableUrl: 'May 17, 2023 at 22:08 UTC',
portableUrl: '',
contentType: 'application/octet-stream',
dateAdded: '',
dateAdded: 'May 17, 2023 at 22:08 UTC',
thumbnail: null,
},
],

View File

@@ -81,6 +81,26 @@ const messages = defineMessages({
id: 'course-authoring.files-and-videos.sort-and-filter.modal.filter.otherCheckbox.label',
defaultMessage: 'Other',
},
overwriteConfirmMessage: {
id: 'course-authoring.files-and-videos.overwrite.modal.confirmation-message',
defaultMessage: 'Some of the uploaded files already exist in this course. Do you want to overwrite the following files?',
description: 'The message displayed in the modal shown when uploading files with pre-existing names',
},
overwriteModalTitle: {
id: 'course-authoring.files-and-videos.overwrite.modal.title',
defaultMessage: 'Overwrite files',
description: 'The title of the modal to confirm overwriting the files',
},
confirmOverwriteButtonLabel: {
id: 'course-authoring.files-and-videos.overwrite.modal.overwrite-button.label',
defaultMessage: 'Overwrite',
description: 'The message displayed in the button to confirm overwriting the files',
},
cancelOverwriteButtonLabel: {
id: 'course-authoring.files-and-videos.overwrite.modal.cancel-button.label',
defaultMessage: 'Cancel',
description: 'The message displayed in the button to confirm cancelling the upload',
},
});
export default messages;

View File

@@ -12,10 +12,9 @@ export const useFileInput = ({
const addFile = (e) => {
const { files } = e.target;
setSelectedRows(files);
Object.values(files).forEach(file => {
onAddFile(file);
setAddOpen();
});
onAddFile(Object.values(files));
setAddOpen();
e.target.value = '';
};
return {
click,

View File

@@ -95,14 +95,14 @@ const FileTable = ({
}, [files]);
const fileInputControl = useFileInput({
onAddFile: (file) => handleAddFile(file),
onAddFile: (uploads) => handleAddFile(uploads),
setSelectedRows,
setAddOpen,
});
const handleDropzoneAsset = ({ fileData, handleError }) => {
try {
const file = fileData.get('file');
handleAddFile(file);
handleAddFile([file]);
} catch (error) {
handleError(error);
}

View File

@@ -80,11 +80,14 @@ const VideosPage = ({
const supportedFileFormats = { 'video/*': videoSupportedFileFormats || FILES_AND_UPLOAD_TYPE_FILTERS.video };
const handleAddFile = (file) => dispatch(addVideoFile(courseId, file, videoIds));
const handleErrorReset = (error) => dispatch(resetErrors(error));
const handleAddFile = (files) => {
handleErrorReset({ errorType: 'add' });
files.forEach(file => dispatch(addVideoFile(courseId, file, videoIds)));
};
const handleDeleteFile = (id) => dispatch(deleteVideoFile(courseId, id));
const handleDownloadFile = (selectedRows) => dispatch(fetchVideoDownload({ selectedRows, courseId }));
const handleUsagePaths = (video) => dispatch(getUsagePaths({ video, courseId }));
const handleErrorReset = (error) => dispatch(resetErrors(error));
const handleFileOrder = ({ newFileIdOrder, sortType }) => {
dispatch(updateVideoOrder(courseId, newFileIdOrder, sortType));
};