From 883d223d59ed2c7764bd4376f571148a749b3034 Mon Sep 17 00:00:00 2001 From: HamzaIbnFarooq Date: Wed, 17 Feb 2021 17:29:04 +0500 Subject: [PATCH] fix: add validation to capa problem markdown for missing option text Before this commit, if a course author created a capa mcqs or similar problem without providing any answer text for an option the question would be created causing abnormal behavior for learners. This commit will validate answer text of all options and raise an error message to author to fix the issue on the go. --- cms/static/js/views/modals/edit_xblock.js | 12 ++++++++++ .../xmodule/js/spec/problem/edit_spec.js | 22 +++++++++++++++++++ .../xmodule/xmodule/js/src/problem/edit.js | 9 ++++++++ .../common/js/components/utils/view_utils.js | 18 ++++++++++++++- 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 8a86b11fb1..2ccacc6c3d 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -172,7 +172,18 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod var self = this, editorView = this.editorView, xblockInfo = this.xblockInfo, + data = null; + try { data = editorView.getXBlockFieldData(); + } catch (e) { + ViewUtils.showErrorMeassage( + gettext("Studio's having trouble while parsing the problem content"), + e.message, + 10000 + ); + ViewUtils.setScrollOffset(editorView.$el, 100); + return null; + } event.preventDefault(); if (data) { ViewUtils.runOperationShowingMessage(gettext('Saving'), @@ -182,6 +193,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod self.onSave(); }); } + return null; }, onSave: function() { diff --git a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js index 7ac7869bae..e6cfa6f231 100644 --- a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js @@ -1046,5 +1046,27 @@ third \ `); }); + + it('should throw error if an option does not have any text associated with it', function() { + let problemContent = `\ +>>The following languages are in the Indo-European family:||<< +[ ] +[x] Urdu +[ ] Finnish\ +` + expect(function(){ MarkdownEditingDescriptor.markdownToXml(problemContent); }).toThrow( + new Error(gettext("One of the provided options doesn't have a valid text value")) + ); + + problemContent = `\ +>>The following languages are in the Indo-European family:||<< +( ) +(x) Urdu +( ) Finnish\ +` + expect(function(){ MarkdownEditingDescriptor.markdownToXml(problemContent); }).toThrow( + new Error(gettext("One of the provided options doesn't have a valid text value")) + ); + }); }); }); diff --git a/common/lib/xmodule/xmodule/js/src/problem/edit.js b/common/lib/xmodule/xmodule/js/src/problem/edit.js index c1f26ef9dc..20275b5d5a 100644 --- a/common/lib/xmodule/xmodule/js/src/problem/edit.js +++ b/common/lib/xmodule/xmodule/js/src/problem/edit.js @@ -402,6 +402,9 @@ line = lines[i].trim(); if (line.length > 0) { textHint = extractHint(line, true); + if (!textHint.nothint) { + throw new Error(gettext("One of the provided options doesn't have a valid text value")); + } correctstr = ' correct="' + (textHint.parens ? 'True' : 'False') + '"'; hintstr = ''; if (textHint.hint) { @@ -434,6 +437,9 @@ options[i] = options[i].trim(); // trim off leading/trailing whitespace if (options[i].length > 0) { value = options[i].split(/^\s*\(.{0,3}\)\s*/)[1]; + if (!value) { + throw new Error(gettext("One of the provided options doesn't have a valid text value")); + } inparens = /^\s*\((.{0,3})\)\s*/.exec(options[i])[1]; correct = /x/i.test(inparens); fixed = ''; @@ -494,6 +500,9 @@ } value = options[i].split(/^\s*\[.?\]\s*/)[1]; + if (!value) { + throw new Error(gettext("One of the provided options doesn't have a valid text value")); + } correct = /^\s*\[x\]/i.test(options[i]); hints = ''; // {{ selected: You’re right that apple is a fruit. }, diff --git a/common/static/common/js/components/utils/view_utils.js b/common/static/common/js/components/utils/view_utils.js index 64ac2fcbd2..5490e9c0ef 100644 --- a/common/static/common/js/components/utils/view_utils.js +++ b/common/static/common/js/components/utils/view_utils.js @@ -16,7 +16,7 @@ /* End Webpack */ var toggleExpandCollapse, showLoadingIndicator, hideLoadingIndicator, confirmThenRunOperation, - runOperationShowingMessage, withDisabledElement, disableElementWhileRunning, + runOperationShowingMessage, showErrorMeassage, withDisabledElement, disableElementWhileRunning, getScrollOffset, setScrollOffset, setScrollTop, redirect, reload, hasChangedAttributes, deleteNotificationHandler, validateRequiredField, validateURLItemEncoding, validateTotalKeyLength, checkTotalKeyLengthViolations, loadJavaScript; @@ -95,6 +95,21 @@ }); }; + /** + * Shows an error notification message for a specifc period of time. + * @param heading The heading of notification. + * @param message The message to show. + * @param timeInterval The time interval to hide the notification. + */ + showErrorMeassage = function(heading, message, timeInterval) { + var errorNotificationView = new NotificationView.Error({ + title: gettext(heading), + message: gettext(message) + }); + errorNotificationView.show(); + + setTimeout(function() { errorNotificationView.hide(); }, timeInterval); + }; /** * Wraps a Backbone event callback to disable the event's target element. * @@ -295,6 +310,7 @@ hideLoadingIndicator: hideLoadingIndicator, confirmThenRunOperation: confirmThenRunOperation, runOperationShowingMessage: runOperationShowingMessage, + showErrorMeassage: showErrorMeassage, withDisabledElement: withDisabledElement, disableElementWhileRunning: disableElementWhileRunning, deleteNotificationHandler: deleteNotificationHandler,