Skip to content

Commit c8e47f1

Browse files
committed
fix: Disable file buttons when actions in progress
In the student view "Your Response" section, there were several race conditions and issues relating to file uploads: - Selecting the file input while multiple file uploads are in progress results in broken uploaded files and inconsistent UI. - A file can be deleted after clicking Submit, while the submission is still processing. - Clicking "Upload files" again after successfully uploading files, but before selecting more files, results in an attempt to upload the same files again. On a high speed network connection, the race condition issues are not very noticeable, but on a slower connection, they are easy to trigger as there is more time in the UI while processing is happening in the backend. These changes fix the above issues by ensuring that the buttons and other input fields relating to file uploads are disabled while actions are in progress (uploading, deleting, submitting). Private-ref: https://tasks.opencraft.com/browse/BB-9938
1 parent 3e4bf88 commit c8e47f1

13 files changed

+87
-53
lines changed

openassessment/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
Initialization Information for Open Assessment Module
33
"""
44

5-
__version__ = '6.16.4'
5+
__version__ = '6.16.5'

openassessment/xblock/static/dist/manifest.json

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@
44
"openassessment-editor-textarea.js.map": "/openassessment-editor-textarea.cbb31e4372be87d437fb.js.map",
55
"openassessment-editor-tinymce.js": "/openassessment-editor-tinymce.2a1e66e98a2a1132f633.js",
66
"openassessment-editor-tinymce.js.map": "/openassessment-editor-tinymce.2a1e66e98a2a1132f633.js.map",
7-
"openassessment-lms.css": "/openassessment-lms.42bdf01af5d117224bc9.css",
8-
"openassessment-lms.js": "/openassessment-lms.42bdf01af5d117224bc9.js",
9-
"openassessment-lms.css.map": "/openassessment-lms.42bdf01af5d117224bc9.css.map",
10-
"openassessment-lms.js.map": "/openassessment-lms.42bdf01af5d117224bc9.js.map",
11-
"openassessment-ltr.css": "/openassessment-ltr.0c6318822d5661f0f9ea.css",
12-
"openassessment-ltr.js": "/openassessment-ltr.0c6318822d5661f0f9ea.js",
13-
"openassessment-ltr.css.map": "/openassessment-ltr.0c6318822d5661f0f9ea.css.map",
14-
"openassessment-ltr.js.map": "/openassessment-ltr.0c6318822d5661f0f9ea.js.map",
15-
"openassessment-rtl.css": "/openassessment-rtl.c938e1614108d8feb891.css",
16-
"openassessment-rtl.js": "/openassessment-rtl.c938e1614108d8feb891.js",
17-
"openassessment-rtl.css.map": "/openassessment-rtl.c938e1614108d8feb891.css.map",
18-
"openassessment-rtl.js.map": "/openassessment-rtl.c938e1614108d8feb891.js.map",
7+
"openassessment-lms.css": "/openassessment-lms.3b32189187e8e2d3e0c3.css",
8+
"openassessment-lms.js": "/openassessment-lms.3b32189187e8e2d3e0c3.js",
9+
"openassessment-lms.css.map": "/openassessment-lms.3b32189187e8e2d3e0c3.css.map",
10+
"openassessment-lms.js.map": "/openassessment-lms.3b32189187e8e2d3e0c3.js.map",
11+
"openassessment-ltr.css": "/openassessment-ltr.97990de9668aaa4cd1d5.css",
12+
"openassessment-ltr.js": "/openassessment-ltr.97990de9668aaa4cd1d5.js",
13+
"openassessment-ltr.css.map": "/openassessment-ltr.97990de9668aaa4cd1d5.css.map",
14+
"openassessment-ltr.js.map": "/openassessment-ltr.97990de9668aaa4cd1d5.js.map",
15+
"openassessment-rtl.css": "/openassessment-rtl.19576ab9f81f644cb7fd.css",
16+
"openassessment-rtl.js": "/openassessment-rtl.19576ab9f81f644cb7fd.js",
17+
"openassessment-rtl.css.map": "/openassessment-rtl.19576ab9f81f644cb7fd.css.map",
18+
"openassessment-rtl.js.map": "/openassessment-rtl.19576ab9f81f644cb7fd.js.map",
1919
"openassessment-studio.js": "/openassessment-studio.b58bb4e55f63dadac6de.js",
2020
"openassessment-studio.js.map": "/openassessment-studio.b58bb4e55f63dadac6de.js.map",
2121
"fallback-default.png": "/4620b30a966533ace489dcc7afb151b9.png",

openassessment/xblock/static/dist/openassessment-lms.3b32189187e8e2d3e0c3.css

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-lms.42bdf01af5d117224bc9.js renamed to openassessment/xblock/static/dist/openassessment-lms.3b32189187e8e2d3e0c3.js

Lines changed: 17 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-lms.42bdf01af5d117224bc9.css

Lines changed: 0 additions & 3 deletions
This file was deleted.

openassessment/xblock/static/dist/openassessment-ltr.0c6318822d5661f0f9ea.css renamed to openassessment/xblock/static/dist/openassessment-ltr.97990de9668aaa4cd1d5.css

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-ltr.0c6318822d5661f0f9ea.js renamed to openassessment/xblock/static/dist/openassessment-ltr.97990de9668aaa4cd1d5.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-rtl.c938e1614108d8feb891.css renamed to openassessment/xblock/static/dist/openassessment-rtl.19576ab9f81f644cb7fd.css

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/dist/openassessment-rtl.c938e1614108d8feb891.js renamed to openassessment/xblock/static/dist/openassessment-rtl.19576ab9f81f644cb7fd.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openassessment/xblock/static/js/src/lms/oa_response.js

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,26 @@ export class ResponseView {
159159

160160
const uploadButton = sel.find('.file__upload');
161161
const spinner = sel.find('.fa-spinner');
162+
162163
// Install a click handler for the save button
163164
uploadButton.click(
164165
(eventObject) => {
165166
// Override default form submission
166167
eventObject.preventDefault();
167168
$('.submission__answer__display__file', view.element).removeClass('is--hidden');
168-
uploadButton.prop('disabled', true);
169+
view.disableFields();
169170
spinner.removeClass('is--hidden');
170171
if (view.hasAllUploadFiles()) {
171172
const promise = view.uploadFiles();
172173
promise.then(() => {
173-
uploadButton.prop('disabled', false);
174+
view.enableFields();
174175
spinner.addClass('is--hidden');
176+
// Reset the internal files state once upload is complete,
177+
// to avoid the files being processed again if the user clicks upload again.
178+
this.files = null;
175179
});
176180
} else {
177-
uploadButton.prop('disabled', false);
181+
view.enableFields();
178182
spinner.addClass('is--hidden');
179183
}
180184
},
@@ -307,6 +311,32 @@ export class ResponseView {
307311
return this.baseView.buttonEnabled('.step--response__submit', enabled);
308312
}
309313

314+
/**
315+
* Disable the fields to avoid any changes while an action is processing.
316+
* For example, use this when beginning a file upload, or submitting the response.
317+
*/
318+
disableFields() {
319+
this.submitEnabled(false);
320+
321+
const responseEl = $('.step--response', this.element);
322+
responseEl.find('.file__upload').prop('disabled', true);
323+
responseEl.find('.delete__uploaded__file').prop('disabled', true);
324+
responseEl.find('.submission__answer__upload').prop('disabled', true);
325+
}
326+
327+
/**
328+
* Enable the fields to allow changes again.
329+
* Use this to revert the changes made by `disableFields()`.
330+
*/
331+
enableFields() {
332+
this.submitEnabled(true);
333+
334+
const responseEl = $('.step--response', this.element);
335+
responseEl.find('.file__upload').prop('disabled', false);
336+
responseEl.find('.delete__uploaded__file').prop('disabled', false);
337+
responseEl.find('.submission__answer__upload').prop('disabled', false);
338+
}
339+
310340
/**
311341
Enable/disable the preview button.
312342
Check whether the preview button is enabled.
@@ -523,8 +553,9 @@ export class ResponseView {
523553
handleSubmitClicked() {
524554
if (!this.isValidForSubmit()) { return; }
525555

526-
// Immediately disable the submit button to prevent multiple submission
527-
this.submitEnabled(false);
556+
// Immediately disable the submit button to prevent multiple submission,
557+
// and other form fields to avoid race conditions if anything changes.
558+
this.disableFields();
528559

529560
const view = this;
530561
const title = gettext('Confirm Submit Response');
@@ -547,7 +578,7 @@ export class ResponseView {
547578
title,
548579
msg,
549580
() => view.submit(),
550-
() => view.submitEnabled(true),
581+
() => view.enableFields(),
551582
);
552583
}
553584

@@ -568,8 +599,8 @@ export class ResponseView {
568599
// If there is an error message, display it
569600
if (errMsg) { this.baseView.toggleActionError('submit', errMsg); }
570601

571-
// Re-enable the submit button so the user can retry
572-
this.submitEnabled(true);
602+
// Re-enable the submit button and the rest of the form so the user can retry
603+
this.enableFields();
573604
}
574605
});
575606
}
@@ -840,20 +871,23 @@ export class ResponseView {
840871
*/
841872
handleDeleteFileClick(target) {
842873
const view = this;
874+
this.disableFields();
843875
const filenum = $(target).attr('filenum');
844876
this.confirmationDialog.confirm(
845877
gettext('Confirm Delete Uploaded File'),
846878
this.getConfirmRemoveUploadedFileMessage(filenum),
847-
() => view.removeUploadedFile(filenum),
848-
() => {},
879+
() => view.removeUploadedFile(filenum).always(() => view.enableFields()),
880+
() => view.enableFields(),
849881
);
850882
}
851883

852884
/**
853885
Remove a previously uploaded file.
886+
887+
Returns a jQuery promise that resolves once the process is complete.
854888
*/
855889
removeUploadedFile(filenum) {
856-
this.server.removeUploadedFile(filenum).done(() => {
890+
return this.server.removeUploadedFile(filenum).done(() => {
857891
const sel = $('.step--response', this.element);
858892
const block = sel.find(`.submission__answer__file__block__${filenum}`);
859893
block.html('');

0 commit comments

Comments
 (0)