Ctp 3560 test fixes

classes/auto_grader/percentage_distance.php

@@ -129,12 +129,13 @@ private function get_allocatable() {
      *
      */
     private function create_final_feedback() {
-        feedback::create([
-                             'stage_identifier' => 'final_agreed_1',
-                             'submissionid' => $this->get_allocatable()->get_submission($this->get_coursework())->id(),
-                             'grade' => $this->automatic_grade(),
-
-                         ]);
+        feedback::create(
+            [
+                'stage_identifier' => 'final_agreed_1',
+                'submissionid' => $this->get_allocatable()->get_submission($this->get_coursework())->id(),
+                'grade' => $this->automatic_grade(),
+            ]
+        );
     }
 
     /**
@@ -163,4 +164,13 @@ private function grades_as_percentages() {
             $initialfeedbacks);
         return $grades;
     }
+
+    /**
+     * Set percentage.
+     * @param int $percentage
+     * @return void
+     */
+    public function set_percentage(int $percentage) {
+        $this->percentage = $percentage;
+    }
 }

classes/cron.php

@@ -48,10 +48,11 @@ class cron {
      * @return bool
      */
     public static function run() {
-        echo "Starting coursework cron functions...\n";
+        if (!defined('PHPUNIT_TEST')) {
+            echo "Starting coursework cron functions...\n";
+        }
         self::finalise_any_submissions_where_the_deadline_has_passed();
         self::send_reminders_to_students();
-        // self::send_first_reminders_to_admins(); #90211934
         self::autorelease_feedbacks_where_the_release_date_has_passed();
         return true;
     }
@@ -149,7 +150,7 @@ private static function send_reminders_to_students() {
 
         self::send_email_reminders_to_students($userswhoneedreminding, $counts, self::EMAIL_TYPE_USER);
 
-        if (self::in_test_environment()) {
+        if (self::in_test_environment() && !defined('PHPUNIT_TEST')) {
             mtrace("cron coursework, sent {$counts['emails']} emails to {$counts['users']} users");
         }
         return true;
@@ -262,7 +263,7 @@ private static function send_first_reminders_to_admins() {
 
         $numberofcourseworks = count($courseworks);
 
-        if (self::in_test_environment()) {
+        if (self::in_test_environment() && !defined('PHPUNIT_TEST')) {
             mtrace("cron coursework, sent {$emailssent} reminder emails to the teachers and managers of {$numberofcourseworks}");
         }
 
@@ -345,8 +346,9 @@ private static function send_email_reminders_to_students(array $users, array &$c
      * Updates all DB columns where the deadline was before now, so that finalised = 1
      */
     private static function finalise_any_submissions_where_the_deadline_has_passed() {
-
-        echo 'Finalising submissions for courseworks where the deadlines have passed...';
+        if (!defined('PHPUNIT_TEST')) {
+            echo 'Finalising submissions for courseworks where the deadlines have passed...';
+        }
 
         $submissions = submission::unfinalised_past_deadline();
         foreach ($submissions as $submission) {
@@ -382,9 +384,10 @@ public static function in_test_environment() {
      */
 
     private static function autorelease_feedbacks_where_the_release_date_has_passed() {
-
         global $DB;
-        echo 'Auto releasing feedbacks for courseworks where the release date have passed...';
+        if (!defined('PHPUNIT_TEST')) {
+            echo 'Auto releasing feedbacks for courseworks where the release date have passed...';
+        }
 
         $sql = "SELECT *
                  FROM {coursework} c

classes/export/csv/cells/finalgrade_cell.php

@@ -35,8 +35,9 @@ class finalgrade_cell extends cell_base {
      * @return null|string
      */
     public function get_cell($submission, $student, $stageidentifier) {
-
-        return $submission->get_final_grade() == false || $submission->editable_final_feedback_exist() ? '' : $this->get_actual_grade($submission->get_final_grade());
+        return $submission->get_final_grade() == false || $submission->editable_final_feedback_exist()
+            ? ''
+            : $this->get_actual_grade($submission->get_final_grade());
 
     }
 

classes/framework/table_base.php

@@ -63,7 +63,7 @@ abstract class table_base {
      *
      * @param \stdClass|int|array $dbrecord
      * @param bool $reload
-     * @return bool
+     * @return bool|object
      * @throws \coding_exception
      * @throws \dml_exception
      */

classes/models/feedback.php

@@ -477,7 +477,10 @@ public function get_submission() {
         if (!isset($this->submission) && !empty($this->submissionid)) {
             global $DB;
             $courseworkid = $this->courseworkid
-                ?? $DB->get_field(submission::$tablename, 'courseworkid', ['id' => $this->submissionid], MUST_EXIST);
+                ?? $DB->get_field(submission::$tablename, 'courseworkid', ['id' => $this->submissionid]);
+            if (!$courseworkid) {
+                return false;
+            }
             if (!isset(submission::$pool[$courseworkid])) {
                 submission::fill_pool_coursework($courseworkid);
             }
@@ -666,8 +669,10 @@ public static function get_object($courseworkid, $key, $params) {
      *
      */
     protected function post_save_hook() {
-        $courseworkid = $this->get_submission()->courseworkid;
-        self::remove_cache($courseworkid);
+        $submission = $this->get_submission();
+        if ($submission && $submission->courseworkid ?? false) {
+            self::remove_cache($submission->courseworkid);
+        }
     }
 
     /**

classes/models/submission.php

@@ -997,7 +997,7 @@ public function publish() {
         if (coursework_grade_item_update($this->get_coursework(), $studentgradestoupdate) == GRADE_UPDATE_OK) {
             if (!$this->is_published()) {
                 $this->update_attribute('firstpublished', time());
-                // send feedback released notification only when first published
+                // Send feedback released notification only when first published.
                 $mailer = new mailer($this->get_coursework());
                 $mailer->send_feedback_notification($this);
             }
@@ -1164,17 +1164,20 @@ public function max_number_of_feedbacks() {
     }
 
     /**
-     * @return array|bool
+     * @return array
      * @throws \coding_exception
      */
-    public function students_for_gradebook() {
+    public function students_for_gradebook(): array {
         if ($this->get_coursework()->is_configured_to_have_group_submissions()) {
             $students = groups_get_members($this->allocatableid);
             return $students;
         } else {
-            $students = [$this->get_allocatable()->id() => $this->get_allocatable()];
-            return $students;
+            $allocatable = $this->get_allocatable();
+            if ($allocatable) {
+                return [$allocatable->id() => $allocatable];
+            }
         }
+        return [];
     }
 
     /**

classes/models/user.php

@@ -48,7 +48,7 @@ class user extends table_base implements allocatable, moderatable {
     protected static $tablename = 'user';
 
     /**
-     * @param bool $data
+     * @param array|object|bool $data
      */
     public function __construct($data = false) {
         $allnames = \core_user\fields::get_name_fields();
@@ -201,7 +201,7 @@ public static function fill_pool($array) {
 
     /**
      * @param $id
-     * @return mixed
+     * @return self
      */
     public static function get_object($id) {
         if (!isset(self::$pool['id'][$id])) {

lib.php

@@ -158,9 +158,8 @@ function coursework_add_instance($formdata) {
     $formdata->timecreated = time();
 
     // You may have to add extra stuff in here.
-
-    // We have to check to see if this coursework has a deadline ifm it doesn't we need to set the
-    // Deadline to zero
+    // We have to check to see if this coursework has a deadline.
+    // If it doesn't we need to set the deadline to zero.
 
     $formdata->deadline = empty($formdata->deadline) ? 0 : $formdata->deadline;
     $subnotify = '';
@@ -173,7 +172,7 @@ function coursework_add_instance($formdata) {
     }
     $formdata->submissionnotification = $subnotify;
 
-    // If blindmarking is set we will rename files
+    // If blind marking is set we will rename files.
     if ($formdata->blindmarking == 1) {
 
         $formdata->renamefiles = 1;

tests/classes/allocation/form/table_processor_test.php

@@ -23,7 +23,6 @@
 use mod_coursework\allocation\table\processor;
 use mod_coursework\models\coursework;
 
-global $CFG;
 
 /**
  * This class takes the manual data about the teachers who should be allocated to various
@@ -44,13 +43,11 @@ final class table_processor_test extends advanced_testcase {
     use mod_coursework\test_helpers\factory_mixin;
 
     public function setUp(): void {
+        global $DB;
         $this->resetAfterTest();
         $this->setAdminUser();
 
         $generator = $this->getDataGenerator();
-        /**
-         * @var mod_coursework_generator $coursework_generator
-         */
         $courseworkgenerator = $generator->get_plugin_generator('mod_coursework');
 
         $this->course = $generator->create_course();
@@ -62,7 +59,9 @@ public function setUp(): void {
         $this->create_a_student();
         $this->create_a_teacher();
         $this->create_another_teacher();
-        $this->delete_all_auto_allocations_caused_by_enrol_hooks();
+
+        // Delete all auto allocations caused by enrol hooks.
+        $DB->delete_records('coursework_allocation_pairs');
     }
 
     public function test_process_rows_makes_a_new_assessor_allocation(): void {
@@ -132,9 +131,18 @@ public function test_process_rows_sets_the_stage_identifiers_for_new_assessor_al
     public function test_process_rows_alters_an_existing_allocation(): void {
 
         global $DB;
+        $this->coursework->update_attribute('numberofmarkers', 1);
 
-        $this->set_coursework_to_single_marker();
-        $allocation = $this->make_a_non_manual_allocation_for_teacher();
+        // Make a non manual allocation for teacher.
+        $allocation = \mod_coursework\models\allocation::build([
+            'courseworkid' => $this->coursework->id,
+            'allocatableid' => $this->student->id,
+            'allocatabletype' => 'user',
+            'assessorid' => $this->teacher->id,
+            'stage_identifier' => 'assessor_1',
+        ]);
+        $allocation->save();
+        $this->assertEquals($allocation->assessorid, $this->teacher->id);
 
         $testrows = [
             $this->student->id => [
@@ -144,21 +152,24 @@ public function test_process_rows_alters_an_existing_allocation(): void {
                 ],
             ],
         ];
-
         $processor = new processor($this->coursework);
         $processor->process_data($testrows);
 
+        $allocation->reload();
+        $this->assertEquals($allocation->assessorid, $this->otherteacher->id);
+
         $params = [
             'courseworkid' => $this->coursework->id,
             'allocatableid' => $this->student->id,
             'allocatabletype' => 'user',
             'stage_identifier' => 'assessor_1',
         ];
         $records = $DB->get_records('coursework_allocation_pairs', $params);
-        $this->assertEquals(1, $DB->count_records('coursework_allocation_pairs'), 'Too many allocations.');
-
+        $this->assertEquals(
+            1, count($records),
+            'Too many allocations ' . json_encode($records)
+        );
         $this->assertEquals($this->otherteacher->id, reset($records)->assessorid, 'Wrong teacher id');
-
     }
 
     public function test_that_missing_columns_dont_mess_it_up(): void {
@@ -170,49 +181,4 @@ public function test_that_missing_rows_dont_mess_it_up(): void {
         $processor = new processor($this->coursework);
         $processor->process_data();
     }
-
-    private function set_coursework_to_single_marker() {
-        $this->coursework->update_attribute('numberofmarkers', 1);
-    }
-
-    /**
-     */
-    private function make_a_non_manual_allocation_for_teacher() {
-        global $DB;
-
-        $allocation = new stdClass();
-        $allocation->assessorid = $this->teacher->id;
-        $allocation->courseworkid = $this->coursework->id;
-        $allocation->allocatableid = $this->student->id;
-        $allocation->allocatabletype = 'user';
-        $allocation->stage_identifier = 'assessor_1';
-
-        $allocation->id = $DB->insert_record('coursework_allocation_pairs', $allocation);
-
-        return $allocation;
-    }
-
-    /**
-     */
-    private function make_a_non_manual_moderator_allocation_for_teacher() {
-        global $DB;
-
-        $allocation = new stdClass();
-        $allocation->allocatableid = $this->student->id;
-        $allocation->allocatabletype = 'user';
-        $allocation->assessorid = $this->teacher->id;
-        $allocation->courseworkid = $this->coursework->id;
-        $allocation->stage_identifier = 'moderator_1';
-        $allocation->moderator = 1;
-
-        $allocation->id = $DB->insert_record('coursework_allocation_pairs', $allocation);
-
-        return $allocation;
-    }
-
-    private function delete_all_auto_allocations_caused_by_enrol_hooks() {
-        global $DB;
-
-        $DB->delete_records('coursework_allocation_pairs');
-    }
 }

tests/classes/allocation/strategy/percentages_test.php

@@ -20,8 +20,6 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-global $CFG;
-
 /**
  * Class mod_coursework_allocation_strategy_percentages_test
  * @property mixed otherteacher

tests/classes/allocation/strategy_test.php

@@ -22,8 +22,6 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-global $CFG;
-
 /**
  * Unit tests for the base allocation strategy class.
  * @group mod_coursework