Merge pull request #894 from edx/mushtaq/tnl-4155-fix-multiple-ora-issue

Add support for multiple ORAs on a unit

openassessment/xblock/static/js/spec/lms/oa_peer.js

@@ -67,13 +67,15 @@ describe("OpenAssessment.PeerView", function() {
         var overallFeedback = "Good job!";
         view.rubric.overallFeedback(overallFeedback);
 
+        var uuid = view.getUUID();
+
         // Submit the peer assessment
         view.peerAssess();
 
         // Expect that the peer assessment was sent to the server
         // with the options and feedback we selected
         expect(server.peerAssess).toHaveBeenCalledWith(
-            optionsSelected, criterionFeedback, overallFeedback, ''
+            optionsSelected, criterionFeedback, overallFeedback, uuid
         );
     };
 

openassessment/xblock/static/js/src/lms/oa_base.js

@@ -26,6 +26,7 @@ OpenAssessment.BaseView = function(runtime, element, server, data) {
     this.messageView = new OpenAssessment.MessageView(this.element, this.server, this);
     // Staff-only area with information and tools for managing student submissions
     this.staffAreaView = new OpenAssessment.StaffAreaView(this.element, this.server, this);
+    this.usageID = '';
 };
 
 if (typeof OpenAssessment.unsavedChanges === 'undefined' || !OpenAssessment.unsavedChanges) {
@@ -73,6 +74,16 @@ OpenAssessment.BaseView.prototype = {
     },
 
     /**
+     * Get usage key of an XBlock.
+     */
+    getUsageID: function() {
+        if (!this.usageID) {
+            this.usageID = $(this.element).data('usage-id');
+        }
+        return this.usageID;
+    },
+
+    /**
      * Asynchronously load each sub-view into the DOM.
      */
     load: function() {

openassessment/xblock/static/js/src/lms/oa_peer.js

@@ -217,7 +217,7 @@ OpenAssessment.PeerView.prototype = {
     **/
     peerAssessRequest: function(successFunction) {
         var view = this;
-        var uuid = $('#openassessment__peer-assessment').data('submission-uuid');
+        var uuid = this.getUUID();
 
         view.baseView.toggleActionError('peer', null);
         view.peerSubmitEnabled(false);
@@ -234,5 +234,13 @@ OpenAssessment.PeerView.prototype = {
             view.baseView.toggleActionError('peer', errMsg);
             view.peerSubmitEnabled(true);
         });
+    },
+
+    /**
+    Get uuid of a peer assessment.
+    **/
+    getUUID: function() {
+        var xBlockElement = $("div[data-usage-id='" + this.baseView.getUsageID() + "']");
+        return xBlockElement.find('#openassessment__peer-assessment').data('submission-uuid');
     }
 };

test/acceptance/accessibility.py

@@ -3,7 +3,7 @@ UI-level acceptance tests for OpenAssessment accessibility.
 """
 import os
 import unittest
-from tests import OpenAssessmentTest, StaffAreaPage, FullWorkflowMixin
+from tests import OpenAssessmentTest, StaffAreaPage, FullWorkflowMixin, MultipleOpenAssessmentMixin
 
 
 class OpenAssessmentA11yTest(OpenAssessmentTest):
@@ -243,6 +243,55 @@ class FullWorkflowRequiredA11yTest(OpenAssessmentA11yTest, FullWorkflowMixin):
         self._check_a11y(self.staff_area_page)
 
 
+class MultipleOpenAssessmentA11yTest(OpenAssessmentA11yTest, MultipleOpenAssessmentMixin):
+    """
+    Test accessibility when we have a unit containing multiple ORA blocks.
+    """
+
+    def setUp(self):
+        super(MultipleOpenAssessmentA11yTest, self).setUp('multiple_ora', staff=True)
+        # Staff area page is not present in OpenAssessmentTest base class, so we are adding it here.
+        self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
+
+    # TODO: remove this method override. See TNL-1593
+    def _check_a11y(self, page):
+        page.a11y_audit.config.set_scope(
+            exclude=[
+                ".container-footer",
+                ".nav-skip",
+                "#global-navigation",
+            ],
+        )
+        page.a11y_audit.config.set_rules({
+            "ignore": [
+                "color-contrast",  # TODO: AC-198
+                "empty-heading",  # TODO: AC-197
+                "link-href",  # TODO: AC-199
+                "link-name",  # TODO: AC-196
+                "skip-link",  # TODO: AC-179
+                "duplicate-id",
+            ]
+        })
+        page.a11y_audit.check_for_accessibility_errors()
+
+    def test_multiple_ora_complete_flow(self):
+        """
+        Test accessibility when we have a unit containing multiple ORA blocks.
+        """
+        # Each problem has vertical index assigned and has a `vert-{vertical_index}` top level class.
+        # That also means that all pages are being differentiated by their vertical index number that is assigned to
+        # each problem type. We are passing vertical index number and setting it by `self.setup_vertical_index` method
+        # so as to move to a different problem.
+
+        # Assess first ORA problem, pass the vertical index number
+        self.assess_component(0)
+        self._check_a11y(self.peer_asmnt_page)
+
+        # Assess second ORA problem, pass the vertical index number
+        self.assess_component(1)
+        self._check_a11y(self.peer_asmnt_page)
+
+
 if __name__ == "__main__":
 
     # Configure the screenshot directory

test/acceptance/pages.py

@@ -19,6 +19,8 @@ class OpenAssessmentPage(PageObject):
     """
     Base class for ORA page objects.
     """
+    # vertical index is the index identifier of a problem component on a page.
+    vertical_index = 0
 
     def __init__(self, browser, problem_location):
         """
@@ -38,7 +40,15 @@ class OpenAssessmentPage(PageObject):
 
         The default implementation just returns the selector
         """
-        return selector
+        return "{vertical_index_class} {selector}".format(vertical_index_class=self.vertical_index_class, selector=selector)
+
+    @property
+    def vertical_index_class(self):
+        """
+        Every problem has a vertical index assigned to it, which creates a vertical index class like
+        `vert-{vertical_index}. If there is one problem on unit page, problem would have .vert-0 class attached to it.
+        """
+        return ".vert-{vertical_index}".format(vertical_index=self.vertical_index)
 
     @property
     def url(self):
@@ -53,13 +63,14 @@ class OpenAssessmentPage(PageObject):
         This relies on the fact that we use the same CSS styles for submit buttons
         in all problem steps (unless custom value for button_css is passed in).
         """
+        submit_button_selector = self._bounded_selector(button_css)
         EmptyPromise(
-            lambda: 'is--disabled' not in " ".join(self.q(css=self._bounded_selector(button_css)).attrs('class')),
+            lambda: 'is--disabled' not in " ".join(self.q(css=submit_button_selector).attrs('class')),
             "Submit button is enabled."
         ).fulfill()
 
         with self.handle_alert():
-            self.q(css=self._bounded_selector(button_css)).first.click()
+            self.q(css=submit_button_selector).first.click()
 
     def hide_django_debug_tool(self):
         if self.q(css='#djDebug').visible:
@@ -85,8 +96,9 @@ class SubmissionPage(OpenAssessmentPage):
             BrokenPromise: The response was not submitted successfully.
 
         """
-        self.wait_for_element_visibility("textarea.submission__answer__part__text__value", "Textarea is present")
-        self.q(css="textarea.submission__answer__part__text__value").fill(response_text)
+        textarea_element = self._bounded_selector("textarea.submission__answer__part__text__value")
+        self.wait_for_element_visibility(textarea_element, "Textarea is present")
+        self.q(css=textarea_element).fill(response_text)
         self.submit()
         EmptyPromise(lambda: self.has_submitted, 'Response is completed').fulfill()
 
@@ -96,7 +108,8 @@ class SubmissionPage(OpenAssessmentPage):
         Args:
          latex_query (unicode): Latex expression text
         """
-        self.wait_for_element_visibility("textarea.submission__answer__part__text__value", "Textarea is present")
+        textarea_element = self._bounded_selector("textarea.submission__answer__part__text__value")
+        self.wait_for_element_visibility(textarea_element, "Textarea is present")
         self.q(css="textarea.submission__answer__part__text__value").fill(latex_query)
 
     def preview_latex(self):
@@ -245,8 +258,8 @@ class AssessmentPage(OpenAssessmentPage, AssessmentMixin):
         """
         Return `selector`, but limited to this Assignment Page.
         """
-        return '#openassessment__{assessment_type} {selector}'.format(
-            assessment_type=self._assessment_type, selector=selector)
+        return super(AssessmentPage, self)._bounded_selector('#openassessment__{assessment_type} {selector}'.format(
+            assessment_type=self._assessment_type, selector=selector))
 
     def is_browser_on_page(self):
         css_id = "#openassessment__{assessment_type}".format(
@@ -270,10 +283,10 @@ class AssessmentPage(OpenAssessmentPage, AssessmentMixin):
         Returns:
             unicode
         """
-        css_sel = ".{assessment_type}__display .submission__answer__part__text__value>p".format(
+        css_sel = ".{assessment_type}__display .submission__answer__part__text__value".format(
             assessment_type=self._assessment_type
         )
-        return u" ".join(self.q(css=css_sel).text)
+        return u" ".join(self.q(css=self._bounded_selector(css_sel)).text)
 
     def wait_for_complete(self):
         """
@@ -359,7 +372,8 @@ class AssessmentPage(OpenAssessmentPage, AssessmentMixin):
         if self._assessment_type not in ['peer-assessment', 'student-training']:
             msg = "Only peer assessment and student training steps can retrieve the number completed"
             raise PageConfigurationError(msg)
-        candidates = [int(x) for x in self.q(css=".step__status__value--completed").text]
+        status_completed_css = self._bounded_selector(".step__status__value--completed")
+        candidates = [int(x) for x in self.q(css=status_completed_css).text]
         return candidates[0] if len(candidates) > 0 else None
 
     @property
@@ -413,7 +427,7 @@ class GradePage(OpenAssessmentPage):
         """
         Return `selector`, but limited to the student grade view.
         """
-        return '#openassessment__grade {}'.format(selector)
+        return super(GradePage, self)._bounded_selector('#openassessment__grade {selector}'.format(selector=selector))
 
     def is_browser_on_page(self):
         return self.q(css="#openassessment__grade").is_present()
@@ -518,7 +532,7 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
         """
         Return `selector`, but limited to the staff area management area.
         """
-        return '.openassessment__staff-area {}'.format(selector)
+        return super(StaffAreaPage, self)._bounded_selector('.openassessment__staff-area {}'.format(selector))
 
     def is_browser_on_page(self):
         return self.q(css=".openassessment__staff-area").is_present()
@@ -577,7 +591,7 @@ class StaffAreaPage(OpenAssessmentPage, AssessmentMixin):
         self.q(css=student_input_css).fill(username)
         submit_button = self.q(css=self._bounded_selector(".action--submit-username"))
         submit_button.first.click()
-        self.wait_for_element_visibility(".staff-info__student__report", "Student report is present")
+        self.wait_for_element_visibility(self._bounded_selector(".staff-info__student__report"), "Student report is present")
 
     def expand_staff_grading_section(self):
         """

test/acceptance/tests.py

@@ -78,6 +78,9 @@ class OpenAssessmentTest(WebAppTest):
         'feedback_only':
             u'courses/{test_course_id}/courseware/'
             u'8d9584d242b44343bc270ea5ef04ab03/a2875e0db1454d0b94728b9a7b28000b/'.format(test_course_id=TEST_COURSE_ID),
+        'multiple_ora':
+            u'courses/{test_course_id}/courseware/'
+            u'3b9aa6e06d8f48818ff6f364b5586f38/b79abd43bb11445486cd1874e6c71a64/'.format(test_course_id=TEST_COURSE_ID),
     }
 
     SUBMISSION = u"This is a test submission."
@@ -850,6 +853,38 @@ class FullWorkflowMixin(object):
 
         return learner
 
+    def staff_assessment(self, peer_grades_me=True):
+        """ Do staff assessment workflow """
+
+        # Ensure grade is not present, since staff assessment has not been made
+        self.assertIsNone(self.grade_page.wait_for_page().score)
+
+        # Now do a staff assessment.
+        self.do_staff_assessment(options_selected=self.STAFF_OVERRIDE_OPTIONS_SELECTED)
+
+        # As an add-on, let's make sure that both submissions (the learner's, and the additional one created
+        # in do_train_self_peer() above) were assessed using staff-grading's "submit and keep going"
+        self.assertEqual(0, self.staff_area_page.available_checked_out_numbers[0])
+
+        # At this point, the learner sees the score (1).
+        self.refresh_page()
+        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
+        self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
+
+        if peer_grades_me:
+            self.verify_grade_entries([
+                [(u"STAFF GRADE - 0 POINTS", u"Poor"), (u"STAFF GRADE - 1 POINT", u"Fair")],
+                [(u"PEER MEDIAN GRADE", u"Poor"), (u"PEER MEDIAN GRADE", u"Poor")],
+                [(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")],
+            ])
+        else:
+            self.verify_grade_entries([
+                [(u"STAFF GRADE - 0 POINTS", u"Poor"), (u"STAFF GRADE - 1 POINT", u"Fair")],
+                [(u'PEER MEDIAN GRADE', u'Waiting for peer reviews'),
+                 (u'PEER MEDIAN GRADE', u'Waiting for peer reviews')],
+                [(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")],
+            ])
+
     def verify_staff_area_fields(self, username, peer_assessments, submitted_assessments, self_assessment):
         """
         Verifies the expected entries in the staff area for peer assessments,
@@ -910,6 +945,33 @@ class FullWorkflowMixin(object):
             self.assertEqual(expected_entry[1], self.grade_page.grade_entry(1, index))
 
 
+class MultipleOpenAssessmentMixin(FullWorkflowMixin):
+    """
+    A Multiple ORA assessment mixin with helper methods and constants for testing a full workflow
+    (training, self assessment, peer assessment, staff override).
+    """
+
+    def setup_vertical_index(self, vertical_index):
+        """
+        Set the vertical index on the page.
+        Each problem has vertical index assigned and has a `vert-{vertical_index}` top level class.
+        Set up vertical index on the page so as to move to a different problem.
+        """
+        self.submission_page.vertical_index = vertical_index
+        self.self_asmnt_page.vertical_index = vertical_index
+        self.peer_asmnt_page.vertical_index = vertical_index
+        self.student_training_page.vertical_index = vertical_index
+        self.staff_asmnt_page.vertical_index = vertical_index
+        self.grade_page.vertical_index = vertical_index
+        self.staff_area_page.vertical_index = vertical_index
+
+    def assess_component(self, vertical_index, peer_grades_me=True):
+        """ Assess the complete flow of an open assessment."""
+        self.setup_vertical_index(vertical_index)
+        self.do_train_self_peer(peer_grades_me)
+        self.staff_assessment(peer_grades_me)
+
+
 class FullWorkflowOverrideTest(OpenAssessmentTest, FullWorkflowMixin):
     """
     Tests of complete workflows, combining multiple required steps together.
@@ -1082,34 +1144,8 @@ class FullWorkflowRequiredTest(OpenAssessmentTest, FullWorkflowMixin):
         # Using ddt booleans to confirm behavior independent of whether I receive a peer score or not
         self.do_train_self_peer(peer_grades_me)
 
-        # Ensure grade is not present, since staff assessment has not been made
-        self.assertIsNone(self.grade_page.wait_for_page().score)
-
-        # Now do a staff assessment.
-        self.do_staff_assessment(options_selected=self.STAFF_OVERRIDE_OPTIONS_SELECTED)
-
-        # As an add-on, let's make sure that both submissions (the learner's, and the additional one created
-        # in do_train_self_peer() above) were assessed using staff-grading's "submit and keep going"
-        self.assertEqual(0, self.staff_area_page.available_checked_out_numbers[0])
-
-        # At this point, the learner sees the score (1).
-        self.refresh_page()
-        self._verify_staff_grade_section(self.STAFF_GRADE_EXISTS, None)
-        self.assertEqual(self.STAFF_OVERRIDE_SCORE, self.grade_page.wait_for_page().score)
-
-        if peer_grades_me:
-            self.verify_grade_entries([
-                [(u"STAFF GRADE - 0 POINTS", u"Poor"), (u"STAFF GRADE - 1 POINT", u"Fair")],
-                [(u"PEER MEDIAN GRADE", u"Poor"), (u"PEER MEDIAN GRADE", u"Poor")],
-                [(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")],
-            ])
-        else:
-            self.verify_grade_entries([
-                [(u"STAFF GRADE - 0 POINTS", u"Poor"), (u"STAFF GRADE - 1 POINT", u"Fair")],
-                [(u'PEER MEDIAN GRADE', u'Waiting for peer reviews'),
-                 (u'PEER MEDIAN GRADE', u'Waiting for peer reviews')],
-                [(u"YOUR SELF ASSESSMENT", u"Good"), (u"YOUR SELF ASSESSMENT", u"Excellent")],
-            ])
+        # Do staff assessment step
+        self.staff_assessment(peer_grades_me)
 
 @ddt.ddt
 class FeedbackOnlyTest(OpenAssessmentTest, FullWorkflowMixin):
@@ -1201,6 +1237,34 @@ class FeedbackOnlyTest(OpenAssessmentTest, FullWorkflowMixin):
         self.staff_area_page.verify_learner_final_score("Final grade: 1 out of 1")
 
 
+class MultipleOpenAssessmentTest(OpenAssessmentTest, MultipleOpenAssessmentMixin):
+    """
+    Test the multiple peer-assessment flow.
+    """
+
+    def setUp(self):
+        super(MultipleOpenAssessmentTest, self).setUp('multiple_ora')
+        # Staff area page is not present in OpenAssessmentTest base class, so we are adding it here.
+        self.staff_area_page = StaffAreaPage(self.browser, self.problem_loc)
+
+    @retry()
+    @attr('acceptance')
+    def test_multiple_ora_complete_flow(self):
+        """
+        Scenario: complete workflow on a unit containing multiple ORA blocks.
+        """
+        # Each problem has vertical index assigned and has a `vert-{vertical_index}` top level class.
+        # That also means that all pages are being differentiated by their vertical index number that is assigned to
+        # each problem type. We are passing vertical index number and setting it by `self.setup_vertical_index` method
+        # so as to move to a different problem.
+
+        # Assess first ORA problem, pass the vertical index number
+        self.assess_component(0)
+
+        # Assess second ORA problem, pass the vertical index number
+        self.assess_component(1)
+
+
 if __name__ == "__main__":
 
     # Configure the screenshot directory