CTP-3560 coursework tests review

classes/allocation/table/builder.php

@@ -38,6 +38,7 @@
  * Represents the table that will show all students and all markers so that they can be matched up with one another for grading.
  * Various automatic strategies will be available for this, but the manual override happens here.
  */
+#[\AllowDynamicProperties]
 class builder {
 
     /**

classes/controllers/feedback_controller.php

@@ -74,16 +74,19 @@ protected function show_feedback() {
 
         $teacherfeedback = new feedback($this->params['feedbackid']);
 
-        $ability = new ability(user::find($USER), $this->coursework);
-        $ability->require_can('show', $teacherfeedback);
+        $user = user::find($USER);
+        if ($user) {
+            $ability = new ability($user, $this->coursework);
+            $ability->require_can('show', $teacherfeedback);
 
-        $renderer = $this->get_page_renderer();
-        $html = $renderer->show_feedback_page($teacherfeedback, $ajax);
+            $renderer = $this->get_page_renderer();
+            $html = $renderer->show_feedback_page($teacherfeedback, (bool)$ajax);
 
-        if (empty($ajax)) {
-            echo $html;
-        } else {
-            echo json_encode(['success' => true, 'formhtml' => $html]);
+            if (empty($ajax)) {
+                echo $html;
+            } else {
+                echo json_encode(['success' => true, 'formhtml' => $html]);
+            }
         }
     }
 

classes/export/csv/cells/feedbackcomments_cell.php

@@ -40,9 +40,10 @@ class feedbackcomments_cell extends cell_base {
      */
     public function get_cell($submission, $student, $stage_identifier) {
 
-        $stage_identifier = ($this->coursework->get_max_markers() == 1) ? "assessor_1" : $this->get_stage_identifier_for_assessor($submission, $student);
+        $stage_identifier = ($this->coursework->get_max_markers() == 1)
+            ? "assessor_1" : $this->get_stage_identifier_for_assessor($submission, $student);
         $grade = $submission->get_assessor_feedback_by_stage($stage_identifier);
-        return   (!$grade) ? '' : strip_tags($grade->feedbackcomment);
+        return (!$grade || !isset($grade->feedbackcomment)) ? '' : strip_tags($grade->feedbackcomment);
     }
 
     /**

classes/framework/table_base.php

@@ -20,7 +20,7 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-namespace mod_coursework\framework;;
+namespace mod_coursework\framework;
 
 use moodle_database;
 use stdClass;

classes/grade_judge.php

@@ -65,10 +65,14 @@ public function get_grade_capped_by_submission_time($submission) {
     }
 
     /**
-     * @param $grade
+     * @param int|float $grade
      * @return float
      */
     private function round_grade_decimals($grade) {
+        if ($grade === '' || $grade === null) {
+            // Avoid PHPUnit exception passing null or empty string to round().
+            return null;
+        }
         return round($grade, 2);
     }
 
@@ -145,10 +149,10 @@ public function has_feedback_that_is_promoted_to_gradebook($submission) {
 
     /**
      * @param submission $submission
-     * @return int
+     * @return int|null
      */
     public function get_time_graded($submission) {
-        return $this->get_feedback_that_is_promoted_to_gradebook($submission)->timemodified;
+        return $this->get_feedback_that_is_promoted_to_gradebook($submission)->timemodified ?? null;
     }
 
     /**

classes/models/allocation.php

@@ -35,6 +35,7 @@
  * @property mixed allocatableid
  * @property mixed allocatabletype
  */
+#[\AllowDynamicProperties]
 class allocation extends table_base {
 
     /**

classes/models/assessment_set_membership.php

@@ -38,6 +38,7 @@
  * @package mod_coursework\models
  *
  */
+#[\AllowDynamicProperties]
 class assessment_set_membership extends table_base implements moderatable {
 
     /**

classes/models/coursework.php

@@ -84,6 +84,7 @@
  * @property mixed startdate
  * @author administrator
  */
+#[\AllowDynamicProperties]
 class coursework extends table_base {
 
     /**
@@ -882,13 +883,10 @@ public function early_finalisation_allowed() {
      * @return void
      */
     public function publish_grades() {
-
-        $submisisons = $this->get_submissions_to_publish();
-
-        foreach ($submisisons as $submisison) {
-            $submisison->publish();
+        $submissions = $this->get_submissions_to_publish();
+        foreach ($submissions as $submission) {
+            $submission->publish();
         }
-
     }
 
     /**

classes/models/deadline_extension.php

@@ -36,6 +36,7 @@
  * @property mixed allocatableid
  * @package mod_coursework\models
  */
+#[\AllowDynamicProperties]
 class deadline_extension extends table_base {
 
     /**

classes/models/feedback.php

@@ -38,6 +38,7 @@
  * @property mixed stage_identifier
  * @property int feedback_manager
  */
+#[\AllowDynamicProperties]
 class feedback extends table_base {
 
     /**
@@ -475,7 +476,8 @@ public function get_submission() {
 
         if (!isset($this->submission) && !empty($this->submissionid)) {
             global $DB;
-            $coursework_id = $this->courseworkid ?? $DB->get_field(submission::$table_name, 'courseworkid', ['id' => $this->submissionid], MUST_EXIST);
+            $coursework_id = $this->courseworkid
+                ?? $DB->get_field(submission::$table_name, 'courseworkid', ['id' => $this->submissionid], MUST_EXIST);
             if (!isset(submission::$pool[$coursework_id])) {
                 submission::fill_pool_coursework($coursework_id);
             }
@@ -591,7 +593,7 @@ public static function fill_pool_coursework($coursework_id) {
         if (isset(self::$pool[$coursework_id])) {
             return;
         }
-        if (submission::$pool[$coursework_id]) {
+        if (submission::$pool[$coursework_id] ?? null) {
             $submission_ids = submission::$pool[$coursework_id]['id'];
         } else {
             $submission_ids = $DB->get_records(submission::$table_name, ['courseworkid' => $coursework_id], '', 'id');
@@ -650,7 +652,7 @@ public static function fill_pool_submissions($coursework_id, $submission_ids) {
      * @param $coursework_id
      * @param $key
      * @param $params
-     * @return bool
+     * @return self|bool
      */
     public static function get_object($coursework_id, $key, $params) {
         if (!isset(self::$pool[$coursework_id])) {

classes/models/group.php

@@ -40,6 +40,7 @@
  * @property mixed courseid
  * @package mod_coursework\models
  */
+#[\AllowDynamicProperties]
 class group extends table_base implements allocatable, moderatable {
 
     use allocatable_functions;

classes/models/module.php

@@ -29,6 +29,7 @@
 /**
  * Represents a row in the modules table.
  */
+#[\AllowDynamicProperties]
 class module extends table_base {
 
     /**

classes/models/null_feedback.php

@@ -27,6 +27,7 @@
  * class.
  *
  */
+#[\AllowDynamicProperties]
 class null_feedback {
 
     /**

classes/models/plagiarism_flag.php

@@ -142,7 +142,7 @@ protected static function get_cache_array($coursework_id) {
      * @param $coursework_id
      * @param $key
      * @param $params
-     * @return bool
+     * @return self|bool
      */
     public static function get_object($coursework_id, $key, $params) {
         if (!isset(self::$pool[$coursework_id])) {

classes/models/submission.php

@@ -51,6 +51,7 @@
  * @property mixed timesubmitted
  * @property mixed lastpublished
  */
+#[\AllowDynamicProperties]
 class submission extends table_base implements \renderable {
 
     /**

classes/models/user.php

@@ -37,6 +37,7 @@
  * Class user
  * @package mod_coursework\models
  */
+#[\AllowDynamicProperties]
 class user extends table_base implements allocatable, moderatable {
 
     use allocatable_functions;

classes/render_helpers/grading_report/sub_rows/multi_marker_feedback_sub_rows.php

@@ -38,6 +38,7 @@
 /**
  * Class no_sub_rows
  */
+#[\AllowDynamicProperties]
 class multi_marker_feedback_sub_rows implements sub_rows_interface {
 
     /**
@@ -268,7 +269,8 @@ protected function new_feedaback_link($feedback_row) {
         global $USER, $OUTPUT;
 
         $this->already_shown_a_new_buton = true;
-        $this->displaytable = true;
+//        $this->displaytable = true; //todo this is deprecated and causes behat exception - was it doing anything useful?
+
         // New
         $linktitle = get_string('newfeedback', 'coursework');
 

classes/stages/assessor.php

@@ -30,6 +30,7 @@
  *
  * @package mod_coursework
  */
+#[\AllowDynamicProperties]
 class assessor extends base {
 
     /**

classes/stages/final_agreed.php

@@ -30,6 +30,7 @@
  *
  * @package mod_coursework
  */
+#[\AllowDynamicProperties]
 class final_agreed extends base {
 
     /**

classes/test_helpers/factory_mixin.php

@@ -74,6 +74,16 @@ trait factory_mixin {
      */
     protected $other_teacher;
 
+    /**
+     * @var group
+     */
+    protected $group;
+
+    /**
+     * @var
+     */
+    protected $final_feedback;
+
     /**
      * @return user
      */

db/install.xml

@@ -33,7 +33,7 @@
         <FIELD NAME="moderatorallocationstrategy" TYPE="char" LENGTH="255" NOTNULL="false" SEQUENCE="false" COMMENT="Strategy for matching moderators to students in the moderation set. Matches the end of a coursework_allocation_strategy_xxx class"/>
         <FIELD NAME="viewothersfeedback" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="If set to true, markers will be able to view the feedback of other markers once they have completed their own. This runs counter to the principles of blind marking (might cause unfair bias), but is useful for some situations."/>
         <FIELD NAME="autoreleasefeedback" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="If true, feedback (but not grades) will be released automatically to the student when the deadline passes."/>
-        <FIELD NAME="retrospectivemoderation" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="This flag, which in future will be replaced bu fully configurable marking stages, tells us to ignore the marks that the moderator gives so that they are just indicative of the quality of the first marker. If they are very different, the coursework can be switched to allow a full remark, where the moderator marks become marker2 marks."/>
+        <FIELD NAME="retrospectivemoderation" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="This flag, which in future will be replaced by fully configurable marking stages, tells us to ignore the marks that the moderator gives so that they are just indicative of the quality of the first marker. If they are very different, the coursework can be switched to allow a full remark, where the moderator marks become marker2 marks."/>
         <FIELD NAME="studentviewcomponentfeedbacks" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="If true, the students can see feedbacks from the component marks given by teachers before the final grade is agreed. If false, they only see the final feedback. Only relevant to multiple marked courseworks."/>
         <FIELD NAME="studentviewmoderatorfeedbacks" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Can students view feedback from moderators?"/>
         <FIELD NAME="strictanonymity" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="If we allow publishing when blind marking is enabled and some students have not submitted, or have an extension, then it may be possible for markers to deduce the student's identity. If this setting is off, such publishing is allowed."/>

renderers/page_renderer.php

@@ -43,14 +43,18 @@ class mod_coursework_page_renderer extends plugin_renderer_base {
     /**
      * @param feedback $feedback
      */
-    public function show_feedback_page($feedback, $ajax) {
+    public function show_feedback_page($feedback, bool $ajax) {
         $html = '';
 
         $object_renderer = $this->get_object_renderer();
 
-        if (empty($ajax))  $html .= $this->output->header();
+        if (!$ajax) {
+            $html .= $this->output->header();
+        }
         $html .= $object_renderer->render_feedback($feedback);
-        if (empty($ajax)) $html .= $this->output->header();
+        if (!$ajax)  {
+            $html .= $this->output->footer();
+        }
 
         return $html;
     }