Merge pull request #118 from edx-solutions/sstudent/grading

implements scorablexblockmixin and moves drag and drop v2 to use raw grades.

.travis.yml

@@ -12,7 +12,7 @@ install:
     - "pip install selenium==2.53.0"
     - "pip uninstall -y xblock-drag-and-drop-v2"
     - "python setup.py sdist"
-    - "pip install dist/xblock-drag-and-drop-v2-2.0.15.tar.gz"
+    - "pip install dist/xblock-drag-and-drop-v2-2.0.16.tar.gz"
 script:
     - pep8 drag_and_drop_v2 tests --max-line-length=120
     - pylint drag_and_drop_v2

drag_and_drop_v2/drag_and_drop_v2.py

@@ -14,6 +14,7 @@ from xblock.core import XBlock
 from xblock.exceptions import JsonHandlerError
 from xblock.fields import Scope, String, Dict, Float, Boolean, Integer
 from xblock.fragment import Fragment
+from xblock.scorable import ScorableXBlockMixin, Score
 from xblockutils.resources import ResourceLoader
 from xblockutils.settings import XBlockWithSettingsMixin, ThemableXBlockMixin
 
@@ -31,7 +32,12 @@ logger = logging.getLogger(__name__)
 
 @XBlock.wants('settings')
 @XBlock.needs('i18n')
-class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
+class DragAndDropBlock(
+    XBlock,
+    XBlockWithSettingsMixin,
+    ThemableXBlockMixin,
+    ScorableXBlockMixin
+):
     """
     XBlock that implements a friendly Drag-and-Drop problem
     """
@@ -164,13 +170,18 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
     )
 
     grade = Float(
-        help=_("Keeps maximum score achieved by student"),
+        help=_("DEPRECATED. Keeps maximum score achieved by student as a weighted value."),
+        scope=Scope.user_state,
+        default=0
+    )
+
+    raw_earned = Float(
+        help=_("Keeps maximum score achieved by student as a raw value between 0 and 1."),
         scope=Scope.user_state,
         default=0
     )
 
     block_settings_key = 'drag-and-drop-v2'
-    has_score = True
 
     def max_score(self):  # pylint: disable=no-self-use
         """
@@ -179,6 +190,44 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         """
         return 1
 
+    def get_score(self):
+        """
+        Return the problem's current score as raw values.
+        """
+        if not self._get_raw_earned_if_set():
+            self.raw_earned = self._learner_raw_score()
+        return Score(self.raw_earned, self.max_score())
+
+    def set_score(self, score):
+        """
+        Sets the score on this block.
+        Takes a Score namedtuple containing a raw
+        score and possible max (for this block, we expect that this will
+        always be 1).
+        """
+        assert score.raw_possible == self.max_score()
+        self.raw_earned = score.raw_earned
+
+    def calculate_score(self):
+        """
+        Returns a newly-calculated raw score on the problem for the learner
+        based on the learner's current state.
+        """
+        return Score(self._learner_raw_score(), self.max_score())
+
+    def has_submitted_answer(self):
+        """
+        Returns True if the user has made a submission.
+        """
+        return self.fields['raw_earned'].is_set_on(self) or self.fields['grade'].is_set_on(self)
+
+    def weighted_grade(self):
+        """
+        Returns the block's current saved grade multiplied by the block's
+        weight- the number of points earned by the learner.
+        """
+        return self.raw_earned * self.weight
+
     def _learner_raw_score(self):
         """
         Calculate raw score for learner submission.
@@ -432,7 +481,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         return {
             'correct': correct,
             'attempts': self.attempts,
-            'grade': self._get_grade_if_set(),
+            'grade': self._get_raw_earned_if_set(),
             'misplaced_items': list(misplaced_ids),
             'feedback': self._present_feedback(feedback_msgs),
             'overall_feedback': self._present_feedback(overall_feedback_msgs)
@@ -598,7 +647,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
                 grade_feedback_template = FeedbackMessages.FINAL_ATTEMPT_TPL
 
             feedback_msgs.append(
-                FeedbackMessage(grade_feedback_template.format(score=self.grade), grade_feedback_class)
+                FeedbackMessage(grade_feedback_template.format(score=self.weighted_grade()), grade_feedback_class)
             )
 
         return feedback_msgs, misplaced_ids
@@ -633,7 +682,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
 
         return {
             'correct': is_correct,
-            'grade': self._get_grade_if_set(),
+            'grade': self._get_raw_earned_if_set(),
             'finished': self._is_answer_correct(),
             'overall_feedback': self._present_feedback(overall_feedback),
             'feedback': self._present_feedback([item_feedback])
@@ -680,38 +729,25 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         """
         # pylint: disable=fixme
         # TODO: (arguable) split this method into "clean" functions (with no side effects and implicit state)
-        # This method implicitly depends on self.item_state (via _is_answer_correct and _calculate_grade)
-        # and also updates self.grade if some conditions are met. As a result this method implies some order of
+        # This method implicitly depends on self.item_state (via _is_answer_correct and _learner_raw_score)
+        # and also updates self.raw_earned if some conditions are met. As a result this method implies some order of
         # invocation:
         # * it should be called after learner-caused updates to self.item_state is applied
         # * it should be called before self.item_state cleanup is applied (i.e. returning misplaced items to item bank)
-        # * it should be called before any method that depends on self.grade (i.e. self._get_feedback)
+        # * it should be called before any method that depends on self.raw_earned (i.e. self._get_feedback)
 
         # Splitting it into a "clean" functions will allow to capture this implicit invocation order in caller method
         # and help avoid bugs caused by invocation order violation in future.
 
         # There's no going back from "completed" status to "incomplete"
         self.completed = self.completed or self._is_answer_correct() or not self.attempts_remain
-        grade = self._calculate_grade()
+        current_raw_earned = self._learner_raw_score()
         # ... and from higher grade to lower
-        current_grade = self._get_grade_if_set()
-        if current_grade is None or grade > current_grade:
-            self.grade = grade
-            self._publish_grade()
-
-    def _publish_grade(self):
-        """
-        Publishes grade
-        """
-        try:
-            self.runtime.publish(self, 'grade', {
-                'value': self.grade,
-                'max_value': self.weight,
-            })
-        except NotImplementedError:
-            # Note, this publish method is unimplemented in Studio runtimes,
-            # so we have to figure that we're running in Studio for now
-            pass
+        # if we have an old-style (i.e. unreliable) grade, override no matter what
+        saved_raw_earned = self._get_raw_earned_if_set()
+        if current_raw_earned is None or current_raw_earned > saved_raw_earned:
+            self.raw_earned = current_raw_earned
+            self._publish_grade(Score(self.raw_earned, self.max_score()))
 
     def _publish_item_dropped_event(self, attempt, is_correct):
         """
@@ -778,7 +814,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
             'items': item_state,
             'finished': is_finished,
             'attempts': self.attempts,
-            'grade': self._get_grade_if_set(),
+            'grade': self._get_raw_earned_if_set(),
             'overall_feedback': self._present_feedback(overall_feedback_msgs)
         }
 
@@ -900,19 +936,13 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
 
         return ItemStats(required, placed, correctly_placed, decoy, decoy_in_bank)
 
-    def _calculate_grade(self):
-        """
-        Calculates the student's grade for this block based on current item state.
-        """
-        return self._learner_raw_score() * self.weight
-
-    def _get_grade_if_set(self):
+    def _get_raw_earned_if_set(self):
         """
         Returns student's grade if already explicitly set, otherwise returns None.
-        This is different from self.grade which returns 0 by default.
+        This is different from self.raw_earned which returns 0 by default.
         """
-        if self.fields['grade'].is_set_on(self):
-            return self.grade
+        if self.fields['raw_earned'].is_set_on(self):
+            return self.raw_earned
         else:
             return None
 

install_test_deps.sh

 # Installs xblock-sdk and dependencies needed to run the tests suite.
 # Run this script inside a fresh virtual environment.
-pip install -e git://github.com/edx/xblock-sdk.git@v0.1.2#egg=xblock-sdk==v0.1.2
+pip install -e git://github.com/edx/xblock-sdk.git@v0.1.3#egg=xblock-sdk==v0.1.3
 cd $VIRTUAL_ENV/src/xblock-sdk/ && pip install -r requirements/base.txt \
                                 && pip install -r requirements/test.txt && cd -
 pip install -r requirements.txt

requirements.txt

-git+https://github.com/edx/xblock-utils.git@v1.0.2#egg=xblock-utils==1.0.2
+git+https://github.com/edx/xblock-utils.git@v1.0.4#egg=xblock-utils==1.0.4
 -e .

setup.py

@@ -23,7 +23,7 @@ def package_data(pkg, root_list):
 
 setup(
     name='xblock-drag-and-drop-v2',
-    version='2.0.15',
+    version='2.0.16',
     description='XBlock - Drag-and-Drop v2',
     packages=['drag_and_drop_v2'],
     install_requires=[

tests/integration/test_events.py

@@ -41,7 +41,7 @@ class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, BaseEventsT
         },
         {
             'name': 'grade',
-            'data': {'max_value': 1, 'value': (2.0 / 5)},
+            'data': {'max_value': 1, 'value': (2.0 / 5), 'only_if_higher': None},
         },
         {
             'name': 'edx.drag_and_drop_v2.item.dropped',
@@ -77,7 +77,7 @@ class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, BaseEventsT
     @unpack
     def test_event(self, index, event):
         self.parameterized_item_positive_feedback_on_good_move_standard(self.items_map)
-        dummy, name, published_data = self.publish.call_args_list[index][0]
+        _, name, published_data = self.publish.call_args_list[index][0]
         self.assertEqual(name, event['name'])
         self.assertEqual(published_data, event['data'])
 
@@ -121,7 +121,7 @@ class AssessmentEventsFiredTest(
         },
         {
             'name': 'grade',
-            'data': {'max_value': 1, 'value': (1.0 / 5)},
+            'data': {'max_value': 1, 'value': (1.0 / 5), 'only_if_higher': None},
         },
         {
             'name': 'edx.drag_and_drop_v2.feedback.opened',
@@ -143,7 +143,7 @@ class AssessmentEventsFiredTest(
         self.click_submit()
         self.wait_for_ajax()
         for index, event in enumerate(self.scenarios):
-            dummy, name, published_data = self.publish.call_args_list[index][0]
+            _, name, published_data = self.publish.call_args_list[index][0]
             self.assertEqual(name, event['name'])
             self.assertEqual(published_data, event['data'])
 
@@ -158,7 +158,7 @@ class AssessmentEventsFiredTest(
 
         events = self.publish.call_args_list
         published_grade = next((event[0][2] for event in events if event[0][1] == 'grade'))
-        expected_grade = {'max_value': 1, 'value': (1.0 / 5.0)}
+        expected_grade = {'max_value': 1, 'value': (1.0 / 5.0), 'only_if_higher': None}
         self.assertEqual(published_grade, expected_grade)
 
 

tests/integration/test_render.py

@@ -223,6 +223,10 @@ class TestDragAndDropRender(BaseIntegrationTest):
         # usually is not the case when running integration tests.
         # See: https://github.com/seleniumhq/selenium-google-code-issue-archive/issues/7346
         self.browser.execute_script('$("button.go-to-beginning-button").focus()')
+
+        # For unknown reasons the element only becomes visible when focus() is called twice.
+        # See: https://openedx.atlassian.net/browse/TNL-6736
+        self.browser.execute_script('$("button.go-to-beginning-button").focus()')
         self.assertFocused(button)
         # Button should be visible when focused.
         self.assertNotIn('sr', button.get_attribute('class').split())