Merge pull request #13933 from edx/aj/TNL-1955_save_ui

Initial set of changes for ui correctness state when saving a problem

common/lib/capa/capa/capa_problem.py

@@ -143,6 +143,7 @@ class LoncapaProblem(object):
             state (dict): containing the following keys:
                 - `seed` (int) random number generator seed
                 - `student_answers` (dict) maps input id to the stored answer for that input
+                - 'has_saved_answers' (Boolean) True if the answer has been saved since last submit.
                 - `correct_map` (CorrectMap) a map of each input to their 'correctness'
                 - `done` (bool) indicates whether or not this problem is considered done
                 - `input_state` (dict) maps input_id to a dictionary that holds the state for that input
@@ -165,6 +166,7 @@ class LoncapaProblem(object):
         assert self.seed is not None, "Seed must be provided for LoncapaProblem."
 
         self.student_answers = state.get('student_answers', {})
+        self.has_saved_answers = state.get('has_saved_answers', False)
         if 'correct_map' in state:
             self.correct_map.set_dict(state['correct_map'])
         self.done = state.get('done', False)
@@ -257,6 +259,7 @@ class LoncapaProblem(object):
         Reset internal state to unfinished, with no answers
         """
         self.student_answers = dict()
+        self.has_saved_answers = False
         self.correct_map = CorrectMap()
         self.done = False
 
@@ -283,6 +286,7 @@ class LoncapaProblem(object):
 
         return {'seed': self.seed,
                 'student_answers': self.student_answers,
+                'has_saved_answers': self.has_saved_answers,
                 'correct_map': self.correct_map.get_dict(),
                 'input_state': self.input_state,
                 'done': self.done}
@@ -789,8 +793,14 @@ class LoncapaProblem(object):
             answervariable = None
             if problemid in self.correct_map:
                 pid = input_id
-                status = self.correct_map.get_correctness(pid)
-                msg = self.correct_map.get_msg(pid)
+
+                # If the the problem has not been saved since the last submit set the status to the
+                # current correctness value and set the message as expected. Otherwise we do not want to
+                # display correctness because the answer may have changed since the problem was graded.
+                if not self.has_saved_answers:
+                    status = self.correct_map.get_correctness(pid)
+                    msg = self.correct_map.get_msg(pid)
+
                 hint = self.correct_map.get_hint(pid)
                 hintmode = self.correct_map.get_hintmode(pid)
                 answervariable = self.correct_map.get_property(pid, 'answervariable')
@@ -810,6 +820,7 @@ class LoncapaProblem(object):
                 'input_state': self.input_state[input_id],
                 'answervariable': answervariable,
                 'response_data': response_data,
+                'has_saved_answers': self.has_saved_answers,
                 'feedback': {
                     'message': msg,
                     'hint': hint,

common/lib/capa/capa/templates/choicegroup.html

@@ -19,21 +19,13 @@
             <%
               label_class = 'response-label field-label label-inline'
             %>
+
             <label id="${id}-${choice_id}-label"
                 ## If the student has selected this choice...
                 % if is_radio_input(choice_id):
-                <%
-                    if status == 'correct':
-                        correctness = 'correct'
-                    elif status == 'partially-correct':
-                        correctness = 'partially-correct'
-                    elif status == 'incorrect':
-                        correctness = 'incorrect'
-                    else:
-                        correctness = None
-                %>
-                    % if correctness and not show_correctness == 'never':
-                        <% label_class += ' choicegroup_' + correctness %>
+
+                    % if status.classname and not show_correctness == 'never':
+                        <% label_class += ' choicegroup_' + status.classname %>
                     % endif
                 % endif
                 class="${label_class}"
@@ -47,7 +39,6 @@
                 checked="true"
                 % endif
                 /> ${HTML(choice_label)}
-
                 % if is_radio_input(choice_id):
                   % if not show_correctness == 'never' and status.classname != 'unanswered':
                     <%include file="status_span.html" args="status=status, status_id=id"/>

common/lib/capa/capa/templates/choicetext.html

@@ -19,19 +19,9 @@ from openedx.core.djangolib.markup import HTML
         <% choice_id = choice_id %>
         <section id="forinput${choice_id}"
             % if input_type == 'radio' and choice_id in value :
-            <%
-                if status == 'correct':
-                    correctness = 'correct'
-                elif status == 'incorrect':
-                    correctness = 'incorrect'
-                elif status == 'partially-correct':
-                    correctness = 'partially-correct'
-                else:
-                    correctness = None
-            %>
-            % if correctness:
-                class="choicetextgroup_${correctness}"
-            % endif
+                % if status.classname:
+                    class="choicetextgroup_${status.classname}"
+                % endif
             % endif
             >
             <input class="ctinput" type="${input_type}" name="choiceinput_${id}" id="${choice_id}" value="${choice_id}"

common/lib/capa/capa/tests/test_input_templates.py

@@ -1084,7 +1084,7 @@ class ChoiceTextGroupTemplateTest(TemplateTestCase):
         conditions = [
             {'input_type': 'radio', 'value': self.VALUE_DICT}]
 
-        self.context['status'] = 'correct'
+        self.context['status'] = Status('correct')
 
         for test_conditions in conditions:
             self.context.update(test_conditions)
@@ -1104,7 +1104,7 @@ class ChoiceTextGroupTemplateTest(TemplateTestCase):
         conditions = [
             {'input_type': 'radio', 'value': self.VALUE_DICT}]
 
-        self.context['status'] = 'incorrect'
+        self.context['status'] = Status('incorrect')
 
         for test_conditions in conditions:
             self.context.update(test_conditions)

common/lib/capa/capa/tests/test_inputtypes.py

@@ -705,17 +705,13 @@ class MatlabTest(unittest.TestCase):
         self.assertEqual(
             etree.tostring(output),
             textwrap.dedent("""
-            <div>{\'status\': Status(\'queued\'), \'button_enabled\': True,
-            \'rows\': \'10\', \'queue_len\': \'3\', \'mode\': \'\',
-            \'tabsize\': 4, \'cols\': \'80\',
-            \'STATIC_URL\': \'/dummy-static/\', \'linenumbers\': \'true\', \'queue_msg\': \'\',
-            \'value\': \'print "good evening"\',
-            \'msg\': u\'Submitted. As soon as a response is returned,
-            this message will be replaced by that feedback.\',
+            <div>{\'status\': Status(\'queued\'), \'button_enabled\': True, \'rows\': \'10\', \'queue_len\': \'3\',
+            \'mode\': \'\', \'tabsize\': 4, \'cols\': \'80\', \'STATIC_URL\': \'/dummy-static/\', \'linenumbers\':
+            \'true\', \'queue_msg\': \'\', \'value\': \'print "good evening"\',
+            \'msg\': u\'Submitted. As soon as a response is returned, this message will be replaced by that feedback.\',
             \'matlab_editor_js\': \'/dummy-static/js/vendor/CodeMirror/octave.js\',
             \'hidden\': \'\', \'id\': \'prob_1_2\',
-            \'describedby_html\': Markup(u\'aria-describedby="status_prob_1_2"\'),
-            \'response_data\': {}}</div>
+            \'describedby_html\': Markup(u\'aria-describedby="status_prob_1_2"\'), \'response_data\': {}}</div>
             """).replace('\n', ' ').strip()
         )
 

common/lib/xmodule/xmodule/capa_base.py

@@ -162,6 +162,8 @@ class CapaFields(object):
                        scope=Scope.user_state, default={})
     input_state = Dict(help=_("Dictionary for maintaining the state of inputtypes"), scope=Scope.user_state)
     student_answers = Dict(help=_("Dictionary with the current student responses"), scope=Scope.user_state)
+    has_saved_answers = Boolean(help=_("Whether or not the answers have been saved since last submit"),
+                                scope=Scope.user_state)
     done = Boolean(help=_("Whether the student has answered the problem"), scope=Scope.user_state)
     seed = Integer(help=_("Random seed for this student"), scope=Scope.user_state)
     last_submission_time = Date(help=_("Last submission time"), scope=Scope.user_state)
@@ -326,6 +328,7 @@ class CapaMixin(CapaFields):
             'done': self.done,
             'correct_map': self.correct_map,
             'student_answers': self.student_answers,
+            'has_saved_answers': self.has_saved_answers,
             'input_state': self.input_state,
             'seed': self.seed,
         }
@@ -339,6 +342,7 @@ class CapaMixin(CapaFields):
         self.correct_map = lcp_state['correct_map']
         self.input_state = lcp_state['input_state']
         self.student_answers = lcp_state['student_answers']
+        self.has_saved_answers = lcp_state['has_saved_answers']
         self.seed = lcp_state['seed']
 
     def set_last_submission_time(self):
@@ -675,6 +679,12 @@ class CapaMixin(CapaFields):
         answer_notification_type, answer_notification_message = self._get_answer_notification(
             render_notifications=submit_notification)
 
+        save_message = None
+        if self.has_saved_answers:
+            save_message = _(
+                "Your answers were previously saved. Click '{button_name}' to grade them."
+            ).format(button_name=self.submit_button_name())
+
         context = {
             'problem': content,
             'id': self.location.to_deprecated_string(),
@@ -691,6 +701,8 @@ class CapaMixin(CapaFields):
             'should_enable_next_hint': should_enable_next_hint,
             'answer_notification_type': answer_notification_type,
             'answer_notification_message': answer_notification_message,
+            'has_saved_answers': self.has_saved_answers,
+            'save_message': save_message,
         }
 
         html = self.runtime.render_template('problem.html', context)
@@ -1080,6 +1092,7 @@ class CapaMixin(CapaFields):
         event_info['state'] = self.lcp.get_state()
         event_info['problem_id'] = self.location.to_deprecated_string()
 
+        self.lcp.has_saved_answers = False
         answers = self.make_dict_of_responses(data)
         answers_without_files = convert_files_to_filenames(answers)
         event_info['answers'] = answers_without_files
@@ -1490,6 +1503,7 @@ class CapaMixin(CapaFields):
             }
 
         self.lcp.student_answers = answers
+        self.lcp.has_saved_answers = True
 
         self.set_state_from_lcp()
 

common/lib/xmodule/xmodule/js/src/capa/display.js

@@ -779,6 +779,7 @@
                         edx.HtmlUtils.HTML(saveMessage)
                     );
                     that.clear_all_notifications();
+                    that.el.find('.wrapper-problem-response .message').hide();
                     that.saveNotification.show();
                     that.focus_on_save_notification();
                 } else {
@@ -938,7 +939,7 @@
                 return $(element).find('input').on('input', function() {
                     var $p;
                     $p = $(element).find('span.status');
-                    return $p.parent().removeClass().addClass('unsubmitted');
+                    return $p.parent().removeAttr('class').addClass('unsubmitted');
                 });
             },
             choicegroup: function(element) {
@@ -949,7 +950,7 @@
                     var $status;
                     $status = $('#status_' + id);
                     if ($status[0]) {
-                        $status.removeClass().addClass('unanswered');
+                        $status.removeAttr('class').addClass('unanswered');
                     } else {
                         $('<span>', {
                             class: 'unanswered',
@@ -957,7 +958,7 @@
                             id: 'status_' + id
                         });
                     }
-                    return $element.find('label').removeClass();
+                    return $element.find('label').removeAttr('class');
                 });
             },
             'option-input': function(element) {
@@ -965,7 +966,7 @@
                 $select = $(element).find('select');
                 id = ($select.attr('id').match(/^input_(.*)$/))[1];
                 return $select.on('change', function() {
-                    return $('#status_' + id).removeClass().addClass('unanswered')
+                    return $('#status_' + id).removeAttr('class').addClass('unanswered')
                         .find('.sr')
                         .text(gettext('unsubmitted'));
                 });

common/test/acceptance/pages/lms/problem.py

@@ -34,6 +34,13 @@ class ProblemPage(PageObject):
         return self.q(css="div.problem p").text
 
     @property
+    def problem_input_content(self):
+        """
+        Return the text of the question of the problem.
+        """
+        return self.q(css="div.wrapper-problem-response").text[0]
+
+    @property
     def problem_content(self):
         """
         Return the content of the problem
@@ -144,6 +151,12 @@ class ProblemPage(PageObject):
         """
         return self.q(css='.notification.notification-hint').visible
 
+    def is_feedback_message_notification_visible(self):
+        """
+        Is the Feedback Messaged notification visible
+        """
+        return self.q(css='.wrapper-problem-response .message').visible
+
     def is_save_notification_visible(self):
         """
         Is the Save Notification Visible?
@@ -156,6 +169,13 @@ class ProblemPage(PageObject):
         """
         return self.q(css='.notification.success.notification-submit').visible
 
+    def wait_for_feedback_message_visibility(self):
+        """
+        Wait for the Feedback Message notification to be visible.
+        """
+        self.wait_for_element_visibility('.wrapper-problem-response .message',
+                                         'Waiting for the Feedback message to be visible')
+
     def wait_for_save_notification(self):
         """
         Wait for the Save Notification to be present
@@ -237,6 +257,15 @@ class ProblemPage(PageObject):
         msg = "Wait for status to be {}".format(message)
         self.wait_for_element_visibility(status_selector, msg)
 
+    def is_expected_status_visible(self, status_selector):
+        """
+        check for the expected status indicator to be visible.
+
+        Args:
+            status_selector(str): status selector string.
+        """
+        return self.q(css=status_selector).visible
+
     def wait_success_notification(self):
         """
         Check for visibility of the success notification and icon.

common/test/acceptance/tests/lms/test_lms_courseware.py

@@ -764,14 +764,16 @@ class ProblemStateOnNavigationTest(UniqueCourseTest):
         self.problem_page.wait_for_save_notification()
 
         # Save problem 1's content state as we're about to switch units in the sequence.
-        problem1_content_before_switch = self.problem_page.problem_content
+        problem1_content_before_switch = self.problem_page.problem_input_content
 
         # Go to sequential position 2 and assert that we are on problem 2.
         self.go_to_tab_and_assert_problem(2, self.problem2_name)
 
+        self.problem_page.wait_for_expected_status('span.unanswered', 'unanswered')
+
         # Come back to our original unit in the sequence and assert that the content hasn't changed.
         self.go_to_tab_and_assert_problem(1, self.problem1_name)
-        problem1_content_after_coming_back = self.problem_page.problem_content
+        problem1_content_after_coming_back = self.problem_page.problem_input_content
         self.assertIn(problem1_content_after_coming_back, problem1_content_before_switch)
 
     def test_perform_problem_reset_and_navigate(self):

common/test/acceptance/tests/lms/test_lms_problems.py

@@ -225,6 +225,94 @@ class ProblemNotificationTests(ProblemsTest):
         self.assertFalse(problem_page.is_save_notification_visible())
 
 
+class ProblemFeedbackNotificationTests(ProblemsTest):
+    """
+    Tests that the feedback notifications are visible when expected.
+    """
+
+    def get_problem(self):
+        """
+        Problem structure.
+        """
+        xml = dedent("""
+            <problem>
+                <label>Which of the following countries has the largest population?</label>
+                    <multiplechoiceresponse>
+                      <choicegroup type="MultipleChoice">
+                        <choice correct="false">Brazil <choicehint>timely feedback -- explain why an almost correct answer is wrong</choicehint></choice>
+                        <choice correct="false">Germany</choice>
+                        <choice correct="true">Indonesia</choice>
+                        <choice correct="false">Russia</choice>
+                      </choicegroup>
+                    </multiplechoiceresponse>
+            </problem>
+        """)
+        return XBlockFixtureDesc('problem', 'TEST PROBLEM', data=xml,
+                                 metadata={'max_attempts': 10},
+                                 grader_type='Final Exam')
+
+    def test_feedback_notification_hides_after_save(self):
+        self.courseware_page.visit()
+        problem_page = ProblemPage(self.browser)
+        problem_page.click_choice("choice_0")
+        problem_page.click_submit()
+        problem_page.wait_for_feedback_message_visibility()
+        problem_page.click_choice("choice_1")
+        problem_page.click_save()
+        self.assertFalse(problem_page.is_feedback_message_notification_visible())
+
+
+class ProblemSaveStatusUpdateTests(ProblemsTest):
+    """
+    Tests the problem status updates correctly with an answer change and save.
+    """
+    def get_problem(self):
+        """
+        Problem structure.
+        """
+        xml = dedent("""
+            <problem>
+                <label>Which of the following countries has the largest population?</label>
+                    <multiplechoiceresponse>
+                      <choicegroup type="MultipleChoice">
+                        <choice correct="false">Brazil <choicehint>timely feedback -- explain why an almost correct answer is wrong</choicehint></choice>
+                        <choice correct="false">Germany</choice>
+                        <choice correct="true">Indonesia</choice>
+                        <choice correct="false">Russia</choice>
+                      </choicegroup>
+                    </multiplechoiceresponse>
+            </problem>
+        """)
+        return XBlockFixtureDesc('problem', 'TEST PROBLEM', data=xml,
+                                 metadata={'max_attempts': 10},
+                                 grader_type='Final Exam')
+
+    def test_status_removed_after_save_before_submit(self):
+        """
+        Scenario: User should see the status removed when saving after submitting an answer and reloading the page.
+        Given that I have loaded the problem page
+        And a choice has been selected and submitted
+        When I change the choice
+        And Save the problem
+        And reload the problem page
+        Then I should see the save notification and I should not see any indication of problem status
+        """
+        self.courseware_page.visit()
+        problem_page = ProblemPage(self.browser)
+        problem_page.click_choice("choice_1")
+        problem_page.click_submit()
+        problem_page.wait_incorrect_notification()
+        problem_page.wait_for_expected_status('label.choicegroup_incorrect', 'incorrect')
+        problem_page.click_choice("choice_2")
+        self.assertFalse(problem_page.is_expected_status_visible('label.choicegroup_incorrect'))
+        problem_page.click_save()
+        problem_page.wait_for_save_notification()
+        # Refresh the page and the status should not be added
+        self.courseware_page.visit()
+        self.assertFalse(problem_page.is_expected_status_visible('label.choicegroup_incorrect'))
+        self.assertTrue(problem_page.is_save_notification_visible())
+
+
 class ProblemSubmitButtonMaxAttemptsTest(ProblemsTest):
     """
     Tests that the Submit button disables after the number of max attempts is reached.

lms/templates/problem.html

@@ -74,6 +74,7 @@ from openedx.core.djangolib.markup import HTML
                 notification_type='success',
                 notification_icon='fa-check',
                 notification_name='submit',
+                is_hidden=False,
                 notification_message=answer_notification_message"
             />
         % endif
@@ -82,6 +83,7 @@ from openedx.core.djangolib.markup import HTML
                 notification_type='error',
                 notification_icon='fa-close',
                 notification_name='submit',
+                is_hidden=False,
                 notification_message=answer_notification_message"
             />
         % endif
@@ -90,6 +92,7 @@ from openedx.core.djangolib.markup import HTML
                 notification_type='success',
                 notification_icon='fa-asterisk',
                 notification_name='submit',
+                is_hidden=False,
                 notification_message=answer_notification_message"
             />
         % endif
@@ -98,6 +101,7 @@ from openedx.core.djangolib.markup import HTML
       notification_type='warning',
       notification_icon='fa-save',
       notification_name='save',
-      notification_message=''"
+      notification_message=save_message,
+      is_hidden=not has_saved_answers"
     />
 </div>

lms/templates/problem_notifications.html

 <%page expression_filter="h" args="notification_name, notification_type, notification_icon,
-                                   notification_message, should_enable_next_hint"/>
+                                   notification_message, should_enable_next_hint, is_hidden=True"/>
 <%! from django.utils.translation import ugettext as _ %>
 
 <div class="notification ${notification_type} ${'notification-'}${notification_name}
-      ${'' if notification_name == 'submit' else 'is-hidden' }"
+      ${'' if not is_hidden else 'is-hidden' }"
      tabindex="-1">
     <span class="icon fa ${notification_icon}" aria-hidden="true"></span>
     <span class="notification-message" aria-describedby="${ short_id }-problem-title">${notification_message}