From 851b5c7f2a091a5a5357b10674aacd1db6b36209 Mon Sep 17 00:00:00 2001
From: Damyon Wiese <damyon@moodle.com>
Date: Mon, 23 Jul 2018 14:09:47 +0800
Subject: [PATCH] MDL-60685 assign: Do not falsely use admin user

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.
---
 mod/assign/locallib.php            | 16 +++++++++++-----
 mod/assign/tests/locallib_test.php |  5 +++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index 55f734cbbec..649a6ef78f1 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -2368,6 +2368,7 @@ class assign {
         // 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,
@@ -2381,7 +2382,7 @@ class assign {
             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(
@@ -3710,7 +3711,8 @@ class assign {
                 $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;
             }
@@ -5096,8 +5098,10 @@ class assign {
                     $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));
                 }
             }
 
@@ -5258,10 +5262,12 @@ class assign {
             // 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.
diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
index 80ec0cd2a04..895a2e013c6 100644
--- a/mod/assign/tests/locallib_test.php
+++ b/mod/assign/tests/locallib_test.php
@@ -3882,6 +3882,7 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $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',
@@ -3899,6 +3900,7 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         // 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().
@@ -3910,6 +3912,9 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
 
         $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);
     }