Update tests

problem_builder/mentoring.py

@@ -962,6 +962,9 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
     @property
     def review_tips(self):
         """ Get review tips, shown for wrong answers. """
+        if self.max_attempts > 0 and self.num_attempts >= self.max_attempts:
+            # Review tips are only shown if the student is allowed to try again.
+            return []
         review_tips = []
         status_cache = dict()
         steps = self.steps
@@ -973,7 +976,7 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
                 # The student got this wrong. Check if there is a review tip to show.
                 tip_html = question.get_review_tip()
                 if tip_html:
-                    if hasattr(self.runtime, 'replace_jump_to_id_urls'):
+                    if getattr(self.runtime, 'replace_jump_to_id_urls', None) is not None:
                         tip_html = self.runtime.replace_jump_to_id_urls(tip_html)
                     review_tips.append(tip_html)
         return review_tips
@@ -1057,9 +1060,8 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
             self.active_step = new_value
         elif new_value == len(self.step_ids):
             # The user just completed the final step.
-            # Update the number of attempts, if necessary:
-            if self.num_attempts < self.max_attempts:
-                self.num_attempts += 1
+            # Update the number of attempts:
+            self.num_attempts += 1
             # Do we need to render a review (summary of the user's score):
             if self.has_review_step:
                 self.active_step = -1

problem_builder/templates/html/sb-review-per-question-feedback.html

@@ -4,6 +4,6 @@
 <p class="review-tips-intro">{% trans "You might consider reviewing the following items before your next assessment attempt:" %}</p>
 <ul class="review-tips-list">
     {% for tip in tips %}
-        <li>{{tip}}</li>
+        <li>{{tip|safe}}</li>
     {% endfor %}
 </ul>

problem_builder/tests/integration/base_test.py

@@ -270,7 +270,7 @@ class MentoringAssessmentBaseTest(ProblemBuilderBaseTest):
         states[result] += 1
 
         for name, count in states.items():
-            self.assertEqual(len(mentoring.find_elements_by_css_selector(".checkmark-{}".format(name))), count)
+            self.assertEqual(len(mentoring.find_elements_by_css_selector(".submit .checkmark-{}".format(name))), count)
 
 
 class GetChoices(object):

problem_builder/tests/integration/test_slider.py

@@ -143,4 +143,4 @@ class SliderStepBlockTest(SliderBlockTestMixins, MentoringAssessmentBaseTest):
 
     def wait_for_init(self):
         """ Wait for the scenario to initialize """
-        self.wait_until_hidden(self.browser.find_element_by_css_selector('.assessment-review-tips'))
+        self.wait_until_visible(self.browser.find_elements_by_css_selector('.sb-step')[0])

problem_builder/tests/integration/test_step_builder.py

@@ -216,6 +216,11 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
                 self.assert_clickable(controls.review_link)
                 self.assert_hidden(controls.try_again)
 
+    def assert_review_conditional_messages_equal(self, step_builder, messages_expected):
+        """ Test that the Conditional Messages seen on the review match messages_expected. """
+        messages = step_builder.find_elements_by_css_selector('.review-conditional-message')
+        self.assertListEqual([msg.text for msg in messages], messages_expected)
+
     def peek_at_review(self, step_builder, controls, expected, extended_feedback=False):
         self.wait_until_text_in("You scored {percentage}% on this assessment.".format(**expected), step_builder)
 
@@ -302,10 +307,12 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         )
 
         # Step should display 5 checkmarks (4 correct items for MRQ, plus step-level feedback about correctness)
-        correct_marks = step_builder.find_elements_by_css_selector('.checkmark-correct')
-        incorrect_marks = step_builder.find_elements_by_css_selector('.checkmark-incorrect')
-        self.assertEqual(len(correct_marks), 5)
+        correct_marks = step_builder.find_elements_by_css_selector('.sb-step .checkmark-correct')
+        incorrect_marks = step_builder.find_elements_by_css_selector('.sb-step .checkmark-incorrect')
+        overall_mark = step_builder.find_elements_by_css_selector('.submit .checkmark-correct')
+        self.assertEqual(len(correct_marks), 4)
         self.assertEqual(len(incorrect_marks), 0)
+        self.assertEqual(len(overall_mark), 1)
 
         item_feedbacks = [
             "This is something everyone has to like about this MRQ",
@@ -416,18 +423,20 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
             self.assertIn("Question 1 and Question 3", step_builder.find_element_by_css_selector('.correct-list').text)
 
         if max_attempts == 1:
-            self.assert_message_text(step_builder, "On review message text")
+            self.assert_review_conditional_messages_equal(
+                step_builder,
+                ["Not quite!", "This message is shown when you run out of attempts."]
+            )
             self.assert_disabled(controls.try_again)
             return
 
-        self.assert_message_text(step_builder, "Block incomplete message text")
+        self.assert_review_conditional_messages_equal(step_builder, ["Not quite! You can try again, though."])
         self.assert_clickable(controls.try_again)
 
         # Try again
         controls.try_again.click()
 
         self.wait_until_hidden(controls.try_again)
-        self.assert_no_message_text(step_builder)
 
         # Step 1
         # Submit free-form answer, go to next step
@@ -464,15 +473,15 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
             )
 
         if max_attempts == 2:
+            self.assert_review_conditional_messages_equal(
+                step_builder,
+                ["Not quite!", "This message is shown when you run out of attempts."]
+            )
             self.assert_disabled(controls.try_again)
         else:
+            self.assert_review_conditional_messages_equal(step_builder, ["Not quite! You can try again, though."])
             self.assert_clickable(controls.try_again)
 
-        if 1 <= max_attempts <= 2:
-            self.assert_message_text(step_builder, "On review message text")
-        else:
-            self.assert_message_text(step_builder, "Block incomplete message text")
-
         if extended_feedback:
             self.extended_feedback_checks(step_builder, controls, expected_results)
 
@@ -492,15 +501,21 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         self.multiple_response_question(None, step_builder, controls, ("Its beauty",), PARTIAL, last=True)
 
         # The review tips for MCQ 2 and the MRQ should be shown:
-        review_tips = step_builder.find_element_by_css_selector('.assessment-review-tips')
+        review_tips_intro = step_builder.find_element_by_css_selector('.review-tips-intro')
+        self.assertEqual(
+            review_tips_intro.text,
+            "You might consider reviewing the following items before your next assessment attempt:"
+        )
+        review_tips = step_builder.find_element_by_css_selector('.review-tips-list')
         self.assertTrue(review_tips.is_displayed())
-        self.assertIn('You might consider reviewing the following items', review_tips.text)
-        self.assertIn('Take another look at', review_tips.text)
-        self.assertIn('Lesson 1', review_tips.text)
+        self.assertIn('Take another look at Lesson 1', review_tips.text)
         self.assertNotIn('Lesson 2', review_tips.text)  # This MCQ was correct
-        self.assertIn('Lesson 3', review_tips.text)
+        self.assertIn('Take another look at Lesson 3', review_tips.text)
         # If attempts remain and student got some answers wrong, show "incomplete" message
-        self.assert_message_text(step_builder, "Block incomplete message text")
+        self.assert_review_conditional_messages_equal(
+            step_builder,
+            ["Not quite! You can try again, though."]
+        )
 
         # Try again
         self.assert_clickable(controls.try_again)
@@ -516,9 +531,10 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         self.html_section(step_builder, controls)
         self.multiple_response_question(None, step_builder, controls, user_selection, CORRECT, last=True)
 
-        # If attempts remain and student got all answers right, show "complete" message
-        self.assert_message_text(step_builder, "Block completed message text")
-        self.assertFalse(review_tips.is_displayed())
+        # We expect to see this congratulatory message now:
+        self.assert_review_conditional_messages_equal(step_builder, ["Great job!"])
+        # And no review tips should be shown:
+        self.assertEqual(len(step_builder.find_elements_by_css_selector('.review-tips-intro')), 0)
 
         # Try again
         self.assert_clickable(controls.try_again)
@@ -534,16 +550,21 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         self.multiple_response_question(None, step_builder, controls, ("Its beauty",), PARTIAL, last=True)
 
         # The review tips will not be shown because no attempts remain:
-        self.assertFalse(review_tips.is_displayed())
+        self.assertEqual(len(step_builder.find_elements_by_css_selector('.review-tips-intro')), 0)
 
-    def test_default_messages(self):
+    @data(True, False)
+    def test_conditional_messages(self, include_messages):
+        """
+        Test that conditional messages in the review step are visible or not, as appropriate.
+        """
         max_attempts = 3
         extended_feedback = False
         params = {
             "max_attempts": max_attempts,
             "extended_feedback": extended_feedback,
+            "include_messages": include_messages,
         }
-        step_builder, controls = self.load_assessment_scenario("step_builder_default_messages.xml", params)
+        step_builder, controls = self.load_assessment_scenario("step_builder_conditional_messages.xml", params)
 
         # First attempt: incomplete (second question wrong)
 
@@ -562,14 +583,16 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         }
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
 
-        # Should show default message for incomplete submission
-        self.assert_message_text(step_builder, "Not quite! You can try again, though.")
+        # Should show the following message for incomplete submission
+        self.assert_review_conditional_messages_equal(
+            step_builder,
+            ["Not quite! You can try again, though."] if include_messages else []
+        )
 
         # Try again
         controls.try_again.click()
 
         self.wait_until_hidden(controls.try_again)
-        self.assert_no_message_text(step_builder)
 
         # Second attempt: complete (both questions correct)
 
@@ -589,14 +612,16 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         }
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
 
-        # Should show default message for complete submission
-        self.assert_message_text(step_builder, "Great job!")
+        # Should show the following message for perfect ("complete") submission
+        self.assert_review_conditional_messages_equal(
+            step_builder,
+            ["Great job!"] if include_messages else []
+        )
 
         # Try again
         controls.try_again.click()
 
         self.wait_until_hidden(controls.try_again)
-        self.assert_no_message_text(step_builder)
 
         # Last attempt: complete (both questions correct)
 
@@ -617,8 +642,11 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         }
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
 
-        # Should show default message for review
-        self.assert_message_text(step_builder, "Note: you have used all attempts. Continue to the next unit.")
+        # Should show the following messages:
+        self.assert_review_conditional_messages_equal(
+            step_builder,
+            ["Great job!", "Note: you have used all attempts. Continue to the next unit."] if include_messages else []
+        )
 
     def answer_rating_question(self, step_number, question_number, step_builder, question, choice_name):
         question_text = self.question_text(question_number)

problem_builder/tests/integration/xml_templates/slider_step.xml

@@ -11,6 +11,8 @@
     </pb-mcq>
   </sb-step>
 
-  <sb-review-step/>
+  <sb-review-step>
+    <sb-review-score/>
+  </sb-review-step>
 
 </step-builder>

problem_builder/tests/integration/xml_templates/step_builder.xml

@@ -30,7 +30,7 @@
       <pb-tip values='["notwant"]'>Your loss!</pb-tip>
       {% if include_review_tips %}
       <pb-message type="on-assessment-review-question">
-        <html>Take another look at <a href="#">Lesson 2</a></html>
+        Take another look at <a href="#">Lesson 2</a>
       </pb-message>
       {% endif %}
     </pb-rating>
@@ -52,24 +52,21 @@
       <pb-tip values='["bugs"]'>Nah, there aren't any!</pb-tip>
       {% if include_review_tips %}
       <pb-message type="on-assessment-review-question">
-        <html>Take another look at <a href="#">Lesson 3</a></html>
+        Take another look at <a href="#">Lesson 3</a>
       </pb-message>
       {% endif %}
     </pb-mrq>
   </sb-step>
 
   <sb-review-step>
-    <pb-message type="completed">
-      <html>Block completed message text</html>
-    </pb-message>
-
-    <pb-message type="incomplete">
-      <html>Block incomplete message text</html>
-    </pb-message>
-
-    <pb-message type="on-review">
-      <html>On review message text</html>
-    </pb-message>
+    <sb-conditional-message score_condition="imperfect" num_attempts_condition="can_try_again">Not quite! You can try again, though.</sb-conditional-message>
+    <sb-conditional-message score_condition="imperfect" num_attempts_condition="cannot_try_again">Not quite!</sb-conditional-message>
+    <sb-conditional-message score_condition="perfect">Great job!</sb-conditional-message>
+    <sb-conditional-message num_attempts_condition="cannot_try_again">This message is shown when you run out of attempts.</sb-conditional-message>
+    <sb-review-score/>
+    {% if include_review_tips %}
+    <sb-review-per-question-feedback/>
+    {% endif %}
   </sb-review-step>
 
 </step-builder>

problem_builder/tests/integration/xml_templates/step_builder_default_messages.xml → problem_builder/tests/integration/xml_templates/step_builder_conditional_messages.xml

@@ -13,6 +13,14 @@
     </pb-mcq>
   </sb-step>
 
-  <sb-review-step></sb-review-step>
+  <sb-review-step>
+    {% if include_messages %}
+      <sb-conditional-message score_condition="imperfect" num_attempts_condition="can_try_again">Not quite! You can try again, though.</sb-conditional-message>
+      <sb-conditional-message score_condition="perfect">Great job!</sb-conditional-message>
+      <sb-conditional-message num_attempts_condition="cannot_try_again">Note: you have used all attempts. Continue to the next unit.</sb-conditional-message>
+    {% endif %}
+    <sb-review-score/>
+    <sb-review-per-question-feedback/>
+  </sb-review-step>
 
 </step-builder>

problem_builder/tests/integration/xml_templates/step_builder_html_last.xml

@@ -21,6 +21,8 @@
     <html_demo>Bla bla bla</html_demo>
   </sb-step>
 
-  <sb-review-step />
+  <sb-review-step>
+    <html_demo>This is the review step.</html_demo>
+  </sb-review-step>
 
 </step-builder>

problem_builder/tests/integration/xml_templates/step_builder_next.xml

@@ -26,6 +26,8 @@
     </pb-rating>
   </sb-step>
 
-  <sb-review-step />
+  <sb-review-step>
+    <sb-review-score/>
+  </sb-review-step>
 
 </step-builder>