MDL-82129 gradereport_history: add grade penalty source

catalyst · Aug 22, 2024 · 3148f11 · 3148f11
1 parent 70be1e3
commit 3148f11
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 9 deletions.
diff --git a/grade/classes/local/penalty/manager.php b/grade/classes/local/penalty/manager.php
@@ -114,7 +114,7 @@ public static function apply_penalty(int $userid, grade_item $gradeitem,
         // Apply the penalty to the grade.
         if (!$previewonly) {
             // Update the final grade after the penalty is applied.
-            $gradeitem->update_raw_grade($userid, $beforepenaltyhook->get_grade_after_penalty());
+            $gradeitem->update_raw_grade($userid, $beforepenaltyhook->get_grade_after_penalty(), 'gradepenalty');
 
             // Hook for plugins to process further after the penalty is applied to the grade.
             $afterpenaltyhook = new after_penalty_applied($userid, $gradeitem, $submissiondate, $duedate,

diff --git a/grade/report/history/classes/output/tablelog.php b/grade/report/history/classes/output/tablelog.php
@@ -468,7 +468,10 @@ protected function get_sql_and_params($count = false) {
                             FROM {grade_grades_history} h
                            WHERE h.itemid = ggh.itemid
                              AND h.userid = ggh.userid
-                             AND h.timemodified < ggh.timemodified
+                             AND (
+                                    h.timemodified < ggh.timemodified
+                                    OR (h.timemodified = ggh.timemodified AND h.source != ggh.source AND h.id < ggh.id)
+                                 )
                              AND NOT EXISTS (
                               SELECT 1
                                 FROM {grade_grades_history} h2

diff --git a/grade/report/history/tests/report_test.php b/grade/report/history/tests/report_test.php
@@ -16,6 +16,8 @@
 
 namespace gradereport_history;
 
+use context_course;
+
 defined('MOODLE_INTERNAL') || die();
 
 /**
@@ -36,8 +38,8 @@ public function test_query_db(): void {
         // Making the setup.
         $c1 = $this->getDataGenerator()->create_course();
         $c2 = $this->getDataGenerator()->create_course();
-        $c1ctx = \context_course::instance($c1->id);
-        $c2ctx = \context_course::instance($c2->id);
+        $c1ctx = context_course::instance($c1->id);
+        $c2ctx = context_course::instance($c2->id);
 
         // Users.
         $u1 = $this->getDataGenerator()->create_user();
@@ -179,7 +181,7 @@ public function test_query_db(): void {
         // Put course in separate groups mode, add grader1 and two students to the same group.
         $c1->groupmode = SEPARATEGROUPS;
         update_course($c1);
-        $this->assertFalse(has_capability('moodle/site:accessallgroups', \context_course::instance($c1->id)));
+        $this->assertFalse(has_capability('moodle/site:accessallgroups', context_course::instance($c1->id)));
         $g1 = self::getDataGenerator()->create_group(['courseid' => $c1->id, 'name' => 'g1']);
         self::getDataGenerator()->create_group_member(['groupid' => $g1->id, 'userid' => $grader1->id]);
         self::getDataGenerator()->create_group_member(['groupid' => $g1->id, 'userid' => $u1->id]);
@@ -200,8 +202,8 @@ public function test_get_users(): void {
         // Making the setup.
         $c1 = $this->getDataGenerator()->create_course();
         $c2 = $this->getDataGenerator()->create_course();
-        $c1ctx = \context_course::instance($c1->id);
-        $c2ctx = \context_course::instance($c2->id);
+        $c1ctx = context_course::instance($c1->id);
+        $c2ctx = context_course::instance($c2->id);
 
         $c1m1 = $this->getDataGenerator()->create_module('assign', array('course' => $c1));
         $c2m1 = $this->getDataGenerator()->create_module('assign', array('course' => $c2));
@@ -354,7 +356,7 @@ public function test_get_users_with_profile_fields(string $showuseridentity, str
 
         // Making the setup.
         $c1 = $this->getDataGenerator()->create_course();
-        $c1ctx = \context_course::instance($c1->id);
+        $c1ctx = context_course::instance($c1->id);
         $c1m1 = $this->getDataGenerator()->create_module('assign', array('course' => $c1));
 
         // Creating grade history for some users.
@@ -483,7 +485,7 @@ public function test_get_users_with_groups($groupmode, $teacherrole, $teachergro
         $this->setUser($t1);
 
         // Fetch the users.
-        $users = \gradereport_history\helper::get_users(\context_course::instance($course->id));
+        $users = \gradereport_history\helper::get_users(context_course::instance($course->id));
         // Confirm that the number of users fetched is the same as the count of expected users.
         $this->assertCount(count($expectedusers), $users);
         foreach ($users as $user) {
@@ -554,6 +556,64 @@ public function test_graders(): void {
         $this->assertCount(1, $graders); // Including "all graders" .
     }
 
+    /**
+     * Test grade history with grade updated by different sources
+     *
+     * @covers \gradereport_history\output\tablelog::get_sql_and_params
+     *
+     * @return void
+     */
+    public function test_grade_history_with_different_sources(): void {
+        $this->resetAfterTest();
+
+        // Create a course.
+        $course = $this->getDataGenerator()->create_course();
+
+        // Create a user and enrol them in the course.
+        $user = $teacher = $this->getDataGenerator()->create_and_enrol($course, 'student');
+
+        // Create an assignment.
+        $assign = $this->getDataGenerator()->create_module('assign', ['course' => $course]);
+
+        // Create a grade item.
+        $gi = \grade_item::fetch(['iteminstance' => $assign->id, 'itemtype' => 'mod', 'itemmodule' => 'assign']);
+
+        // Create some grade history entries with same time modifies.
+        $time = time();
+        $this->create_grade_history([
+            'itemid' => $gi->id,
+            'userid' => $user->id,
+            'usermodified' => $teacher->id,
+            'finalgrade' => 50,
+            'source' => 'mod/assign',
+            'timemodified' => $time,
+        ]);
+        $this->create_grade_history([
+            'itemid' => $gi->id,
+            'userid' => $user->id,
+            'usermodified' => $teacher->id,
+            'finalgrade' => 60,
+            'source' => 'cli',
+            'timemodified' => $time,
+        ]);
+
+        // Fetch the grade history.
+        $results = $this->get_tablelog_results(context_course::instance($course->id));
+
+        // Check the grade history.
+        $this->assertCount(2, $results);
+        $assigngrade = array_pop($results);
+        $cligrade = array_pop($results);
+
+        // Check their values.
+        $this->assertEquals(60, $cligrade->finalgrade);
+        $this->assertEquals(50, $cligrade->prevgrade);
+        $this->assertEquals('cli', $cligrade->source);
+        $this->assertEquals(50, $assigngrade->finalgrade);
+        $this->assertEquals(null, $assigngrade->prevgrade);
+        $this->assertEquals('mod/assign', $assigngrade->source);
+    }
+
     /**
      * Asserts that the array of grade objects contains exactly the right IDs.
      *