feat: use available peer reviews even if flexible grading is used (#2195

)

* test: modify test to made fix more apparent

* feat: use available peer reviews even if flexible grading is used

openassessment/__init__.py

@@ -2,4 +2,4 @@
 Initialization Information for Open Assessment Module
 """
 
-__version__ = '6.5.0'
+__version__ = '6.5.1'

openassessment/assessment/api/peer.py

@@ -37,6 +37,20 @@ def flexible_peer_grading_enabled(peer_requirements, course_settings):
     return peer_requirements.get("enable_flexible_grading")
 
 
+def flexible_peer_grading_active(submission_uuid, peer_requirements, course_settings):
+    """
+    Is flexible peer grading on, and has enough time elapsed since submission to enable it?
+    """
+    if not flexible_peer_grading_enabled(peer_requirements, course_settings):
+        return False
+
+    submission = sub_api.get_submission(submission_uuid)
+    # find how many days elapsed since subimitted
+    days_elapsed = (timezone.now().date() - submission['submitted_at'].date()).days
+    # check if flexible grading applies. if it does, then update must_grade
+    return days_elapsed >= FLEXIBLE_PEER_GRADING_REQUIRED_SUBMISSION_AGE_IN_DAYS
+
+
 def required_peer_grades(submission_uuid, peer_requirements, course_settings):
     """
     Given a submission id, finds how many peer assessment required.
@@ -51,20 +65,11 @@ def required_peer_grades(submission_uuid, peer_requirements, course_settings):
         int
     """
 
-    submission = sub_api.get_submission(submission_uuid)
-
     must_grade = peer_requirements["must_be_graded_by"]
-
-    if flexible_peer_grading_enabled(peer_requirements, course_settings):
-
-        # find how many days elapsed since subimitted
-        days_elapsed = (timezone.now().date() - submission['submitted_at'].date()).days
-
-        # check if flexible grading applies. if it does, then update must_grade
-        if days_elapsed >= FLEXIBLE_PEER_GRADING_REQUIRED_SUBMISSION_AGE_IN_DAYS:
-            must_grade = int(must_grade * FLEXIBLE_PEER_GRADING_GRADED_BY_PERCENTAGE / 100)
-            if must_grade == 0:
-                must_grade = 1
+    if flexible_peer_grading_active(submission_uuid, peer_requirements, course_settings):
+        must_grade = int(must_grade * FLEXIBLE_PEER_GRADING_GRADED_BY_PERCENTAGE / 100)
+        if must_grade == 0:
+            must_grade = 1
 
     return must_grade
 
@@ -243,10 +248,22 @@ def get_score(submission_uuid, peer_requirements, course_settings):
     ).order_by('-assessment')
 
     # Check if enough peers have graded this submission
+    # This value will be the number configured on the peer step, or the reduced number if flexible
+    # peer grading is active
     num_required_peer_grades = required_peer_grades(submission_uuid, peer_requirements, course_settings)
-    if items.count() < num_required_peer_grades:
+    num_recieved_peer_grades = items.count()
+    if num_recieved_peer_grades < num_required_peer_grades:
         return None
 
+    # If we are in a scenario where flexible grading is active, but we have more peer grades than
+    # flexible would reduces us to need, use as many grades as we can to generate the grade
+    # (up to the defined requirement on the peer step)
+    if flexible_peer_grading_active(submission_uuid, peer_requirements, course_settings):
+        num_required_peer_grades = min(
+            num_recieved_peer_grades,
+            peer_requirements['must_be_graded_by']
+        )
+
     # Unfortunately, we cannot use update() after taking a slice,
     # so we need to update the and save the items individually.
     # One might be tempted to first query for the first n assessments,
@@ -256,9 +273,12 @@ def get_score(submission_uuid, peer_requirements, course_settings):
     # Although this approach generates more database queries, the number is likely to
     # be relatively small (at least 1 and very likely less than 5).
     for scored_item in items[:num_required_peer_grades]:
-        if not scored_item.scored:
-            scored_item.scored = True
-            scored_item.save()
+        # If we've already gone through and marked items as scored, that should
+        # not change; if we've found a scored item we've done it already and should stop
+        if scored_item.scored:
+            break
+        scored_item.scored = True
+        scored_item.save()
     assessments = [item.assessment for item in items]
 
     return {

openassessment/assessment/test/test_peer.py

@@ -2179,19 +2179,20 @@ def test_flexible_peer_grading_waiting_on_submitter(self):
         Test for behavior when rather than waiting on peers to review a learner, the submitter
         themselves is the one who needs to complete grading
         """
-        requirements = {'must_grade': 3, 'must_be_graded_by': 2, 'enable_flexible_grading': True}
+        requirements = {'must_grade': 5, 'must_be_graded_by': 4, 'enable_flexible_grading': True}
         t0 = datetime.datetime(2024, 3, 13, tzinfo=pytz.UTC)
 
-        # Alice, Bob, Carl, and Dave submit on t0
+        # Alice, and five others submit on t0
         alice_sub, alice = self._create_student_and_submission('Alice', 'Alice submission', date=t0)
-        bob_sub, bob = self._create_student_and_submission('Bob', 'Bob submission', date=t0)
-        carl_sub, carl = self._create_student_and_submission('Carl', 'Carl submission', date=t0)
-        dave_sub, dave = self._create_student_and_submission('Dave', 'Dave submission', date=t0)
+        other_students = [
+            self._create_student_and_submission(f'student {i}', f'student {i} submission', date=t0)
+            for i in range(5)
+        ]
 
-        # Bob, Carl, and Dave all dutifully complete their required peer assessment on t1
+        # The other students all dutifully complete their required peer assessment on t1
         with freeze_time(t0 + datetime.timedelta(days=1)):
-            for learner, submission in [(bob, bob_sub), (carl, carl_sub), (dave, dave_sub)]:
-                for _ in range(3):
+            for submission, learner in other_students:
+                for _ in range(5):
                     peer_api.get_submission_to_assess(submission['uuid'], learner['student_id'])
                     peer_api.create_assessment(
                         submission['uuid'],
@@ -2203,17 +2204,17 @@ def test_flexible_peer_grading_waiting_on_submitter(self):
                         requirements['must_be_graded_by']
                     )
 
-            # Bob, Carl, and Dave all have scores but Alice does not because she
+            # other_students all have scores but Alice does not because she
             # has not completed her peer assessments
             self.assertIsNone(peer_api.get_score(alice_sub['uuid'], requirements, COURSE_SETTINGS))
-            for sub in [bob_sub, carl_sub, dave_sub]:
-                self.assertIsNotNone(peer_api.get_score(sub['uuid'], requirements, COURSE_SETTINGS))
+            for submission, _ in other_students:
+                self.assertIsNotNone(peer_api.get_score(submission['uuid'], requirements, COURSE_SETTINGS))
                 # The scores come from must_be_graded_by number of scores
-                self._assert_num_scored_items(sub, requirements['must_be_graded_by'])
+                self._assert_num_scored_items(submission, requirements['must_be_graded_by'])
 
         # Alice doesn't complete her required grades until t8, which then gives her a score
         with freeze_time(t0 + datetime.timedelta(days=8)):
-            for _ in range(3):
+            for _ in range(5):
                 peer_api.get_submission_to_assess(alice_sub['uuid'], alice['student_id'])
                 peer_api.create_assessment(
                     alice_sub['uuid'],
@@ -2225,8 +2226,9 @@ def test_flexible_peer_grading_waiting_on_submitter(self):
                     requirements['must_be_graded_by']
                 )
             self.assertIsNotNone(peer_api.get_score(alice_sub['uuid'], requirements, COURSE_SETTINGS))
-        # But it's only using the first score because flexible peer grading had activated
-        self._assert_num_scored_items(alice_sub, 1)
+            # Even though flexible peer grading is activated, we use all available grades
+            self.assertTrue(peer_api.flexible_peer_grading_active(alice_sub['uuid'], requirements, COURSE_SETTINGS))
+            self._assert_num_scored_items(alice_sub, 4)
 
 
 class PeerWorkflowTest(CacheResetTest):

package.json

@@ -1,6 +1,6 @@
 {
   "name": "edx-ora2",
-  "version": "6.5.0",
+  "version": "6.5.1",
   "repository": "https://github.com/openedx/edx-ora2.git",
   "dependencies": {
     "@edx/frontend-build": "8.0.6",