Merge pull request #34 from open-craft/feedback-persistence

Fixed submit button being enabled after page reload

problem_builder/mrq.py

@@ -81,17 +81,17 @@ class MRQBlock(QuestionnaireAbstractBlock):
             return self._(u"Ignored")
         return self._(u"Not Acceptable")
 
-    def get_results(self, previous_result, only_selected=False):
+    def get_results(self, previous_result):
         """
         Get the results a student has already submitted.
         """
-        result = self.calculate_results(previous_result['submissions'], only_selected)
+        result = self.calculate_results(previous_result['submissions'])
         result['completed'] = True
         return result
 
     def get_last_result(self):
         if self.student_choices:
-            return self.get_results({'submissions': self.student_choices}, only_selected=True)
+            return self.get_results({'submissions': self.student_choices})
         else:
             return {}
 
@@ -104,7 +104,7 @@ class MRQBlock(QuestionnaireAbstractBlock):
         log.debug(u'MRQ submissions result: %s', result)
         return result
 
-    def calculate_results(self, submissions, only_selected=False):
+    def calculate_results(self, submissions):
         score = 0
         results = []
 
@@ -112,8 +112,6 @@ class MRQBlock(QuestionnaireAbstractBlock):
             choice_completed = True
             choice_tips_html = []
             choice_selected = choice.value in submissions
-            if not choice_selected and only_selected:
-                continue
 
             if choice.value in self.required_choices:
                 if not choice_selected:

problem_builder/public/js/mentoring_standard_view.js

@@ -4,7 +4,7 @@ function MentoringStandardView(runtime, element, mentoring) {
 
     var callIfExists = mentoring.callIfExists;
 
-    function handleSubmitResults(response) {
+    function handleSubmitResults(response, disable_submit) {
         messagesDOM.empty().hide();
 
         $.each(response.results || [], function(index, result_spec) {
@@ -28,9 +28,16 @@ function MentoringStandardView(runtime, element, mentoring) {
             messagesDOM.prepend('<div class="title1">' + mentoring.data.feedback_label + '</div>');
             messagesDOM.show();
         }
+
+        // this method is called on successful submission and on page load
+        // results will be empty only for initial load if no submissions was made
+        // in such case we must allow submission to support submitting empty read-only long answer recaps
+        if (disable_submit || response.results.length > 0) {
+            submitDOM.attr('disabled', 'disabled');
+        }
     }
 
-    function handleSubmitError(jqXHR, textStatus, errorThrown) {
+    function handleSubmitError(jqXHR, textStatus, errorThrown, disable_submit) {
         if (textStatus == "error") {
             var errMsg = errorThrown;
             // Check if there's a more specific JSON error message:
@@ -44,6 +51,10 @@ function MentoringStandardView(runtime, element, mentoring) {
             mentoring.setContent(messagesDOM, errMsg);
             messagesDOM.show();
         }
+
+        if (disable_submit) {
+            submitDOM.attr('disabled', 'disabled');
+        }
     }
 
     function calculate_results(handler_name, disable_submit) {
@@ -59,12 +70,9 @@ function MentoringStandardView(runtime, element, mentoring) {
         if (submitXHR) {
             submitXHR.abort();
         }
-        submitXHR = $.post(handlerUrl, JSON.stringify(data)).success(handleSubmitResults).error(handleSubmitError);
-
-        if (disable_submit) {
-            var disable_submit_callback = function(){ submitDOM.attr('disabled', 'disabled'); };
-            submitXHR.success(disable_submit_callback).error(disable_submit_callback);
-        }
+        submitXHR = $.post(handlerUrl, JSON.stringify(data))
+            .success(function(response) { handleSubmitResults(response, disable_submit); })
+            .error(function(jqXHR, textStatus, errorThrown) { handleSubmitError(jqXHR, textStatus, errorThrown, disable_submit); });
     }
 
     function get_results(){

problem_builder/tests/integration/test_mentoring.py

@@ -146,20 +146,17 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
             if checkbox.is_selected():
                 checkbox.click()
 
-    def _standard_checks(self, answer, mcq, mrq, rating, messages, only_selected=False):
+    def _standard_checks(self, answer, mcq, mrq, rating, messages):
         self.assertEqual(answer.get_attribute('value'), 'This is the answer')
         self._assert_feedback_showed(mcq, 0, "Great!")
         self._assert_feedback_showed(
             mrq, 0, "This is something everyone has to like about this MRQ",
             click_choice_result=True
         )
-        if not only_selected:
         self._assert_feedback_showed(
             mrq, 1, "This is something everyone has to like about beauty",
             click_choice_result=True, success=False
         )
-        else:
-            self._assert_feedback_hidden(mrq, 1)
         self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
         self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False)
         self._assert_feedback_showed(rating, 3, "I love good grades.", click_choice_result=True)
@@ -170,6 +167,7 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         mentoring = self.load_scenario("feedback_persistence.xml")
         answer, mcq, mrq, rating = self._get_controls(mentoring)
         messages = self._get_messages_element(mentoring)
+        submit = mentoring.find_element_by_css_selector('.submit input.input-main')
 
         answer_checkmark = answer.find_element_by_xpath("parent::*").find_element_by_css_selector(".answer-checkmark")
 
@@ -181,6 +179,7 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         for i in range(5):
             self._assert_feedback_hidden(rating, i)
         self.assertFalse(messages.is_displayed())
+        self.assertFalse(submit.is_enabled())
 
     def test_persists_feedback_on_page_reload(self):
         mentoring = self.load_scenario("feedback_persistence.xml")
@@ -195,7 +194,15 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         mentoring = self.go_to_view("student_view")
         answer, mcq, mrq, rating = self._get_controls(mentoring)
         messages = self._get_messages_element(mentoring)
-        self._standard_checks(answer, mcq, mrq, rating, messages, only_selected=True)
+        submit = mentoring.find_element_by_css_selector('.submit input.input-main')
+
+        self._standard_checks(answer, mcq, mrq, rating, messages)
+        # after reloading submit is disabled...
+        self.assertFalse(submit.is_enabled())
+
+        # ...until some changes are done
+        self.click_choice(mrq, "Its elegance")
+        self.assertTrue(submit.is_enabled())
 
     def test_given_perfect_score_in_past_loads_current_result(self):
         mentoring = self.load_scenario("feedback_persistence.xml")
@@ -237,4 +244,42 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         mentoring = self.go_to_view("student_view")
         answer, mcq, mrq, rating = self._get_controls(mentoring)
         messages = self._get_messages_element(mentoring)
-        self._standard_checks(answer, mcq, mrq, rating, messages, only_selected=True)
+        self._standard_checks(answer, mcq, mrq, rating, messages)
+
+    def test_partial_mrq_is_not_completed(self):
+        mentoring = self.load_scenario("feedback_persistence.xml")
+        answer, mcq, mrq, rating = self._get_controls(mentoring)
+        messages = self._get_messages_element(mentoring)
+
+        answer.send_keys('This is the answer')
+        self.click_choice(mcq, "Yes")
+        # 1st, 3rd and 4th options, first three are correct, i.e. two mistakes: 2nd and 4th
+        self.click_choice(mrq, "Its elegance")
+        self.click_choice(mrq, "Its gracefulness")
+        self.click_choice(rating, "4")
+        self.click_submit(mentoring)
+
+        def assert_state(answer, mcq, mrq, rating, messages):
+            self.assertEqual(answer.get_attribute('value'), 'This is the answer')
+            self._assert_feedback_showed(mcq, 0, "Great!")
+            self._assert_feedback_showed(
+                mrq, 0, "This is something everyone has to like about this MRQ",
+                click_choice_result=True
+            )
+            self._assert_feedback_showed(
+                mrq, 1, "This is something everyone has to like about beauty",
+                click_choice_result=True, success=False
+            )
+            self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
+            self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True)
+            self._assert_feedback_showed(rating, 3, "I love good grades.", click_choice_result=True)
+            self.assertTrue(messages.is_displayed())
+            self.assertEqual(messages.text, "FEEDBACK\nNot done yet")
+
+        assert_state(answer, mcq, mrq, rating, messages)
+
+        # now, reload the page and make sure the same result is shown
+        mentoring = self.go_to_view("student_view")
+        answer, mcq, mrq, rating = self._get_controls(mentoring)
+        messages = self._get_messages_element(mentoring)
+        assert_state(answer, mcq, mrq, rating, messages)