diff --git a/mod/assign/backup/moodle2/restore_assign_stepslib.php b/mod/assign/backup/moodle2/restore_assign_stepslib.php
index 16b37020d3f..bc8ab136c48 100644
--- a/mod/assign/backup/moodle2/restore_assign_stepslib.php
+++ b/mod/assign/backup/moodle2/restore_assign_stepslib.php
@@ -240,7 +240,10 @@ class restore_assign_activity_structure_step extends restore_activity_structure_
$flags->userid = $this->get_mappingid('user', $data->userid);
$DB->insert_record('assign_user_flags', $flags);
}
-
+ // Fix null grades that were rescaled.
+ if ($data->grade < 0 && $data->grade != -1) {
+ $data->grade = -1;
+ }
$newitemid = $DB->insert_record('assign_grades', $data);
// Note - the old contextid is required in order to be able to restore files stored in
diff --git a/mod/assign/db/upgrade.php b/mod/assign/db/upgrade.php
index 035aa88468a..b1bcd2f41e5 100644
--- a/mod/assign/db/upgrade.php
+++ b/mod/assign/db/upgrade.php
@@ -233,5 +233,18 @@ function xmldb_assign_upgrade($oldversion) {
// Automatically generated Moodle v3.2.0 release upgrade line.
// Put any upgrade step following this.
+ if ($oldversion < 2016120501) {
+ 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, 2016120501, 'assign');
+ }
+
return true;
}
diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php
index dabe025ccd6..94801299752 100644
--- a/mod/assign/lang/en/assign.php
+++ b/mod/assign/lang/en/assign.php
@@ -195,6 +195,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/lib.php b/mod/assign/lib.php
index 9a11771d7b6..199da91c0ee 100644
--- a/mod/assign/lib.php
+++ b/mod/assign/lib.php
@@ -1511,7 +1511,8 @@ function assign_rescale_activity_grades($course, $cm, $oldmin, $oldmax, $newmin,
'a' => $cm->instance
);
- $sql = 'UPDATE {assign_grades} set grade = (((grade - :p1) * :p2) + :p3) where assignment = :a';
+ // Only rescale grades that are greater than or equal to 0. Anything else is a special value.
+ $sql = 'UPDATE {assign_grades} set grade = (((grade - :p1) * :p2) + :p3) where assignment = :a and grade >= 0';
$dbupdate = $DB->execute($sql, $params);
if (!$dbupdate) {
return false;
diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index 96f91305017..82bf85b31d2 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -593,6 +593,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();
}
@@ -4176,6 +4178,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();
@@ -5130,6 +5134,8 @@ class assign {
$instance = $this->get_instance();
+ $this->add_grade_notices();
+
$o = '';
$postfix = '';
@@ -8465,6 +8471,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',
+ -1,
+ 'grade <> ? AND grade < 0',
+ [-1]
+ );
+ $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();
+ unset_config('has_rescaled_null_grades_' . $instance->id, '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/behat/rescale_grades.feature b/mod/assign/tests/behat/rescale_grades.feature
index 47f57276076..ab0c35f2af2 100644
--- a/mod/assign/tests/behat/rescale_grades.feature
+++ b/mod/assign/tests/behat/rescale_grades.feature
@@ -12,10 +12,12 @@ Feature: Check that the assignment grade can be rescaled when the max grade is c
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| student1 | Student | 1 | student10@example.com |
+ | student2 | Student | 2 | student10@example.com |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| student1 | C1 | student |
+ | student2 | C1 | student |
And the following "groups" exist:
| name | course | idnumber |
| Group 1 | C1 | G1 |
@@ -70,3 +72,18 @@ Feature: Check that the assignment grade can be rescaled when the max grade is c
When I press "Save and display"
And I navigate to "View all submissions" in current page administration
Then "Student 1" row "Grade" column of "generaltable" table should contain "20.00"
+
+ Scenario: Rescaling should not produce negative grades
+ Given I navigate to "View all submissions" in current page administration
+ And I click on "Grade" "link" in the "Student 2" "table_row"
+ And I wait until the page is ready
+ And I follow "Assignment: Test assignment name"
+ And I navigate to "Edit settings" in current page administration
+ And I expand all fieldsets
+ And I set the field "Rescale existing grades" to "Yes"
+ And I set the field "Maximum grade" to "50"
+ When I press "Save and display"
+ And I navigate to "View all submissions" in current page administration
+ # Since the decimal places are always displayed (e.g. 0.00), this will catch any grade. The student should not have one.
+ # We can't just check for - (empty grade) because a negative grade would also have that character.
+ Then "Student 2" row "Grade" column of "generaltable" table should not contain "."
diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php
index 30fffe02786..cd64282c2e6 100644
--- a/mod/assign/tests/lib_test.php
+++ b/mod/assign/tests/lib_test.php
@@ -394,4 +394,32 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase {
$this->assertFalse(assign_refresh_events('aaa'));
}
+ /**
+ * Test that if some grades are not set, they are left alone and not rescaled
+ */
+ public function test_assign_rescale_activity_grades_some_unset() {
+ $this->resetAfterTest();
+
+ // As a teacher...
+ $this->setUser($this->editingteachers[0]);
+ $assign = $this->create_instance();
+
+ // Grade the student.
+ $data = ['grade' => 50];
+ $assign->testable_apply_grade_to_user((object)$data, $this->students[0]->id, 0);
+
+ // Try getting another students grade. This will give a grade of -1.
+ $assign->get_user_grade($this->students[1]->id, true);
+
+ // Rescale.
+ assign_rescale_activity_grades($this->course, $assign->get_course_module(), 0, 100, 0, 10);
+
+ // Get the grades for both students.
+ $student0grade = $assign->get_user_grade($this->students[0]->id, true);
+ $student1grade = $assign->get_user_grade($this->students[1]->id, true);
+
+ // Make sure the real grade is scaled, but the -1 stays the same.
+ $this->assertEquals($student0grade->grade, 5);
+ $this->assertEquals($student1grade->grade, -1);
+ }
}
diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
index 906614ec4d2..cada0c3aa5c 100644
--- a/mod/assign/tests/locallib_test.php
+++ b/mod/assign/tests/locallib_test.php
@@ -2937,4 +2937,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' => -1,
+ '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' => -1,
+ '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 e1e63b9dadd..c1ed45e6a9e 100644
--- a/mod/assign/upgradelib.php
+++ b/mod/assign/upgradelib.php
@@ -413,3 +413,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 6bdbfc7c8f2..45959eaee5f 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 = 2016120500; // The current module version (Date: YYYYMMDDXX).
+$plugin->version = 2016120501; // The current module version (Date: YYYYMMDDXX).
$plugin->requires = 2016112900; // Requires this Moodle version.
$plugin->cron = 60;