Skip to content

Commit

Permalink
MDL-60685 assign: Do not falsely use admin user
Browse files Browse the repository at this point in the history
When an assign_grades record is automatically populated, do not use the admin user as the default grader.

This would generate false information on the assignment summary screen and send false notifications from
the admin user.
  • Loading branch information
Damyon Wiese committed Aug 2, 2018
1 parent a1d8976 commit 6b74d76
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
16 changes: 11 additions & 5 deletions mod/assign/locallib.php
Expand Up @@ -2368,6 +2368,7 @@ public static function cron() {
// Submissions are included if all are true:
// - The assignment is visible in the gradebook.
// - No previous notification has been sent.
// - The grader was a real user, not an automated process.
// - If marking workflow is not enabled, the grade was updated in the past 24 hours, or
// if marking workflow is enabled, the workflow state is at 'released'.
$sql = "SELECT g.id as gradeid, a.course, a.name, a.blindmarking, a.revealidentities,
Expand All @@ -2381,7 +2382,7 @@ public static function cron() {
LEFT JOIN {assign_user_mapping} um ON g.id = um.userid AND um.assignment = a.id
WHERE ((a.markingworkflow = 0 AND g.timemodified >= :yesterday AND g.timemodified <= :today) OR
(a.markingworkflow = 1 AND uf.workflowstate = :wfreleased)) AND
uf.mailed = 0 AND gri.hidden = 0
g.grader > 0 AND uf.mailed = 0 AND gri.hidden = 0
ORDER BY a.course, cm.id";

$params = array(
Expand Down Expand Up @@ -3710,7 +3711,8 @@ public function get_user_grade($userid, $create, $attemptnumber=-1) {
$grade->timemodified = $grade->timecreated;
}
$grade->grade = -1;
$grade->grader = $USER->id;
// Do not set the grader id here as it would be the admin users which is incorrect.
$grade->grader = -1;
if ($attemptnumber >= 0) {
$grade->attemptnumber = $attemptnumber;
}
Expand Down Expand Up @@ -5096,8 +5098,10 @@ public function get_assign_feedback_status_renderable($user) {
$gradefordisplay = $this->display_grade($gradebookgrade->grade, false);
}
$gradeddate = $gradebookgrade->dategraded;
if (isset($grade->grader)) {
if (isset($grade->grader) && $grade->grader > 0) {
$grader = $DB->get_record('user', array('id' => $grade->grader));
} else if (isset($gradebookgrade->usermodified) && $gradebookgrade->usermodified > 0) {
$grader = $DB->get_record('user', array('id' => $gradebookgrade->usermodified));
}
}

Expand Down Expand Up @@ -5258,10 +5262,12 @@ protected function get_all_grades($userid) {
// First lookup the grader info.
if (isset($gradercache[$grade->grader])) {
$grade->grader = $gradercache[$grade->grader];
} else {
} else if ($grade->grader > 0) {
// Not in cache - need to load the grader record.
$grade->grader = $DB->get_record('user', array('id'=>$grade->grader));
$gradercache[$grade->grader->id] = $grade->grader;
if ($grade->grader) {
$gradercache[$grade->grader->id] = $grade->grader;
}
}

// Now get the gradefordisplay.
Expand Down
5 changes: 5 additions & 0 deletions mod/assign/tests/locallib_test.php
Expand Up @@ -3882,6 +3882,7 @@ public function test_fix_null_grades($grade, $gradebookvalue) {
$assign->get_user_grade($student->id, true);

// Set the grade to something errant.
// We don't set the grader here, so we expect it to be -1 as a result.
$DB->set_field(
'assign_grades',
'grade',
Expand All @@ -3899,6 +3900,7 @@ public function test_fix_null_grades($grade, $gradebookvalue) {
// 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' => $student->id, 'itemid' => $assign->get_grade_item()->id));
$this->assertEquals(-1, $gradegrade->usermodified);
$this->assertEquals($expectedgrade, $gradegrade->rawgrade);

// Call fix_null_grades().
Expand All @@ -3910,6 +3912,9 @@ public function test_fix_null_grades($grade, $gradebookvalue) {

$gradegrade = grade_grade::fetch(array('userid' => $student->id, 'itemid' => $assign->get_grade_item()->id));

$this->assertEquals(-1, $gradegrade->usermodified);
$this->assertEquals($gradebookvalue, $gradegrade->finalgrade);

// Check that the grade was updated in the gradebook by fix_null_grades.
$this->assertEquals($gradebookvalue, $gradegrade->finalgrade);
}
Expand Down

0 comments on commit 6b74d76

Please sign in to comment.