Merge pull request #12068 from edx/mrehan/sust-40-reduce-peroblem-get-ajax

SUST-40 – Eliminate unwanted problem_get ajax requests flooding the LMS.

common/lib/xmodule/xmodule/capa_base.py

@@ -399,6 +399,7 @@ class CapaMixin(CapaFields):
             'ajax_url': self.runtime.ajax_url,
             'progress_status': Progress.to_js_status_str(progress),
             'progress_detail': Progress.to_js_detail_str(progress),
+            'content': self.get_problem_html(encapsulate=False),
         })
 
     def check_button_name(self):
@@ -1422,6 +1423,7 @@ class CapaMixin(CapaFields):
         return {
             'success': True,
             'msg': msg,
+            'html': self.get_problem_html(encapsulate=False),
         }
 
     def reset_problem(self, _data):

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

@@ -5,6 +5,7 @@ class @Problem
     @id = @el.data('problem-id')
     @element_id = @el.attr('id')
     @url = @el.data('url')
+    @content = @el.data('content')
 
     # has_timed_out and has_response are used to ensure that are used to
     # ensure that we wait a minimum of ~ 1s before transitioning the check
@@ -12,7 +13,7 @@ class @Problem
     @has_timed_out = false
     @has_response = false
 
-    @render()
+    @render(@content)
 
   $: (selector) ->
     $(selector, @el)
@@ -316,6 +317,7 @@ class @Problem
       switch response.success
         when 'incorrect', 'correct'
           window.SR.readElts($(response.contents).find('.status'))
+          @el.trigger('contentChanged', [@id, response.contents])
           @render(response.contents)
           @updateProgress response
           if @el.hasClass 'showed'
@@ -331,6 +333,7 @@ class @Problem
   reset_internal: =>
     Logger.log 'problem_reset', @answers
     $.postWithPrefix "#{@url}/problem_reset", id: @id, (response) =>
+        @el.trigger('contentChanged', [@id, response.html])
         @render(response.html)
         @updateProgress response
 
@@ -419,6 +422,8 @@ class @Problem
     Logger.log 'problem_save', @answers
     $.postWithPrefix "#{@url}/problem_save", @answers, (response) =>
       saveMessage = response.msg
+      if response.success
+        @el.trigger('contentChanged', [@id, response.html])
       @gentle_alert saveMessage
       @updateProgress response
 

common/lib/xmodule/xmodule/js/src/sequence/display.coffee

 class @Sequence
   constructor: (element) ->
+    @updatedProblems = {}
     @requestToken = $(element).data('request-token')
     @el = $(element).find('.sequence')
     @contents = @$('.seq_contents')
@@ -40,6 +41,33 @@ class @Sequence
     if position_link and position_link.data('page-title')
         document.title = position_link.data('page-title') + @base_page_title
 
+  hookUpContentStateChangeEvent: ->
+    $('.problems-wrapper').bind(
+      'contentChanged',
+      (event, problem_id, new_content_state) =>
+        @addToUpdatedProblems problem_id, new_content_state
+    )
+
+  addToUpdatedProblems: (problem_id, new_content_state) =>
+    # Used to keep updated problem's state temporarily.
+    # params:
+    #   'problem_id' is problem id.
+    #   'new_content_state' is updated problem's state.
+
+    # initialize for the current sequence if there isn't any updated problem
+    # for this position.
+    if not @anyUpdatedProblems @position
+      @updatedProblems[@position] = {}
+
+    # Now, put problem content against problem id for current active sequence.
+    @updatedProblems[@position][problem_id] = new_content_state
+
+  anyUpdatedProblems:(position) ->
+    # check for the updated problems for given sequence position.
+    # params:
+    #   'position' can be any sequence position.
+    return @updatedProblems[position] != undefined
+
   hookUpProgressEvent: ->
     $('.problems-wrapper').bind 'progressChanged', @updateProgress
 
@@ -129,12 +157,21 @@ class @Sequence
 
       bookmarked = if @el.find('.active .bookmark-icon').hasClass('bookmarked') then true else false
       @content_container.html(current_tab.text()).attr("aria-labelledby", current_tab.attr("aria-labelledby")).data('bookmarked', bookmarked)
+
+      # update the data-attributes with latest contents only for updated problems.
+      if @anyUpdatedProblems new_position
+        $.each @updatedProblems[new_position], (problem_id, latest_content) =>
+          @content_container
+          .find("[data-problem-id='#{ problem_id }']")
+          .data('content', latest_content)
+
       XBlock.initializeBlocks(@content_container, @requestToken)
 
       window.update_schematics() # For embedded circuit simulator exercises in 6.002x
 
       @position = new_position
       @toggleArrows()
+      @hookUpContentStateChangeEvent()
       @hookUpProgressEvent()
       @updatePageTitle()
 

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

@@ -30,6 +30,13 @@ class ProblemPage(PageObject):
         return self.q(css="div.problem p").text
 
     @property
+    def problem_content(self):
+        """
+        Return the content of the problem
+        """
+        return self.q(css="div.problems-wrapper").text[0]
+
+    @property
     def message_text(self):
         """
         Return the "message" text of the question of the problem.
@@ -103,17 +110,42 @@ class ProblemPage(PageObject):
 
     def click_check(self):
         """
-        Click the Check button!
+        Click the Check button.
         """
         self.q(css='div.problem button.check').click()
         self.wait_for_ajax()
 
+    def click_save(self):
+        """
+        Click the Save button.
+        """
+        self.q(css='div.problem button.save').click()
+        self.wait_for_ajax()
+
+    def click_reset(self):
+        """
+        Click the Reset button.
+        """
+        self.q(css='div.problem button.reset').click()
+        self.wait_for_ajax()
+
     def wait_for_status_icon(self):
         """
         wait for status icon
         """
         self.wait_for_element_visibility('div.problem section.inputtype div .status', 'wait for status icon')
 
+    def wait_for_expected_status(self, status_selector, message):
+        """
+        Waits for the expected status indicator.
+
+        Args:
+            status_selector(str): status selector string.
+            message(str): description of promise, to be logged.
+        """
+        msg = "Wait for status to be {}".format(message)
+        self.wait_for_element_visibility(status_selector, msg)
+
     def click_hint(self):
         """
         Click the Hint button.

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

@@ -5,6 +5,7 @@ End-to-end tests for the LMS.
 from nose.plugins.attrib import attr
 
 from ..helpers import UniqueCourseTest
+from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
 from ...pages.studio.auto_auth import AutoAuthPage
 from ...pages.lms.create_mode import ModeCreationPage
 from ...pages.studio.overview import CourseOutlinePage
@@ -542,3 +543,167 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest):
         self.courseware_page.a11y_audit.config.set_scope(
             include=['div.sequence-nav'])
         self.courseware_page.a11y_audit.check_for_accessibility_errors()
+
+
+class ProblemStateOnNavigationTest(UniqueCourseTest):
+    """
+    Test courseware with problems in multiple verticals
+    """
+    USERNAME = "STUDENT_TESTER"
+    EMAIL = "student101@example.com"
+
+    problem1_name = 'MULTIPLE CHOICE TEST PROBLEM 1'
+    problem2_name = 'MULTIPLE CHOICE TEST PROBLEM 2'
+
+    def setUp(self):
+        super(ProblemStateOnNavigationTest, self).setUp()
+
+        self.courseware_page = CoursewarePage(self.browser, self.course_id)
+
+        # Install a course with section, tabs and multiple choice problems.
+        course_fix = CourseFixture(
+            self.course_info['org'], self.course_info['number'],
+            self.course_info['run'], self.course_info['display_name']
+        )
+
+        course_fix.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section 1').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection 1,1').add_children(
+                    self.create_multiple_choice_problem(self.problem1_name),
+                    self.create_multiple_choice_problem(self.problem2_name),
+                ),
+            ),
+        ).install()
+
+        # Auto-auth register for the course.
+        AutoAuthPage(
+            self.browser, username=self.USERNAME, email=self.EMAIL,
+            course_id=self.course_id, staff=False
+        ).visit()
+
+        self.courseware_page.visit()
+        self.problem_page = ProblemPage(self.browser)
+
+    def create_multiple_choice_problem(self, problem_name):
+        """
+        Return the Multiple Choice Problem Descriptor, given the name of the problem.
+        """
+        factory = MultipleChoiceResponseXMLFactory()
+        xml_data = factory.build_xml(
+            question_text='The correct answer is Choice 2',
+            choices=[False, False, True, False],
+            choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3']
+        )
+
+        return XBlockFixtureDesc(
+            'problem',
+            problem_name,
+            data=xml_data,
+            metadata={'rerandomize': 'always'}
+        )
+
+    def go_to_tab_and_assert_problem(self, position, problem_name):
+        """
+        Go to sequential tab and assert that we are on problem whose name is given as a parameter.
+        Args:
+            position: Position of the sequential tab
+            problem_name: Name of the problem
+        """
+        self.courseware_page.go_to_sequential_position(position)
+        self.problem_page.wait_for_element_presence(
+            self.problem_page.CSS_PROBLEM_HEADER,
+            'wait for problem header'
+        )
+        self.assertEqual(self.problem_page.problem_name, problem_name)
+
+    def test_perform_problem_check_and_navigate(self):
+        """
+        Scenario:
+        I go to sequential position 1
+        Facing problem1, I select 'choice_1'
+        Then I click check button
+        Then I go to sequential position 2
+        Then I came back to sequential position 1 again
+        Facing problem1, I observe the problem1 content is not
+        outdated before and after sequence navigation
+        """
+        # Go to sequential position 1 and assert that we are on problem 1.
+        self.go_to_tab_and_assert_problem(1, self.problem1_name)
+
+        # Update problem 1's content state by clicking check button.
+        self.problem_page.click_choice('choice_choice_1')
+        self.problem_page.click_check()
+        self.problem_page.wait_for_expected_status('label.choicegroup_incorrect', 'incorrect')
+
+        # 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
+
+        # Go to sequential position 2 and assert that we are on problem 2.
+        self.go_to_tab_and_assert_problem(2, self.problem2_name)
+
+        # 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
+        self.assertEqual(problem1_content_before_switch, problem1_content_after_coming_back)
+
+    def test_perform_problem_save_and_navigate(self):
+        """
+        Scenario:
+        I go to sequential position 1
+        Facing problem1, I select 'choice_1'
+        Then I click save button
+        Then I go to sequential position 2
+        Then I came back to sequential position 1 again
+        Facing problem1, I observe the problem1 content is not
+        outdated before and after sequence navigation
+        """
+        # Go to sequential position 1 and assert that we are on problem 1.
+        self.go_to_tab_and_assert_problem(1, self.problem1_name)
+
+        # Update problem 1's content state by clicking save button.
+        self.problem_page.click_choice('choice_choice_1')
+        self.problem_page.click_save()
+        self.problem_page.wait_for_expected_status('div.capa_alert', 'saved')
+
+        # 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
+
+        # Go to sequential position 2 and assert that we are on problem 2.
+        self.go_to_tab_and_assert_problem(2, self.problem2_name)
+
+        # 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
+        self.assertIn(problem1_content_after_coming_back, problem1_content_before_switch)
+
+    def test_perform_problem_reset_and_navigate(self):
+        """
+        Scenario:
+        I go to sequential position 1
+        Facing problem1, I select 'choice_1'
+        Then perform the action – check and reset
+        Then I go to sequential position 2
+        Then I came back to sequential position 1 again
+        Facing problem1, I observe the problem1 content is not
+        outdated before and after sequence navigation
+        """
+        # Go to sequential position 1 and assert that we are on problem 1.
+        self.go_to_tab_and_assert_problem(1, self.problem1_name)
+
+        # Update problem 1's content state – by performing reset operation.
+        self.problem_page.click_choice('choice_choice_1')
+        self.problem_page.click_check()
+        self.problem_page.wait_for_expected_status('label.choicegroup_incorrect', 'incorrect')
+        self.problem_page.click_reset()
+        self.problem_page.wait_for_expected_status('span.unanswered', 'unanswered')
+
+        # 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
+
+        # Go to sequential position 2 and assert that we are on problem 2.
+        self.go_to_tab_and_assert_problem(2, self.problem2_name)
+
+        # 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
+        self.assertEqual(problem1_content_before_switch, problem1_content_after_coming_back)

lms/templates/problem_ajax.html

-<div id="problem_${element_id}" class="problems-wrapper" data-problem-id="${id}" data-url="${ajax_url}" data-progress_status="${progress_status}" data-progress_detail="${progress_detail}"></div>
+<div id="problem_${element_id}" class="problems-wrapper" data-problem-id="${id}" data-url="${ajax_url}" data-progress_status="${progress_status}" data-progress_detail="${progress_detail}" data-content="${content | h}"></div>