From 7323c473cc3b0334a384766cb4361284eaaa1dff Mon Sep 17 00:00:00 2001 From: John Okely Date: Tue, 1 Aug 2017 15:57:55 +0800 Subject: [PATCH] MDL-56646 assign: Add ability to fix errant grades --- mod/assign/db/upgrade.php | 13 +++ mod/assign/lang/en/assign.php | 3 + mod/assign/locallib.php | 92 +++++++++++++++++++ mod/assign/tests/locallib_test.php | 143 +++++++++++++++++++++++++++++ mod/assign/upgradelib.php | 24 +++++ mod/assign/version.php | 2 +- 6 files changed, 276 insertions(+), 1 deletion(-) diff --git a/mod/assign/db/upgrade.php b/mod/assign/db/upgrade.php index f96c36922aa..4a564adbbe5 100644 --- a/mod/assign/db/upgrade.php +++ b/mod/assign/db/upgrade.php @@ -155,5 +155,18 @@ function xmldb_assign_upgrade($oldversion) { upgrade_mod_savepoint(true, 2017061200, 'assign'); } + if ($oldversion < 2017061205) { + require_once($CFG->dirroot.'/mod/assign/upgradelib.php'); + $brokenassigns = get_assignments_with_rescaled_null_grades(); + + // Set config value. + foreach ($brokenassigns as $assign) { + set_config('has_rescaled_null_grades_' . $assign, 1, 'assign'); + } + + // Main savepoint reached. + upgrade_mod_savepoint(true, 2017061205, 'assign'); + } + return true; } diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index 921c54c81fc..87cf06f7d06 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -196,6 +196,9 @@ $string['expandreviewpanel'] = 'Expand review panel'; $string['extensionduedate'] = 'Extension due date'; $string['extensionnotafterduedate'] = 'Extension date must be after the due date'; $string['extensionnotafterfromdate'] = 'Extension date must be after the allow submissions from date'; +$string['fixrescalednullgrades'] = 'This assignment contains some erroneous grades. You can automatically repair these grades. This may affect course totals.'; +$string['fixrescalednullgradesconfirm'] = 'Are you sure you want to fix erroneous grades? All broken grades will be removed. This may affect course totals.'; +$string['fixrescalednullgradesdone'] = 'Grades fixed.'; $string['gradecanbechanged'] = 'Grade can be changed'; $string['gradersubmissionupdatedtext'] = '{$a->username} has updated their assignment submission for \'{$a->assignment}\' at {$a->timeupdated} diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index a09ee9949a7..e8ca9477cde 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -603,6 +603,8 @@ class assign { $o .= $this->view_batch_markingallocation($mform); } else if ($action == 'viewsubmitforgradingerror') { $o .= $this->view_error_page(get_string('submitforgrading', 'assign'), $notices); + } else if ($action == 'fixrescalednullgrades') { + $o .= $this->view_fix_rescaled_null_grades(); } else { $o .= $this->view_submission_page(); } @@ -4237,6 +4239,8 @@ class assign { $this->require_view_grades(); require_once($CFG->dirroot . '/mod/assign/gradeform.php'); + $this->add_grade_notices(); + // Only load this if it is. $o .= $this->view_grading_table(); @@ -5210,6 +5214,8 @@ class assign { $instance = $this->get_instance(); + $this->add_grade_notices(); + $o = ''; $postfix = ''; @@ -8543,6 +8549,92 @@ class assign { $completion = new completion_info($this->get_course()); $completion->set_module_viewed($this->get_course_module()); } + + /** + * Checks for any grade notices, and adds notifications. Will display on assignment main page and grading table. + * + * @return void The notifications API will render the notifications at the appropriate part of the page. + */ + protected function add_grade_notices() { + if (has_capability('mod/assign:grade', $this->get_context()) && get_config('assign', 'has_rescaled_null_grades_' . $this->get_instance()->id)) { + $link = new \moodle_url('/mod/assign/view.php', array('id' => $this->get_course_module()->id, 'action' => 'fixrescalednullgrades')); + \core\notification::warning(get_string('fixrescalednullgrades', 'mod_assign', ['link' => $link->out()])); + } + } + + /** + * View fix rescaled null grades. + * + * @return bool True if null all grades are now fixed. + */ + protected function fix_null_grades() { + global $DB; + $result = $DB->set_field_select( + 'assign_grades', + 'grade', + ASSIGN_GRADE_NOT_SET, + 'grade <> ? AND grade < 0', + [ASSIGN_GRADE_NOT_SET] + ); + $assign = clone $this->get_instance(); + $assign->cmidnumber = $this->get_course_module()->idnumber; + assign_update_grades($assign); + return $result; + } + + /** + * View fix rescaled null grades. + * + * @return void The notifications API will render the notifications at the appropriate part of the page. + */ + protected function view_fix_rescaled_null_grades() { + global $OUTPUT; + + $o = ''; + + require_capability('mod/assign:grade', $this->get_context()); + + $instance = $this->get_instance(); + + $o .= $this->get_renderer()->render( + new assign_header( + $instance, + $this->get_context(), + $this->show_intro(), + $this->get_course_module()->id + ) + ); + + $confirm = optional_param('confirm', 0, PARAM_BOOL); + + if ($confirm) { + confirm_sesskey(); + + // Fix the grades. + $this->fix_null_grades(); + set_config('has_rescaled_null_grades_' . $instance->id, 0, 'assign'); + + // Display the notice. + $o .= $this->get_renderer()->notification(get_string('fixrescalednullgradesdone', 'assign'), 'notifysuccess'); + $url = new moodle_url( + '/mod/assign/view.php', + array( + 'id' => $this->get_course_module()->id, + 'action' => 'grading' + ) + ); + $o .= $this->get_renderer()->continue_button($url); + } else { + // Ask for confirmation. + $continue = new \moodle_url('/mod/assign/view.php', array('id' => $this->get_course_module()->id, 'action' => 'fixrescalednullgrades', 'confirm' => true, 'sesskey' => sesskey())); + $cancel = new \moodle_url('/mod/assign/view.php', array('id' => $this->get_course_module()->id)); + $o .= $OUTPUT->confirm(get_string('fixrescalednullgradesconfirm', 'mod_assign'), $continue, $cancel); + } + + $o .= $this->view_footer(); + + return $o; + } } /** diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index 193f7fa0b65..43391b640b8 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -2953,4 +2953,147 @@ Anchor link 2:Link text $completiondata = $completion->get_data($cm, false, $student2->id); $this->assertEquals(1, $completiondata->completionstate); } + + public function get_assignments_with_rescaled_null_grades_provider() { + return [ + 'Negative less than one is errant' => [ + 'grade' => -0.64, + 'count' => 1, + ], + 'Negative more than one is errant' => [ + 'grade' => -30.18, + 'count' => 1, + ], + 'Negative one exactly is not errant' => [ + 'grade' => ASSIGN_GRADE_NOT_SET, + 'count' => 0, + ], + 'Positive grade is not errant' => [ + 'grade' => 1, + 'count' => 0, + ], + 'Large grade is not errant' => [ + 'grade' => 100, + 'count' => 0, + ], + 'Zero grade is not errant' => [ + 'grade' => 0, + 'count' => 0, + ], + ]; + } + + /** + * Test determining if the assignment as any null grades that were rescaled. + * @dataProvider get_assignments_with_rescaled_null_grades_provider + */ + public function test_get_assignments_with_rescaled_null_grades($grade, $count) { + global $DB; + + $this->resetAfterTest(); + + $assign = $this->create_instance(array('grade' => 100)); + + // Try getting a student's grade. This will give a grade of -1. + // Then we can override it with a bad negative grade. + $assign->get_user_grade($this->students[0]->id, true); + + // Set the grade to something errant. + $DB->set_field( + 'assign_grades', + 'grade', + $grade, + [ + 'userid' => $this->students[0]->id, + 'assignment' => $assign->get_instance()->id, + ] + ); + + $this->assertCount($count, get_assignments_with_rescaled_null_grades()); + } + + /** + * Data provider for test_fix_null_grades + * @return array[] Test data for test_fix_null_grades. Each element should contain grade, expectedcount and gradebookvalue + */ + public function fix_null_grades_provider() { + return [ + 'Negative less than one is errant' => [ + 'grade' => -0.64, + 'gradebookvalue' => null, + ], + 'Negative more than one is errant' => [ + 'grade' => -30.18, + 'gradebookvalue' => null, + ], + 'Negative one exactly is not errant, but shouldn\'t be pushed to gradebook' => [ + 'grade' => ASSIGN_GRADE_NOT_SET, + 'gradebookvalue' => null, + ], + 'Positive grade is not errant' => [ + 'grade' => 1, + 'gradebookvalue' => 1, + ], + 'Large grade is not errant' => [ + 'grade' => 100, + 'gradebookvalue' => 100, + ], + 'Zero grade is not errant' => [ + 'grade' => 0, + 'gradebookvalue' => 0, + ], + ]; + } + + /** + * Test fix_null_grades + * @param number $grade The grade we should set in the assign grading table. + * @param number $expectedcount The finalgrade we expect in the gradebook after fixing the grades. + * @dataProvider fix_null_grades_provider + */ + public function test_fix_null_grades($grade, $gradebookvalue) { + global $DB; + + $this->resetAfterTest(); + + $studentid = $this->students[0]->id; + + $assign = $this->create_instance(); + + // Try getting a student's grade. This will give a grade of -1. + // Then we can override it with a bad negative grade. + $assign->get_user_grade($studentid, true); + + // Set the grade to something errant. + $DB->set_field( + 'assign_grades', + 'grade', + $grade, + [ + 'userid' => $studentid, + 'assignment' => $assign->get_instance()->id, + ] + ); + $assign->grade = $grade; + $assigntemp = clone $assign->get_instance(); + $assigntemp->cmidnumber = $assign->get_course_module()->idnumber; + assign_update_grades($assigntemp); + + // Check that the gradebook was updated with the assign grade. So we can guarentee test results later on. + $expectedgrade = $grade == -1 ? null : $grade; // Assign sends null to the gradebook for -1 grades. + $gradegrade = grade_grade::fetch(array('userid' => $studentid, 'itemid' => $assign->get_grade_item()->id)); + $this->assertEquals($expectedgrade, $gradegrade->rawgrade); + + // Call fix_null_grades(). + $method = new ReflectionMethod(assign::class, 'fix_null_grades'); + $method->setAccessible(true); + $result = $method->invoke($assign); + + $this->assertSame(true, $result); + + $gradegrade = grade_grade::fetch(array('userid' => $studentid, 'itemid' => $assign->get_grade_item()->id)); + + // Check that the grade was updated in the gradebook by fix_null_grades. + $this->assertEquals($gradebookvalue, $gradegrade->finalgrade); + } } diff --git a/mod/assign/upgradelib.php b/mod/assign/upgradelib.php index c4fff773043..10ab600920a 100644 --- a/mod/assign/upgradelib.php +++ b/mod/assign/upgradelib.php @@ -414,3 +414,27 @@ class assign_upgrade_manager { return $newcm; } } + +/** + * Determines if the assignment as any null grades that were rescaled. + * + * Null grades are stored as -1 but should never be rescaled. + * + * @return int[] Array of the ids of all the assignments with rescaled null grades. + */ +function get_assignments_with_rescaled_null_grades() { + global $DB; + + $query = 'SELECT id, assignment FROM {assign_grades} + WHERE grade < 0 AND grade <> -1'; + + $assignments = array_values($DB->get_records_sql($query)); + + $getassignmentid = function ($assignment) { + return $assignment->assignment; + }; + + $assignments = array_map($getassignmentid, $assignments); + + return $assignments; +} diff --git a/mod/assign/version.php b/mod/assign/version.php index 2c7e0e0c45a..9bcfe69dde2 100644 --- a/mod/assign/version.php +++ b/mod/assign/version.php @@ -25,6 +25,6 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'mod_assign'; // Full name of the plugin (used for diagnostics). -$plugin->version = 2017061200; // The current module version (Date: YYYYMMDDXX). +$plugin->version = 2017061205; // The current module version (Date: YYYYMMDDXX). $plugin->requires = 2017050500; // Requires this Moodle version. $plugin->cron = 60;