From f820b873b1f021c730f40be0a6457f246cfe1c41 Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Mon, 25 Mar 2024 10:22:38 -0400 Subject: [PATCH] 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 +- openassessment/assessment/api/peer.py | 54 ++++++++++++++------- openassessment/assessment/test/test_peer.py | 32 ++++++------ package-lock.json | 2 +- package.json | 2 +- 5 files changed, 57 insertions(+), 35 deletions(-) diff --git a/openassessment/__init__.py b/openassessment/__init__.py index ea3f805cdd..8b5b87d35b 100644 --- a/openassessment/__init__.py +++ b/openassessment/__init__.py @@ -2,4 +2,4 @@ Initialization Information for Open Assessment Module """ -__version__ = '6.5.0' +__version__ = '6.5.1' diff --git a/openassessment/assessment/api/peer.py b/openassessment/assessment/api/peer.py index a2947a985e..467c1b5a8c 100644 --- a/openassessment/assessment/api/peer.py +++ b/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 { diff --git a/openassessment/assessment/test/test_peer.py b/openassessment/assessment/test/test_peer.py index 585a20081a..99c6f70b93 100644 --- a/openassessment/assessment/test/test_peer.py +++ b/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): diff --git a/package-lock.json b/package-lock.json index 934890c552..5ab0df5104 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "edx-ora2", - "version": "6.4.0", + "version": "6.5.1", "lockfileVersion": 3, "requires": true, "packages": { diff --git a/package.json b/package.json index 7716ad437b..683716b385 100644 --- a/package.json +++ b/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",