fix: grace period validation and parsing (#2222)

* fix: correct default for grace period when parsing xml
* fix: front-end validation of grace periods
* refactor: un-duplicate code
* chore: version bumped to 6.11.3

.gitignore

@@ -36,6 +36,7 @@ htmlcov
 coverage.xml
 test_ora2db
 test_ora2db-journal
+test/logs/
 
 # Mr Developer
 .mr.developer.cfg

openassessment/__init__.py

@@ -2,4 +2,4 @@
 Initialization Information for Open Assessment Module
 """
 
-__version__ = '6.11.2'
+__version__ = '6.11.3'

openassessment/xblock/static/dist/manifest.json

@@ -4,10 +4,10 @@
   "openassessment-editor-textarea.js.map": "/openassessment-editor-textarea.2cee26d88c3441ada635.js.map",
   "openassessment-editor-tinymce.js": "/openassessment-editor-tinymce.0b97b77ad7f1b7150f67.js",
   "openassessment-editor-tinymce.js.map": "/openassessment-editor-tinymce.0b97b77ad7f1b7150f67.js.map",
-  "openassessment-lms.css": "/openassessment-lms.d29f0bd34bb5d82d36fb.css",
-  "openassessment-lms.js": "/openassessment-lms.d29f0bd34bb5d82d36fb.js",
-  "openassessment-lms.css.map": "/openassessment-lms.d29f0bd34bb5d82d36fb.css.map",
-  "openassessment-lms.js.map": "/openassessment-lms.d29f0bd34bb5d82d36fb.js.map",
+  "openassessment-lms.css": "/openassessment-lms.525a519b6c2e4bc043b0.css",
+  "openassessment-lms.js": "/openassessment-lms.525a519b6c2e4bc043b0.js",
+  "openassessment-lms.css.map": "/openassessment-lms.525a519b6c2e4bc043b0.css.map",
+  "openassessment-lms.js.map": "/openassessment-lms.525a519b6c2e4bc043b0.js.map",
   "openassessment-ltr.css": "/openassessment-ltr.7955a1e2cc11fc6948de.css",
   "openassessment-ltr.js": "/openassessment-ltr.7955a1e2cc11fc6948de.js",
   "openassessment-ltr.css.map": "/openassessment-ltr.7955a1e2cc11fc6948de.css.map",
@@ -16,8 +16,8 @@
   "openassessment-rtl.js": "/openassessment-rtl.9de7c9bc7c1048c07707.js",
   "openassessment-rtl.css.map": "/openassessment-rtl.9de7c9bc7c1048c07707.css.map",
   "openassessment-rtl.js.map": "/openassessment-rtl.9de7c9bc7c1048c07707.js.map",
-  "openassessment-studio.js": "/openassessment-studio.fba153d2eafbb4c8e78c.js",
-  "openassessment-studio.js.map": "/openassessment-studio.fba153d2eafbb4c8e78c.js.map",
-  "fallback-default.png": "/4620b30a966533ace489dcc7afb151b9.png",
+  "openassessment-studio.js": "/openassessment-studio.227e19b5d099e14574a2.js",
+  "openassessment-studio.js.map": "/openassessment-studio.227e19b5d099e14574a2.js.map",
+  "fallback-default.png": "/1b90ce76fe01a1aa6e5ec289a5fb3799.png",
   "default-avatar.svg": "/95ec738c0b7faac5b5c9126794446bbd.svg"
 }

openassessment/xblock/static/js/spec/studio/oa_edit_settings.js

@@ -236,4 +236,44 @@ describe("OpenAssessment.EditSettingsView", function() {
         });
         expect($('#openassessment_leaderboard_editor').prop('disabled')).toBe(true);
     });
+
+    it("enables the grace period option when resubmission is enabled, and disabled it otherwise", () => {
+        const isGracePeriodHidden = () => $('#openassessment_resubmissions_grace_period_wrapper')[0].classList.contains('is--hidden');
+
+        view.allowLearnerResubmissions(false);
+        expect(isGracePeriodHidden()).toBe(true);
+
+        view.allowLearnerResubmissions(true);
+        expect(isGracePeriodHidden()).toBe(false);
+    });
+
+    describe('validates grace period', () => {
+        it('when resubmission is enabled', () => {
+            view.allowLearnerResubmissions(true);
+
+            view.resubmissionsGracePeriod('00:01:00');
+            expect(view.validate()).toBe(true);
+
+            view.resubmissionsGracePeriod('invalid');
+            expect(view.validate()).toBe(false);
+        });
+
+        describe('when resubmission is disabled', () => {
+            beforeEach(() => {
+                view.allowLearnerResubmissions(false);
+            })
+
+            it('a valid grace period is not cleared on validate', () =>{
+                view.resubmissionsGracePeriod('00:01:00');
+                expect(view.validate()).toBe(true);
+                expect(view.resubmissionsGracePeriod()).toBe('00:01:00');    
+            })
+
+            it('an invalid grace period is cleared on validate', () =>{
+                view.resubmissionsGracePeriod('invalid');
+                expect(view.validate()).toBe(true);
+                expect(view.resubmissionsGracePeriod()).toBe('');
+            });
+        });
+    });
 });

openassessment/xblock/static/js/src/studio/oa_edit_settings.js

@@ -71,7 +71,7 @@ export class EditSettingsView {
       new Notifier([new AssessmentToggleListener()]),
     ).install();
 
-    new SelectControl(
+    this.allowResubmissionsSelectControl = new SelectControl(
       $('#openassessment_allow_learner_resubmissions_selector', this.element),
       (selectedValue) => {
         const isHidden = selectedValue === '0';
@@ -496,9 +496,25 @@ export class EditSettingsView {
         boolean
      * */
   allowLearnerResubmissions(isEnabled) {
+    if (isEnabled !== undefined) {
+      this.allowResubmissionsSelectControl.change(isEnabled ? '1' : '0');
+    }
     return this.settingSelectorEnabled('#openassessment_allow_learner_resubmissions_selector', isEnabled);
   }
 
+  conditionallyValidateOrClear(field, fieldEnabled) {
+    if (fieldEnabled) {
+      return field.validate();
+    }
+    // we want to keep a valid value in case the author reenables the field
+    /* eslint-disable-next-line no-lonely-if */
+    if (field.get() && !field.validate()) {
+      // but will clear the field in case it is invalid
+      field.set('');
+    }
+    return true;
+  }
+
   /**
     Mark validation errors.
 
@@ -511,18 +527,15 @@ export class EditSettingsView {
     let isValid = true;
 
     isValid = (this.leaderboardIntField.validate() && isValid);
-    isValid = (this.resubmissionsGracePeriodInputField.validate() && isValid);
 
-    if (this.fileUploadType() === 'custom') {
-      isValid = (this.fileTypeWhiteListInputField.validate() && isValid);
-    } else {
-      // we want to keep the valid white list in case author changes upload type back to custom
-      /* eslint-disable-next-line no-lonely-if */
-      if (this.fileTypeWhiteListInputField.get() && !this.fileTypeWhiteListInputField.validate()) {
-        // but will clear the field in case it is invalid
-        this.fileTypeWhiteListInputField.set('');
-      }
-    }
+    isValid = (this.conditionallyValidateOrClear(
+      this.resubmissionsGracePeriodInputField,
+      this.allowLearnerResubmissions(),
+    ) && isValid);
+    isValid = (this.conditionallyValidateOrClear(
+      this.fileTypeWhiteListInputField,
+      this.fileUploadType() === 'custom',
+    ) && isValid);
 
     return isValid;
   }

openassessment/xblock/test/data/update_from_xml.json

@@ -26,6 +26,8 @@
         "submission_due": null,
         "leaderboard_show": 0,
         "show_rubric_during_response": false,
+        "allow_learner_resubmissions": false,
+        "resubmissions_grace_period": "",
         "rubric_criteria": [
             {
                 "order_num": 0,
@@ -675,6 +677,30 @@
             "</rubric>",
             "</openassessment>"
         ],
-        "allow_learner_resubmissions": true
+        "allow_learner_resubmissions": true,
+        "resubmissions_grace_period": ""
+    },
+
+    "allow_learner_resubmissions_grace_period": {
+        "xml": [
+            "<openassessment allow_learner_resubmissions=\"true\" resubmissions_grace_period=\"00:00:30\">",
+            "<title>Foo</title>",
+            "<assessments>",
+                "<assessment name=\"peer-assessment\" start=\"2014-02-27T09:46:28\" due=\"2014-03-01T00:00:00\" must_grade=\"5\" must_be_graded_by=\"3\" />",
+                "<assessment name=\"self-assessment\" start=\"2014-04-01T00:00:00\" due=\"2014-06-01T00:00:00\" />",
+            "</assessments>",
+            "<rubric>",
+                "<prompt>Test prompt</prompt>",
+                "<criterion>",
+                    "<name>Test criterion</name>",
+                    "<prompt>Test criterion prompt</prompt>",
+                    "<option points=\"0\"><name>No</name><explanation>No explanation</explanation></option>",
+                    "<option points=\"2\"><name>Yes</name><explanation>Yes explanation</explanation></option>",
+                "</criterion>",
+            "</rubric>",
+            "</openassessment>"
+        ],
+        "allow_learner_resubmissions": true,
+        "resubmissions_grace_period": "00:00:30"
     }
 }

openassessment/xblock/utils/xml.py

@@ -927,7 +927,7 @@ def parse_from_xml(root):
     if 'allow_learner_resubmissions' in root.attrib:
         allow_learner_resubmissions = _parse_boolean(str(root.attrib['allow_learner_resubmissions']))
 
-    resubmissions_grace_period = None
+    resubmissions_grace_period = ''
     if 'resubmissions_grace_period' in root.attrib:
         resubmissions_grace_period = str(root.attrib['resubmissions_grace_period'])
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "edx-ora2",
-  "version": "6.11.2",
+  "version": "6.11.3",
   "repository": "https://github.com/openedx/edx-ora2.git",
   "dependencies": {
     "@edx/frontend-build": "8.0.6",