MDL-66841 forum: Display grade update errors to user
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 3 Oct 2019 06:36:20 +0000 (14:36 +0800)
committerMathew May <mathewm@hotmail.co.nz>
Wed, 30 Oct 2019 02:23:41 +0000 (10:23 +0800)
Part of MDL-66074

This change modifies the return val for all of the grading functions to
allow them to add additional information.
This means that a grading service can suppress a Grade saved message if
there were no changes, for example.

This also adds a distinction between:
- Errored (Exception thrown in the WS call)
- Failed (Warning in the output of the WS call)
- Success (Grade actually saved)
- None of the above (No save, no fail, no change)

28 files changed:
grade/amd/build/grades/grader/gradingpanel/normalise.min.js [new file with mode: 0644]
grade/amd/build/grades/grader/gradingpanel/normalise.min.js.map [new file with mode: 0644]
grade/amd/build/grades/grader/gradingpanel/point.min.js
grade/amd/build/grades/grader/gradingpanel/point.min.js.map
grade/amd/build/grades/grader/gradingpanel/repository.min.js
grade/amd/build/grades/grader/gradingpanel/repository.min.js.map
grade/amd/build/grades/grader/gradingpanel/scale.min.js
grade/amd/build/grades/grader/gradingpanel/scale.min.js.map
grade/amd/src/grades/grader/gradingpanel/normalise.js [new file with mode: 0644]
grade/amd/src/grades/grader/gradingpanel/point.js
grade/amd/src/grades/grader/gradingpanel/repository.js
grade/amd/src/grades/grader/gradingpanel/scale.js
grade/classes/component_gradeitem.php
grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js
grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js.map
grade/grading/form/guide/amd/src/grades/grader/gradingpanel.js
mod/forum/amd/build/local/grades/grader.min.js
mod/forum/amd/build/local/grades/grader.min.js.map
mod/forum/amd/build/local/grades/local/grader/selectors.min.js
mod/forum/amd/build/local/grades/local/grader/selectors.min.js.map
mod/forum/amd/build/local/grades/local/grader/user_picker.min.js
mod/forum/amd/build/local/grades/local/grader/user_picker.min.js.map
mod/forum/amd/src/local/grades/grader.js
mod/forum/amd/src/local/grades/local/grader/selectors.js
mod/forum/amd/src/local/grades/local/grader/user_picker.js
mod/forum/lang/en/forum.php
mod/forum/templates/local/grades/local/grader/grading.mustache
mod/forum/templates/local/grades/local/grader/gradingpanel/error.mustache [new file with mode: 0644]

diff --git a/grade/amd/build/grades/grader/gradingpanel/normalise.min.js b/grade/amd/build/grades/grader/gradingpanel/normalise.min.js
new file mode 100644 (file)
index 0000000..7ee975f
Binary files /dev/null and b/grade/amd/build/grades/grader/gradingpanel/normalise.min.js differ
diff --git a/grade/amd/build/grades/grader/gradingpanel/normalise.min.js.map b/grade/amd/build/grades/grader/gradingpanel/normalise.min.js.map
new file mode 100644 (file)
index 0000000..c1ae5e7
Binary files /dev/null and b/grade/amd/build/grades/grader/gradingpanel/normalise.min.js.map differ
index a4191f0..e7096de 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/point.min.js and b/grade/amd/build/grades/grader/gradingpanel/point.min.js differ
index 6712014..7bc1669 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/point.min.js.map and b/grade/amd/build/grades/grader/gradingpanel/point.min.js.map differ
index d019b21..34faeff 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/repository.min.js and b/grade/amd/build/grades/grader/gradingpanel/repository.min.js differ
index 2d08834..2fd7938 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/repository.min.js.map and b/grade/amd/build/grades/grader/gradingpanel/repository.min.js.map differ
index df80d6a..edb9c07 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/scale.min.js and b/grade/amd/build/grades/grader/gradingpanel/scale.min.js differ
index 0ef4ed9..7ceeb00 100644 (file)
Binary files a/grade/amd/build/grades/grader/gradingpanel/scale.min.js.map and b/grade/amd/build/grades/grader/gradingpanel/scale.min.js.map differ
diff --git a/grade/amd/src/grades/grader/gradingpanel/normalise.js b/grade/amd/src/grades/grader/gradingpanel/normalise.js
new file mode 100644 (file)
index 0000000..2ac0bea
--- /dev/null
@@ -0,0 +1,67 @@
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Repository for simple direct grading panel.
+ *
+ * @module     core_grades/grades/grader/gradingpanel/repository
+ * @package    core_grades
+ * @copyright  2019 Andrew Nicols <andrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+/**
+ * Normalise a resultset for consumption by the grader.
+ *
+ * @param {Object} result The result returned from a grading web service
+ * @return {Object}
+ */
+export const normaliseResult = result => {
+    return {
+        result,
+        failed: !!result.warnings.length,
+        success: !result.warnings.length,
+        error: null,
+    };
+};
+
+/**
+ * Return the resultset used to describe an invalid result.
+ *
+ * @return {Object}
+ */
+export const invalidResult = () => {
+    return {
+        success: false,
+        failed: false,
+        result: {},
+        error: null,
+    };
+};
+
+/**
+ * Return the resultset used to describe a failed update.
+ *
+ * @param {Object} error
+ * @return {Object}
+ */
+export const failedUpdate = error => {
+    return {
+        success: false,
+        failed: true,
+        result: {},
+        error,
+    };
+};
index 29f0e5c..5dea4db 100644 (file)
 import {saveGrade, fetchGrade} from './repository';
 // Note: We use jQuery.serializer here until we can rewrite Ajax to use XHR.send()
 import jQuery from 'jquery';
+import {invalidResult} from './normalise';
 
+/**
+ * Fetch the current grade for a user.
+ *
+ * @param {String} component
+ * @param {Number} context
+ * @param {String} itemname
+ * @param {Number} userId
+ * @param {Element} rootNode
+ * @return {Object}
+ */
 export const fetchCurrentGrade = (...args) => fetchGrade('point')(...args);
 
-export const storeCurrentGrade = (component, context, itemname, userId, rootNode) => {
+/**
+ * Store a new grade for a user.
+ *
+ * @param {String} component
+ * @param {Number} context
+ * @param {String} itemname
+ * @param {Number} userId
+ * @param {Element} rootNode
+ * @return {Object}
+ */
+export const storeCurrentGrade = async(component, context, itemname, userId, rootNode) => {
     const form = rootNode.querySelector('form');
-    return saveGrade('point')(component, context, itemname, userId, jQuery(form).serialize());
+    const grade = form.querySelector('input[name="grade"]');
+
+    if (!grade.checkValidity() || !grade.value.trim()) {
+        return invalidResult;
+    }
+
+    return await saveGrade('point')(component, context, itemname, userId, jQuery(form).serialize());
 };
index 65ade57..c7a51bd 100644 (file)
@@ -22,6 +22,7 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 import {call as fetchMany} from 'core/ajax';
+import {normaliseResult} from './normalise';
 
 export const fetchGrade = type => (component, contextid, itemname, gradeduserid) => {
     return fetchMany([{
@@ -35,8 +36,8 @@ export const fetchGrade = type => (component, contextid, itemname, gradeduserid)
     }])[0];
 };
 
-export const saveGrade = type => (component, contextid, itemname, gradeduserid, formdata) => {
-    return fetchMany([{
+export const saveGrade = type => async(component, contextid, itemname, gradeduserid, formdata) => {
+    return normaliseResult(await fetchMany([{
         methodname: `core_grades_grader_gradingpanel_${type}_store`,
         args: {
             component,
@@ -45,5 +46,5 @@ export const saveGrade = type => (component, contextid, itemname, gradeduserid,
             gradeduserid,
             formdata,
         },
-    }])[0];
+    }])[0]);
 };
index 8413ad2..3caca5e 100644 (file)
 import {saveGrade, fetchGrade} from './repository';
 // Note: We use jQuery.serializer here until we can rewrite Ajax to use XHR.send()
 import jQuery from 'jquery';
+import {invalidResult} from './normalise';
 
 export const fetchCurrentGrade = (...args) => fetchGrade('scale')(...args);
 
 export const storeCurrentGrade = (component, context, itemname, userId, rootNode) => {
     const form = rootNode.querySelector('form');
+    const grade = form.querySelector('select[name="grade"]');
+
+    if (!grade.checkValidity() || !grade.value.trim()) {
+        return invalidResult;
+    }
+
     return saveGrade('scale')(component, context, itemname, userId, jQuery(form).serialize());
 };
index eae68a6..115d61d 100644 (file)
@@ -415,6 +415,11 @@ abstract class component_gradeitem {
             $instanceid = $formdata->instanceid;
             $gradinginstance = $this->get_advanced_grading_instance($grader, $grade, (int) $instanceid);
             $grade->grade = $gradinginstance->submit_and_get_grade($formdata->advancedgrading, $grade->id);
+
+            if ($grade->grade == -1) {
+                // In advanced grading, a value of -1 means no data.
+                return false;
+            }
         } else {
             // Handle the case when grade is set to No Grade.
             if (isset($formdata->grade)) {
index 89c26ab..4ef79c5 100644 (file)
Binary files a/grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js and b/grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js differ
index 0d531ed..c8e52e6 100644 (file)
Binary files a/grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js.map and b/grade/grading/form/guide/amd/build/grades/grader/gradingpanel.min.js.map differ
index a0324c4..567ec6a 100644 (file)
@@ -23,6 +23,7 @@
  */
 
 import {call as fetchMany} from 'core/ajax';
+import {normaliseResult} from 'core_grades/grades/grader/gradingpanel/normalise';
 
 // Note: We use jQuery.serializer here until we can rewrite Ajax to use XHR.send()
 import jQuery from 'jquery';
@@ -40,10 +41,10 @@ export const fetchCurrentGrade = (component, contextid, itemname, gradeduserid)
 };
 
 
-export const storeCurrentGrade = (component, contextid, itemname, gradeduserid, rootNode) => {
+export const storeCurrentGrade = async(component, contextid, itemname, gradeduserid, rootNode) => {
     const form = rootNode.querySelector('form');
 
-    return fetchMany([{
+    return normaliseResult(await fetchMany([{
         methodname: `gradingform_guide_grader_gradingpanel_store`,
         args: {
             component,
@@ -52,5 +53,5 @@ export const storeCurrentGrade = (component, contextid, itemname, gradeduserid,
             gradeduserid,
             formdata: jQuery(form).serialize(),
         },
-    }])[0];
+    }])[0]);
 };
index 8947f9f..3c62a3e 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/grader.min.js and b/mod/forum/amd/build/local/grades/grader.min.js differ
index 78982ce..dbfdf99 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/grader.min.js.map and b/mod/forum/amd/build/local/grades/grader.min.js.map differ
index f63728b..18816fd 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/local/grader/selectors.min.js and b/mod/forum/amd/build/local/grades/local/grader/selectors.min.js differ
index 79f26b4..09295a0 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/local/grader/selectors.min.js.map and b/mod/forum/amd/build/local/grades/local/grader/selectors.min.js.map differ
index 67a39bd..14b67fe 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/local/grader/user_picker.min.js and b/mod/forum/amd/build/local/grades/local/grader/user_picker.min.js differ
index e69fd6f..8f21051 100644 (file)
Binary files a/mod/forum/amd/build/local/grades/local/grader/user_picker.min.js.map and b/mod/forum/amd/build/local/grades/local/grader/user_picker.min.js.map differ
index 0087b4f..6830192 100644 (file)
@@ -28,10 +28,14 @@ import {createLayout as createFullScreenWindow} from 'mod_forum/local/layout/ful
 import getGradingPanelFunctions from './local/grader/gradingpanel';
 import {add as addToast} from 'core/toast';
 import {get_string as getString} from 'core/str';
+import {failedUpdate} from 'core_grades/grades/grader/gradingpanel/normalise';
 
 const templateNames = {
     grader: {
         app: 'mod_forum/local/grades/grader',
+        gradingPanel: {
+            error: 'mod_forum/local/grades/local/grader/gradingpanel/error',
+        },
     },
 };
 
@@ -92,23 +96,51 @@ const registerEventListeners = (graderLayout, userPicker, saveGradeFunction) =>
 /**
  * Get the function used to save a user grade.
  *
- * @param {Element} root The contaienr
+ * @param {Element} root The container for the grader
  * @param {Function} setGradeForUser The function that will be called.
  * @return {Function}
  */
 const getSaveUserGradeFunction = (root, setGradeForUser) => {
     return async user => {
         try {
+            root.querySelector(Selectors.regions.gradingPanelErrors).innerHTML = '';
             const result = await setGradeForUser(user.id, root.querySelector(Selectors.regions.gradingPanel));
-            addToast(await getString('grades:gradesavedfor', 'mod_forum', user));
+            if (result.success) {
+                addToast(await getString('grades:gradesavedfor', 'mod_forum', user));
+            }
+            if (result.failed) {
+                displayGradingError(root, user, result.error);
+            }
 
             return result;
-        } catch (error) {
-            throw error;
+        } catch (err) {
+            displayGradingError(root, user, err);
+
+            return failedUpdate(err);
         }
     };
 };
 
+/**
+ * Display a grading error, typically from a failed save.
+ *
+ * @param {Element} root The container for the grader
+ * @param {Object} user The user who was errored
+ * @param {Object} err The details of the error
+ */
+const displayGradingError = async(root, user, err) => {
+    const [
+        {html, js},
+        errorString
+    ] = await Promise.all([
+        Templates.renderForPromise(templateNames.grader.gradingPanel.error, {error: err}),
+        await getString('grades:gradesavefailed', 'mod_forum', {error: err.message, ...user}),
+    ]);
+
+    Templates.replaceNodeContents(root.querySelector(Selectors.regions.gradingPanelErrors), html, js);
+    addToast(errorString);
+};
+
 /**
  * Launch the grader interface with the specified parameters.
  *
index 8f55bba..a8889af 100644 (file)
@@ -36,6 +36,7 @@ export default {
         moduleReplace: getDataSelector('region', 'module_content'),
         pickerRegion: getDataSelector('region', 'user_picker'),
         gradingPanel: getDataSelector('region', 'grade'),
+        gradingPanelErrors: getDataSelector('region', 'grade-errors'),
     },
 };
 
index 0a9be23..f32ac59 100644 (file)
@@ -121,12 +121,13 @@ class UserPicker {
         this.root.addEventListener('click', async e => {
             const button = e.target.closest(Selectors.actions.changeUser);
             if (button) {
-                const whole = document.querySelector('[data-region="unified-grader"]');
-                const spinner = addIconToContainerWithPromise(whole);
+                const result = await this.preChangeUserCallback(this.currentUser);
+                const spinner = addIconToContainerWithPromise(document.querySelector('[data-region="unified-grader"]'));
 
-                await this.preChangeUserCallback(this.currentUser);
-                this.updateIndex(parseInt(button.dataset.direction));
-                await this.showUser(this.currentUser);
+                if (!result.failed) {
+                    this.updateIndex(parseInt(button.dataset.direction));
+                    await this.showUser(this.currentUser);
+                }
 
                 spinner.resolve();
             }
index 8d0a946..bad04fd 100644 (file)
@@ -728,6 +728,7 @@ $string['gradeforwholeforumhidden'] = 'Grade for forum hidden';
 $string['gradeitemnameforwholeforum'] = 'Whole forum grade for {$a->name}';
 $string['gradeitemnameforrating'] = 'Rating grade for {$a->name}';
 $string['grades:gradesavedfor'] = 'Grade saved for {$a->fullname}';
+$string['grades:gradesavefailed'] = 'Unable to save grade for {$a->fullname}: {$a->error}';
 
 // Deprecated since Moodle 3.8.
 $string['cannotdeletediscussioninsinglediscussion'] = 'You cannot delete the first post in a single discussion';
index 11889b4..997b4c3 100644 (file)
@@ -47,6 +47,7 @@
         <div data-region="grade" class="col-md-12 mt-3">
           {{> mod_forum/local/grades/local/grader/grade_placeholder }}
         </div>
+        <div data-region="grade-errors" role="alert" aria-role="assertive"></div>
         <hr/>
     </div>
 </div>
diff --git a/mod/forum/templates/local/grades/local/grader/gradingpanel/error.mustache b/mod/forum/templates/local/grades/local/grader/gradingpanel/error.mustache
new file mode 100644 (file)
index 0000000..c6396e0
--- /dev/null
@@ -0,0 +1,35 @@
+{{!
+    This file is part of Moodle - http://moodle.org/
+
+    Moodle is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    Moodle is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+}}
+{{!
+    @template mod_forum/local/grades/local/grader/gradingpanel/error
+
+    Classes required for JS:
+    * TODO
+
+    Data attributes required for JS:
+    * TODO
+
+    Context variables required for this template:
+    * TODO
+
+    Example context (json):
+    {
+    }
+}}
+<div class="col-md-12 bg-danger text-white rounded pt-3 pb-3" id="gradingpanel-error-{{uniqid}}">
+    {{{error.message}}}
+</div>