Compare commits

...

8 Commits

Author SHA1 Message Date
jansenk
eae2da950e chore: remove 'enhanced' from page title 2022-05-09 16:32:02 -04:00
edx-semantic-release
b2098be114 chore(i18n): update translations 2022-04-17 11:50:11 -04:00
leangseu-edx
64ac98c310 download filename, error handling and cache busting (#98)
* feat: handle download error and display them

* chore: update test environment for easier single file test

* feat: add cache bursting to the download
2022-04-14 13:52:39 -04:00
Leangseu Kim
8a80e2a70e chore: update package 2022-04-12 10:36:25 -04:00
Matthew Carter
a936d970db Merge pull request #95 from muselesscreator/batch_unlock_api
feat: Batch unlock api
2022-04-11 10:56:21 -04:00
Ben Warzeski
56c6c88638 feat: connect batch unlock to the api 2022-04-07 15:55:49 -04:00
Ben Warzeski
9c42bfbd8a fix: update snapshot 2022-04-07 15:53:09 -04:00
Ben Warzeski
69733f7837 fix: update routing for images 2022-04-07 15:52:50 -04:00
17 changed files with 16948 additions and 15871 deletions

View File

@@ -14,4 +14,5 @@ module.exports = createConfig('jest', {
'src/postcss.config.js',
],
testTimeout: 120000,
testEnvironment: 'jsdom',
});

32566
package-lock.json generated

File diff suppressed because it is too large Load Diff

View File

@@ -28,7 +28,7 @@
"dependencies": {
"@edx/brand": "npm:@edx/brand-edx.org@^2.0.3",
"@edx/frontend-component-footer": "10.1.6",
"@edx/frontend-platform": "1.12.4",
"@edx/frontend-platform": "^1.15.6",
"@edx/paragon": "16.14.4",
"@fortawesome/fontawesome-svg-core": "^1.2.36",
"@fortawesome/free-brands-svg-icons": "^5.15.4",
@@ -51,11 +51,10 @@
"history": "5.0.1",
"html-react-parser": "^1.3.0",
"lodash": "^4.17.21",
"node-sass": "^6.0.1",
"prop-types": "15.7.2",
"query-string": "7.0.1",
"react": "17.0.2",
"react-dom": "17.0.2",
"react": "^16.14.0",
"react-dom": "^16.14.0",
"react-intl": "^5.20.9",
"react-pdf": "^5.5.0",
"react-redux": "^7.2.4",
@@ -73,7 +72,7 @@
"whatwg-fetch": "^3.6.2"
},
"devDependencies": {
"@edx/frontend-build": "9.1.1",
"@edx/frontend-build": "^9.1.4",
"@testing-library/jest-dom": "^5.14.1",
"@testing-library/react": "^12.1.0",
"axios-mock-adapter": "^1.20.0",
@@ -86,7 +85,7 @@
"jest": "27.0.6",
"jest-expect-message": "^1.0.2",
"react-dev-utils": "^11.0.4",
"react-test-renderer": "^17.0.2",
"react-test-renderer": "^16.14.0",
"reactifex": "1.1.1",
"redux-mock-store": "^1.5.4",
"semantic-release": "^17.4.5"

View File

@@ -1,7 +1,7 @@
<!doctype html>
<html lang="en-us" dir="ltr">
<head>
<title>ORA Enhanced Staff Grader | <%= process.env.SITE_NAME %></title>
<title>ORA Staff Grader | <%= process.env.SITE_NAME %></title>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="shortcut icon" href="<%=htmlWebpackPlugin.options.FAVICON_URL%>" type="image/x-icon" />

View File

@@ -6,7 +6,7 @@ import {
Icon, Form, ActionRow, IconButton,
} from '@edx/paragon';
import { ChevronLeft, ChevronRight } from '@edx/paragon/icons';
import pdfjsWorker from 'react-pdf/node_modules/pdfjs-dist/build/pdf.worker.entry';
import pdfjsWorker from 'react-pdf/dist/esm/pdf.worker.entry';
import 'react-pdf/dist/esm/Page/AnnotationLayer.css';

View File

@@ -28,7 +28,7 @@ export class DownloadErrors extends React.Component {
if (!this.props.isFailed) { return null; }
return (
<ReviewError
key="lockFailed"
key="downloadFailed"
headingMessage={messages.downloadFailedHeading}
actions={{
cancel: { onClick: this.cancelAction, message: messages.dismiss },
@@ -36,19 +36,36 @@ export class DownloadErrors extends React.Component {
}}
>
<FormattedMessage {...messages.downloadFailedContent} />
<br />
<FormattedMessage {...messages.failedFiles} />
<ul>
{this.props.error.files.map(filename => (
<li key={filename}>{filename}</li>
))}
</ul>
</ReviewError>
);
}
}
DownloadErrors.defaultProps = {
error: {
files: [],
},
};
DownloadErrors.propTypes = {
// redux
clearState: PropTypes.func.isRequired,
isFailed: PropTypes.bool.isRequired,
error: PropTypes.shape({
files: PropTypes.arrayOf(PropTypes.string),
}),
downloadFiles: PropTypes.func.isRequired,
};
export const mapStateToProps = (state) => ({
isFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.downloadFiles }),
error: selectors.requests.error(state, { requestKey: RequestKeys.downloadFiles }),
});
export const mapDispatchToProps = {

View File

@@ -14,7 +14,10 @@ let el;
jest.mock('data/redux', () => ({
selectors: {
requests: { isFailed: (...args) => ({ isFailed: args }) },
requests: {
isFailed: (...args) => ({ isFailed: args }),
error: (...args) => ({ error: args }),
},
},
actions: {
requests: { clearRequest: jest.fn() },
@@ -28,6 +31,9 @@ jest.mock('./ReviewError', () => 'ReviewError');
describe('DownloadErrors component', () => {
const props = {
isFailed: false,
error: {
files: [],
},
};
describe('component', () => {
beforeEach(() => {
@@ -40,7 +46,12 @@ describe('DownloadErrors component', () => {
el.instance().cancelAction = jest.fn().mockName('this.cancelAction');
});
test('failed: show error', () => {
el.setProps({ isFailed: true });
el.setProps({
isFailed: true,
error: {
files: ['file-1-failed.error', 'file-2.failed'],
},
});
expect(el.instance().render()).toMatchSnapshot();
expect(el.isEmptyRender()).toEqual(false);
});
@@ -68,6 +79,10 @@ describe('DownloadErrors component', () => {
const requestKey = RequestKeys.downloadFiles;
expect(mapped.isFailed).toEqual(selectors.requests.isFailed(testState, { requestKey }));
});
test('error loads from requests.error(downloadFiles)', () => {
const requestKey = RequestKeys.downloadFiles;
expect(mapped.error).toEqual(selectors.requests.error(testState, { requestKey }));
});
});
describe('mapDispatchToProps', () => {
it('loads clearState from actions.requests.clearRequest', () => {

View File

@@ -34,6 +34,20 @@ exports[`DownloadErrors component component snapshots failed: show error 1`] = `
description="Failed download error content"
id="ora-grading.ReviewModal.errorDownloadFailedContent"
/>
<br />
<FormattedMessage
defaultMessage="Failed files:"
description="List header for file download failure alert"
id="ora-grading.ReviewModal.errorDownloadFailedFiles"
/>
<ul>
<li>
file-1-failed.error
</li>
<li>
file-2.failed
</li>
</ul>
</ReviewError>
`;

View File

@@ -82,6 +82,11 @@ const messages = defineMessages({
defaultMessage: 'Retry download',
description: 'Failed download retry button text',
},
failedFiles: {
id: 'ora-grading.ReviewModal.errorDownloadFailedFiles',
defaultMessage: 'Failed files:',
description: 'List header for file download failure alert',
},
});
export default StrictDict(messages);

View File

@@ -1,15 +1,17 @@
import * as zip from '@zip.js/zip.js';
import FileSaver from 'file-saver';
import { StrictDict } from 'utils';
import { RequestKeys } from 'data/constants/requests';
import { selectors } from 'data/redux';
import { locationId } from 'data/constants/app';
import { stringifyUrl } from 'data/services/lms/utils';
import { networkRequest } from './requests';
import * as module from './download';
export const ERRORS = StrictDict({
fetchFailed: 'Fetch failed',
export const DownloadException = (files) => ({
files,
name: 'DownloadException',
});
/**
@@ -21,22 +23,13 @@ export const genManifest = (files) => files.map(
(file) => `Filename: ${file.name}\nDescription: ${file.description}\nSize: ${file.size}`,
).join('\n\n');
/**
* Returns the zip filename
* @return {string} - zip download file name
*/
export const zipFileName = () => {
const currentDate = new Date().getTime();
return `ora-files-download-${currentDate}.zip`;
};
/**
* Zip the blob output of a set of files with a manifest file.
* @param {obj[]} files - list of file entries with downloadUrl, name, and description
* @param {blob[]} blobs - file content blobs
* @return {Promise} - zip async process promise.
*/
export const zipFiles = async (files, blobs) => {
export const zipFiles = async (files, blobs, username) => {
const zipWriter = new zip.ZipWriter(new zip.BlobWriter('application/zip'));
await zipWriter.add('manifest.txt', new zip.TextReader(module.genManifest(files)));
@@ -50,24 +43,60 @@ export const zipFiles = async (files, blobs) => {
}
const zipFile = await zipWriter.close();
FileSaver.saveAs(zipFile, module.zipFileName());
const zipName = `${username}-${locationId}.zip`;
FileSaver.saveAs(zipFile, zipName);
};
/**
* generate url with additional timestamp for cache busting.
* This is implemented for fixing issue with the browser not
* allowing the user to fetch the same url as the image tag.
* @param {string} url
* @returns {string}
*/
export const getTimeStampUrl = (url) => stringifyUrl(url, {
ora_grading_download_timestamp: new Date().getTime(),
});
/**
* Download a file and return its blob is successful, or null if not.
* @param {obj} file - file entry with downloadUrl
* @return {blob} - file blob or null
* @return {Promise} - file blob or null
*/
export const downloadFile = (file) => fetch(file.downloadUrl).then(resp => (
resp.ok ? resp.blob() : null
));
export const downloadFile = (file) => fetch(
module.getTimeStampUrl(file.downloadUrl),
).then((response) => {
if (!response.ok) {
// This is necessary because some of the error such as 404 does not throw.
// Due to that inconsistency, I have decide to share catch statement like this.
throw new Error(response.statusText);
}
return response.blob();
});
/**
* Download blobs given file objects. Returns a promise map.
* @param {obj[]} files - list of file entries with downloadUrl, name, and description
* @return {Promise[]} - Promise map of download attempts (null for failed fetches)
*/
export const downloadBlobs = (files) => Promise.all(files.map(module.downloadFile));
export const downloadBlobs = async (files) => {
const blobs = [];
const errors = [];
// eslint-disable-next-line no-restricted-syntax
for (const file of files) {
try {
// eslint-disable-next-line no-await-in-loop
blobs.push(await module.downloadFile(file));
} catch (error) {
errors.push(file.name);
}
}
if (errors.length) {
throw DownloadException(errors);
}
return blobs;
};
/**
* Download all files for the selected submission as a zip file.
@@ -75,14 +104,10 @@ export const downloadBlobs = (files) => Promise.all(files.map(module.downloadFil
*/
export const downloadFiles = () => (dispatch, getState) => {
const { files } = selectors.grading.selected.response(getState());
const username = selectors.grading.selected.username(getState());
dispatch(networkRequest({
requestKey: RequestKeys.downloadFiles,
promise: module.downloadBlobs(files).then(blobs => {
if (blobs.some(blob => blob === null)) {
throw Error(ERRORS.fetchFailed);
}
return module.zipFiles(files, blobs);
}),
promise: module.downloadBlobs(files).then(blobs => module.zipFiles(files, blobs, username)),
}));
};

View File

@@ -37,7 +37,10 @@ jest.mock('./requests', () => ({
jest.mock('data/redux', () => ({
selectors: {
grading: {
selected: { response: jest.fn() },
selected: {
response: jest.fn(),
username: jest.fn(),
},
},
},
}));
@@ -55,6 +58,7 @@ describe('download thunkActions', () => {
const files = [mockFile('test-file1.jpg'), mockFile('test-file2.pdf')];
const blobs = ['blob1', 'blob2'];
const response = { files };
const username = 'student-name';
let dispatch;
const getState = () => testState;
describe('genManifest', () => {
@@ -67,13 +71,10 @@ describe('download thunkActions', () => {
);
});
});
describe('zipFileName', () => {
// add tests when name is more nailed down
});
describe('zipFiles', () => {
test('zips files and manifest', () => {
const mockZipWriter = new zip.ZipWriter();
return download.zipFiles(files, blobs).then(() => {
return download.zipFiles(files, blobs, username).then(() => {
expect(mockZipWriter.files).toEqual([
['manifest.txt', mockTextReader],
[files[0].name, mockBlobReader],
@@ -86,63 +87,98 @@ describe('download thunkActions', () => {
});
});
describe('getTimeStampUrl', () => {
it('generate different url every milisecond for cache busting', () => {
const testUrl = 'test/url?param1=true';
const firstGen = download.getTimeStampUrl(testUrl);
// fast forward for 1 milisecond
jest.advanceTimersByTime(1);
const secondGen = download.getTimeStampUrl(testUrl);
expect(firstGen).not.toEqual(secondGen);
});
});
describe('downloadFile', () => {
let fetch;
let getTimeStampUrl;
const blob = 'test-blob';
const file = files[0];
beforeEach(() => {
fetch = window.fetch;
window.fetch = jest.fn();
getTimeStampUrl = download.getTimeStampUrl;
download.getTimeStampUrl = jest.fn();
});
afterEach(() => {
window.fetch = fetch;
download.getTimeStampUrl = getTimeStampUrl;
});
it('returns blob output if successful', () => {
window.fetch.mockReturnValue(Promise.resolve({ ok: true, blob: () => blob }));
return download
.downloadFile(files[0])
.then((val) => expect(val).toEqual(blob));
expect(download.downloadFile(file)).resolves.toEqual(blob);
expect(download.getTimeStampUrl).toBeCalledWith(file.downloadUrl);
});
it('returns null if not successful', () => {
window.fetch.mockReturnValue(Promise.resolve({ ok: false }));
return download
.downloadFile(files[0])
.then((val) => expect(val).toEqual(null));
it('throw if not successful', () => {
const failFetchStatusText = 'failed to fetch';
window.fetch.mockReturnValue(Promise.resolve({ ok: false, statusText: failFetchStatusText }));
expect(() => download.downloadFile(file)).rejects.toThrow(failFetchStatusText);
expect(download.getTimeStampUrl).toBeCalledWith(file.downloadUrl);
});
});
describe('downloadBlobs', () => {
it('returns a joing promise mapping all files to download action', async () => {
download.downloadFile = (file) => Promise.resolve(file.name);
const responses = await download.downloadBlobs(files);
expect(responses).toEqual(files.map((file) => file.name));
let downloadFile;
beforeEach(() => {
downloadFile = download.downloadFile;
download.downloadFile = jest.fn((file) => Promise.resolve(file.name));
});
afterEach(() => { download.downloadFile = downloadFile; });
it('returns a mapping of all files to download action', async () => {
const downloadedBlobs = await download.downloadBlobs(files);
expect(download.downloadFile).toHaveBeenCalledTimes(files.length);
expect(downloadedBlobs.length).toEqual(files.length);
expect(downloadedBlobs).toEqual(files.map(file => file.name));
});
it('returns a mapping of errors from download action', () => {
download.downloadFile = jest.fn(() => { throw new Error(); });
expect(download.downloadBlobs(files)).rejects.toEqual(download.DownloadException(files.map(file => file.name)));
expect(download.downloadFile).toHaveBeenCalledTimes(files.length);
});
});
describe('downloadFiles', () => {
let downloadBlobs;
beforeEach(() => {
dispatch = jest.fn();
selectors.grading.selected.response = () => ({ files });
selectors.grading.selected.username = () => username;
download.zipFiles = jest.fn();
});
it('dispatches network request with downloadFiles key', () => {
downloadBlobs = download.downloadBlobs;
download.downloadBlobs = () => Promise.resolve(blobs);
});
afterEach(() => { download.downloadBlobs = downloadBlobs; });
it('dispatches network request with downloadFiles key', () => {
download.downloadFiles()(dispatch, getState);
const { networkRequest } = dispatch.mock.calls[0][0];
expect(networkRequest.requestKey).toEqual(RequestKeys.downloadFiles);
});
it('dispatches network request for downloadFiles, zipping output of downloadBlobs', () => {
it('dispatches network request for downloadFiles, zipping output of downloadBlobs', async () => {
download.downloadBlobs = () => Promise.resolve(blobs);
download.downloadFiles()(dispatch, getState);
const { networkRequest } = dispatch.mock.calls[0][0];
networkRequest.promise.then(() => {
expect(download.zipFiles).toHaveBeenCalledWith(files, blobs);
});
await networkRequest.promise;
expect(download.zipFiles).toHaveBeenCalledWith(files, blobs, username);
});
it('throws an error on failure', () => {
download.downloadBlobs = () => Promise.all([Promise.resolve(null)]);
it('network request catch all of the errors', () => {
const blobsErrors = ['arbitary', 'error'];
download.downloadBlobs = () => Promise.reject(blobsErrors);
download.downloadFiles()(dispatch, getState);
const { networkRequest } = dispatch.mock.calls[0][0];
expect(networkRequest.promise).rejects.toThrow('Fetch failed');
expect(networkRequest.promise).rejects.toEqual(blobsErrors);
});
});
});

View File

@@ -97,10 +97,13 @@ const unlockSubmission = (submissionUUID) => client().delete(
* batchUnlockSubmissions(submissionUUIDs)
* @param {string[]} submissionUUIDs - list of submission uuids
*/
const batchUnlockSubmissions = (submissionUUIDs) => {
console.log({ batchUnlockSubmissions: submissionUUIDs });
return new Promise(resolve => resolve());
};
const batchUnlockSubmissions = (submissionUUIDs) => post(
stringifyUrl(
urls.batchUnlockSubmissionsUrl,
{ [paramKeys.oraLocation]: locationId },
),
{ submissionUUIDs },
).then(response => response.data);
/*
* post('api/updateGrade', { submissionUUID, gradeData })

View File

@@ -10,6 +10,7 @@ const oraInitializeUrl = `${baseEsgUrl}initialize`;
const fetchSubmissionUrl = `${baseEsgUrl}submission`;
const fetchSubmissionStatusUrl = `${baseEsgUrl}submission/status`;
const fetchSubmissionLockUrl = `${baseEsgUrl}submission/lock`;
const batchUnlockSubmissionsUrl = `${baseEsgUrl}submission/batch/unlock`;
const updateSubmissionGradeUrl = `${baseEsgUrl}submission/grade`;
const course = (courseId) => `${baseUrl}/courses/${courseId}`;
@@ -25,6 +26,7 @@ export default StrictDict({
fetchSubmissionUrl,
fetchSubmissionStatusUrl,
fetchSubmissionLockUrl,
batchUnlockSubmissionsUrl,
updateSubmissionGradeUrl,
baseUrl,
course,

View File

@@ -93,6 +93,7 @@
"ora-grading.ReviewModal.errorDownloadFailed": "Couldn't download files",
"ora-grading.ReviewModal.errorDownloadFailedContent": "We're sorry, something went wrong when we tried to download these files. Please try again.",
"ora-grading.ReviewModal.errorRetryDownload": "Retry download",
"ora-grading.ReviewModal.errorDownloadFailedFiles": "Failed files:",
"ora-grading.Rubric.gradeSubmitted": "Grade Submitted",
"ora-grading.Rubric.rubric": "Rubric",
"ora-grading.Rubric.submitGrade": "Submit grade",

View File

@@ -93,6 +93,7 @@
"ora-grading.ReviewModal.errorDownloadFailed": "No se pudieron descargar los archivos",
"ora-grading.ReviewModal.errorDownloadFailedContent": "Lo sentimos, algo salió mal cuando intentamos descargar estos archivos. Inténtalo de nuevo.",
"ora-grading.ReviewModal.errorRetryDownload": "Vuelva a intentar descargar",
"ora-grading.ReviewModal.errorDownloadFailedFiles": "Failed files:",
"ora-grading.Rubric.gradeSubmitted": "Calificación enviada",
"ora-grading.Rubric.rubric": "Rúbrica",
"ora-grading.Rubric.submitGrade": "Enviar calificación",

View File

@@ -93,6 +93,7 @@
"ora-grading.ReviewModal.errorDownloadFailed": "Impossible de télécharger les fichiers",
"ora-grading.ReviewModal.errorDownloadFailedContent": "Nous sommes désolés, une erreur s&#39;est produite lorsque nous avons essayé de télécharger ces fichiers. Veuillez réessayer.",
"ora-grading.ReviewModal.errorRetryDownload": "Réessayez le téléchargement",
"ora-grading.ReviewModal.errorDownloadFailedFiles": "Failed files:",
"ora-grading.Rubric.gradeSubmitted": "Note soumise",
"ora-grading.Rubric.rubric": "Rubrique",
"ora-grading.Rubric.submitGrade": "Soumettre la note",

View File

@@ -93,6 +93,7 @@
"ora-grading.ReviewModal.errorDownloadFailed": "Couldn't download files",
"ora-grading.ReviewModal.errorDownloadFailedContent": "We're sorry, something went wrong when we tried to download these files. Please try again.",
"ora-grading.ReviewModal.errorRetryDownload": "Retry download",
"ora-grading.ReviewModal.errorDownloadFailedFiles": "Failed files:",
"ora-grading.Rubric.gradeSubmitted": "Grade Submitted",
"ora-grading.Rubric.rubric": "Rubric",
"ora-grading.Rubric.submitGrade": "Submit grade",