Merge pull request #19 from ucl-isd/CTP-3796-Review-Sonarqube-feedbac…

…k-on-Coursework CTP-3796 sonarqube error fixes
ucl-isd · Sep 30, 2024 · 70ebdb7 · 70ebdb7
2 parents 8b760cf + fef3bef
commit 70ebdb7
Show file tree

Hide file tree

Showing 31 changed files with 106 additions and 107 deletions.
diff --git a/actions/feedbacks/create.php b/actions/feedbacks/create.php
@@ -32,7 +32,7 @@
 $isfinalgrade = optional_param('isfinalgrade', 0, PARAM_INT);
 $assessorid = optional_param('assessorid', $USER->id, PARAM_INT);
 $stageidentifier = optional_param('stage_identifier', '', PARAM_ALPHANUMEXT);
-$finalised = !!optional_param('submitbutton', 0, PARAM_TEXT);
+$finalised = (bool)optional_param('submitbutton', 0, PARAM_TEXT);
 $ajax = optional_param('ajax', 0, PARAM_INT);
 
 $params = [

diff --git a/actions/feedbacks/update.php b/actions/feedbacks/update.php
@@ -29,7 +29,7 @@
 global $CFG, $USER;
 
 $feedbackid = required_param('feedbackid', PARAM_INT);
-$finalised = !!optional_param('submitbutton', 0, PARAM_TEXT);
+$finalised = (bool)optional_param('submitbutton', 0, PARAM_TEXT);
 $ajax = optional_param('ajax', 0, PARAM_INT);
 $remove = !!optional_param('removefeedbackbutton', 0, PARAM_TEXT);
 $confirm = optional_param('confirm', 0, PARAM_INT);

diff --git a/actions/submissions/create.php b/actions/submissions/create.php
@@ -32,7 +32,7 @@
 $allocatableid = required_param('allocatableid', PARAM_INT);
 $allocatabletype = required_param('allocatabletype', PARAM_ALPHANUMEXT);
 $submissionid = optional_param('submissionid', 0, PARAM_INT);
-$finalised = !!optional_param('finalisebutton', 0, PARAM_TEXT);
+$finalised = (bool)optional_param('finalisebutton', 0, PARAM_TEXT);
 
 if (!in_array($allocatabletype, ['user', 'group'])) {
     throw new \mod_coursework\exceptions\access_denied(\mod_coursework\models\coursework::find($courseworkid),

diff --git a/actions/submissions/update.php b/actions/submissions/update.php
@@ -27,7 +27,7 @@
 require_once(dirname(__FILE__) . '/../../../../config.php');
 
 $submissionid = required_param('submissionid', PARAM_INT);
-$finalised = !!optional_param('finalisebutton', 0, PARAM_TEXT);
+$finalised = (bool)optional_param('finalisebutton', 0, PARAM_TEXT);
 
 $params = [
     'submissionid' => $submissionid,

diff --git a/classes/allocation/table/cell/data.php b/classes/allocation/table/cell/data.php
@@ -87,17 +87,17 @@ public function has_assessor() {
     /**
      * @return bool
      */
-    public function allocatable_should_be_in_sampling() {
+    public function allocatable_should_be_in_sampling(): bool {
         $key = $this->moderation_set_key();
-        return array_key_exists($key, $this->data) && !!$this->data[$key];
+        return array_key_exists($key, $this->data) && $this->data[$key];
     }
 
     /**
      * @return bool
      */
-    public function is_pinned() {
+    public function is_pinned(): bool {
         $key = $this->pinned_key();
-        return array_key_exists($key, $this->data) && !!$this->data[$key];
+        return array_key_exists($key, $this->data) && $this->data[$key];
     }
 
     /**

diff --git a/classes/assessor_feedback_row.php b/classes/assessor_feedback_row.php
@@ -171,8 +171,8 @@ public function get_time_modified() {
      *
      * @return bool
      */
-    public function has_feedback() {
-        return !!$this->get_feedback();
+    public function has_feedback(): bool {
+        return (bool)$this->get_feedback();
     }
 
     /**
@@ -206,8 +206,8 @@ public function get_submission() {
     /**
      * @return bool
      */
-    private function has_submission() {
-        return !!$this->get_submission();
+    private function has_submission(): bool {
+        return (bool)$this->get_submission();
     }
 
     /**
@@ -234,8 +234,8 @@ private function get_allocation() {
     /**
      * @return bool
      */
-    private function has_allocation() {
-        return !!$this->get_allocation();
+    private function has_allocation(): bool {
+        return (bool)$this->get_allocation();
     }
 }
 
diff --git a/classes/controllers/feedback_controller.php b/classes/controllers/feedback_controller.php
@@ -230,10 +230,8 @@ protected function create_feedback() {
                 $submission->publish();
             }
 
-            // Only implement auto feedback (automatic agreement) if the settings is set to disabled otherwise
-            // We will do this in the cron
-            // Only implement auto feedback (automatic agreement) if the settings is set to disabled otherwise
-            // We will do this in the cron
+            // Only implement auto feedback (automatic agreement) if the settings is set to disabled.
+            // Otherwise, we will do this in the cron.
             $gradeeditingtime = $teacherfeedback->get_coursework()->get_grade_editing_time();
 
             if (empty($gradeeditingtime) || time() > $teacherfeedback->timecreated + $gradeeditingtime) {
@@ -417,6 +415,7 @@ protected function update_feedback() {
         $teacherfeedback->save();
         $form->save_feedback_files($teacherfeedback);
 
+        $gradeeditingtime = $teacherfeedback->get_coursework()->get_grade_editing_time();
         if (empty($gradeeditingtime) || time() > $teacherfeedback->timecreated + $gradeeditingtime) {
             $this->try_auto_feedback_creation($teacherfeedback->get_submission());
         }

diff --git a/classes/export/csv/cells/agreedfeedback_cell.php b/classes/export/csv/cells/agreedfeedback_cell.php
@@ -48,16 +48,26 @@ public function get_cell($submission, $student, $stageidentifier) {
      * @throws \coding_exception
      */
     public function get_header($stage) {
-        return  get_string('agreedgradefeedback', 'coursework');
+        return get_string('agreedgradefeedback', 'coursework');
     }
 
-    public function validate_cell($value, $submissionid, $stageidentifier='', $uploadedgradecells  = []) {
-
+    /**
+     * Validate cell.
+     * @param string $value
+     * @param int $submissionid
+     * @param string $stageidentifier
+     * @param array $uploadedgradecells
+     * @return \lang_string|mixed|string|true
+     * @throws \coding_exception
+     * @throws \dml_exception
+     */
+    public function validate_cell($value, $submissionid, $stageidentifier = '', $uploadedgradecells  = []) {
         global $DB, $PAGE, $USER;
-
-        $stageidentifier = 'final_agreed_1';
-        $agreedgradecap = ['mod/coursework:addagreedgrade', 'mod/coursework:editagreedgrade',
-            'mod/coursework:addallocatedagreedgrade', 'mod/coursework:editallocatedagreedgrade'];
+        $stageidentfinal = 'final_agreed_1';
+        $agreedgradecap = [
+            'mod/coursework:addagreedgrade', 'mod/coursework:editagreedgrade',
+            'mod/coursework:addallocatedagreedgrade', 'mod/coursework:editallocatedagreedgrade',
+        ];
 
         if (has_any_capability($agreedgradecap, $PAGE->context)
             || has_capability('mod/coursework:administergrades', $PAGE->context)) {
@@ -70,52 +80,50 @@ public function validate_cell($value, $submissionid, $stageidentifier='', $uploa
                 return get_string('submissionnotreadyforagreedgrade', 'coursework');
             }
 
-            // Has the submission been published if yes then no further grades are allowed
+            // Has the submission been published if yes then no further grades are allowed.
             if ($submission->get_state() >= submission::PUBLISHED) {
                 return $submission->get_status_text();
             }
 
-            // If you have administer grades you can grade anything
+            // If you have administer grades you can grade anything.
             if (has_capability('mod/coursework:administergrades', $PAGE->context)) {
                 return true;
             }
 
             // Has this submission been graded if yes then check if the current user graded it (only if allocation is not enabled).
             $feedbackparams = [
                 'submissionid' => $submission->id,
-                'stage_identifier' => $stageidentifier,
+                'stage_identifier' => $stageidentfinal,
             ];
 
             $feedback = feedback::find($feedbackparams);
 
             $ability = new ability(user::find($USER), $this->coursework);
 
-            // Does a feedback exist for this stage
+            // Does a feedback exist for this stage.
             if (empty($feedback)) {
                 $feedbackparams = [
                     'submissionid' => $submissionid,
                     'assessorid' => $USER->id,
-                    'stage_identifier' => $stageidentifier,
+                    'stage_identifier' => $stageidentfinal,
                 ];
                 $newfeedback = feedback::build($feedbackparams);
 
-                // This is a new feedback check it against the new ability checks
-                if (!has_capability('mod/coursework:administergrades', $PAGE->context) && !has_capability('mod/coursework:addallocatedagreedgrade', $PAGE->context) && !$ability->can('new', $newfeedback)) {
+                // This is a new feedback check it against the new ability checks.
+                if (!has_capability('mod/coursework:administergrades', $PAGE->context)
+                    && !has_capability('mod/coursework:addallocatedagreedgrade', $PAGE->context)
+                    && !$ability->can('new', $newfeedback)) {
                     return get_string('nopermissiontogradesubmission', 'coursework');
                 }
             } else {
-                // This is a new feedback check it against the edit ability checks
+                // This is a new feedback check it against the edit ability checks.
                 if (!has_capability('mod/coursework:administergrades', $PAGE->context) && !$ability->can('edit', $feedback)) {
                     return get_string('nopermissiontoeditgrade', 'coursework');
                 }
             }
-
         } else {
             return get_string('nopermissiontoimportgrade', 'coursework');
         }
-
         return true;
-
     }
-
 }
diff --git a/classes/export/csv/cells/agreedgrade_cell.php b/classes/export/csv/cells/agreedgrade_cell.php
@@ -215,7 +215,7 @@ public function validate_cell($value, $submissionid, $stageidentifier='', $uploa
      * @param $value the value that should be checked to see if it is valid
      * @return bool
      */
-    function value_in_rubric($criteria,    $value) {
+    public function value_in_rubric($criteria,    $value) {
 
         global  $DB;
 
@@ -246,7 +246,7 @@ function value_in_rubric($criteria,    $value) {
      * @param $csvcells
      * @return array
      */
-    function get_rubrics($coursework, $csvcells) {
+    public function get_rubrics($coursework, $csvcells) {
 
         if ($coursework->is_using_rubric()  && $this->coursework->finalstagegrading != 1) {
 

diff --git a/classes/export/csv/cells/assessorgrade_cell.php b/classes/export/csv/cells/assessorgrade_cell.php
@@ -281,7 +281,7 @@ public function validate_cell($value, $submissionid, $stageidentifier='', $uploa
      * @param $value the value that should be checked to see if it is valid
      * @return bool
      */
-    function value_in_rubric($criteria,    $value) {
+    public function value_in_rubric($criteria,    $value) {
 
         global  $DB;
 
@@ -312,7 +312,7 @@ function value_in_rubric($criteria,    $value) {
      * @param $csvcells
      * @return array
      */
-    function get_rubrics($coursework, $csvcells) {
+    public function get_rubrics($coursework, $csvcells) {
 
         if ($coursework->is_using_rubric()) {
 

diff --git a/classes/export/csv/cells/feedbackcomments_cell.php b/classes/export/csv/cells/feedbackcomments_cell.php
@@ -40,9 +40,10 @@ class feedbackcomments_cell extends cell_base {
      */
     public function get_cell($submission, $student, $stageidentifier) {
 
-        $stageidentifier = ($this->coursework->get_max_markers() == 1)
-            ? "assessor_1" : $this->get_stage_identifier_for_assessor($submission, $student);
-        $grade = $submission->get_assessor_feedback_by_stage($stageidentifier);
+        $stageident = ($this->coursework->get_max_markers() == 1)
+            ? "assessor_1"
+            : $this->get_stage_identifier_for_assessor($submission, $student);
+        $grade = $submission->get_assessor_feedback_by_stage($stageident);
         return (!$grade || !isset($grade->feedbackcomment)) ? '' : strip_tags($grade->feedbackcomment);
     }
 

diff --git a/classes/export/csv/cells/singlegrade_cell.php b/classes/export/csv/cells/singlegrade_cell.php
@@ -39,17 +39,16 @@ class singlegrade_cell extends cell_base {
      * @return array|mixed|null|string
      */
     public function get_cell($submission, $student, $stageidentifier) {
+        $stageident = ($this->coursework->get_max_markers() == 1)
+            ? "assessor_1" : $this->get_stage_identifier_for_assessor($submission, $student);
 
-        $stageidentifier = ($this->coursework->get_max_markers() == 1) ? "assessor_1" : $this->get_stage_identifier_for_assessor($submission, $student);
-
-        $grade = $submission->get_assessor_feedback_by_stage($stageidentifier);
+        $grade = $submission->get_assessor_feedback_by_stage($stageident);
         if ($this->coursework->is_using_rubric()) {
             $gradedata = [];
-            $this->get_rubric_scores_gradedata($grade, $gradedata); // multiple parts are handled here
+            $this->get_rubric_scores_gradedata($grade, $gradedata); // Multiple parts are handled here.
         } else {
             $gradedata = (!$grade) ? '' : $this->get_actual_grade($grade->grade);
         }
-
         return   $gradedata;
     }
 
@@ -221,7 +220,7 @@ public function validate_cell($value, $submissionid, $stageidentifier='', $uploa
      * @param $value the value that should be checked to see if it is valid
      * @return bool
      */
-    function value_in_rubric($criteria,    $value) {
+    public function value_in_rubric($criteria,    $value) {
 
         global  $DB;
 
@@ -252,7 +251,7 @@ function value_in_rubric($criteria,    $value) {
      * @param $csvcells
      * @return array
      */
-    function get_rubrics($coursework, $csvcells) {
+    public function get_rubrics($coursework, $csvcells) {
 
         if ($coursework->is_using_rubric()) {
 

diff --git a/classes/export/import.php b/classes/export/import.php
@@ -161,7 +161,7 @@ public function validate_csv($content, $encoding, $delimeter, $csvcells) {
         return (!empty($errors)) ? $errors : false;
     }
 
-    function rubric_count_correct($csvheader, $linefromimportedcsv) {
+    public function rubric_count_correct($csvheader, $linefromimportedcsv) {
 
         // get criteria of rubrics and match it to grade cells
         if ($this->coursework->is_using_rubric()) {
@@ -229,7 +229,7 @@ function rubric_count_correct($csvheader, $linefromimportedcsv) {
         }
     }
 
-    function get_rubric_headers($csvheader) {
+    public function get_rubric_headers($csvheader) {
 
         // get criteria of rubrics and match it to grade cells
         if ($this->coursework->is_using_rubric()) {
@@ -545,7 +545,7 @@ public function process_csv($content, $encoding, $delimiter, $csvcells, $process
      * @param $value the value that we will retrieve the levelid for
      * @return bool
      */
-    function get_value_rubric_levelid($criteria,    $value) {
+    public function get_value_rubric_levelid($criteria,    $value) {
 
         global  $DB;
 

diff --git a/classes/forms/upload_allocations_form.php b/classes/forms/upload_allocations_form.php
@@ -26,13 +26,13 @@ class upload_allocations_form extends moodleform {
 
     private $cmid;
 
-    function __construct($cmid) {
+    public function __construct($cmid) {
         $this->cmid = $cmid;
 
         parent::__construct();
     }
 
-    function definition() {
+    public function definition() {
         $mform =& $this->_form;
 
         $mform->addElement('filepicker', 'allocationsdata', get_string('allocationsfile', 'coursework'), null, [ 'accepted_types' => '*.csv']);
@@ -60,7 +60,7 @@ function definition() {
         $this->add_action_buttons(true, get_string('uploadallocations', 'coursework'));
     }
 
-    function display() {
+    public function display() {
         return $this->_form->toHtml();
     }
 

diff --git a/classes/forms/upload_feedback_form.php b/classes/forms/upload_feedback_form.php
@@ -27,14 +27,14 @@ class upload_feedback_form extends moodleform {
     private $cmid;
     private $coursework;
 
-    function __construct($coursework, $cmid) {
+    public function __construct($coursework, $cmid) {
         $this->cmid = $cmid;
         $this->coursework = $coursework;
 
         parent::__construct();
     }
 
-    function definition() {
+    public function definition() {
         $mform =& $this->_form;
 
         $mform->addElement('filepicker', 'feedbackzip', get_string('feedbackzipfile', 'coursework'), null, [ 'accepted_types' => '*.zip']);
@@ -86,7 +86,7 @@ function definition() {
         $this->add_action_buttons(true, get_string('uploadfeedbackzip', 'coursework'));
     }
 
-    function display() {
+    public function display() {
         return $this->_form->toHtml();
     }