From f652b769fe34a86d7968d247acfd7cc9de765658 Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Thu, 14 Sep 2023 11:23:57 +0200 Subject: [PATCH] MDL-79351 completion: fix form_trait code smells --- completion/classes/bulkedit_form.php | 6 +++--- completion/classes/defaultedit_form.php | 10 ---------- completion/classes/edit_base_form.php | 11 ++++++++++- completion/classes/form/form_trait.php | 21 ++++----------------- course/moodleform_mod.php | 6 +++++- 5 files changed, 22 insertions(+), 32 deletions(-) diff --git a/completion/classes/bulkedit_form.php b/completion/classes/bulkedit_form.php index 27020dd4348d1..061ac53117fd9 100644 --- a/completion/classes/bulkedit_form.php +++ b/completion/classes/bulkedit_form.php @@ -56,11 +56,11 @@ protected function get_module_names() { /** * It will return the course module when $cms has only one course module; otherwise, null will be returned. * - * @return \stdClass|null + * @return cm_info|null */ - protected function get_cm(): ?\stdClass { + protected function get_cm(): ?cm_info { if (count($this->cms) === 1) { - return reset($this->cms)->get_course_module_record(); + return reset($this->cms); } // If there are multiple modules, so none will be selected. diff --git a/completion/classes/defaultedit_form.php b/completion/classes/defaultedit_form.php index b92e2044eaa67..a663622e7edb4 100644 --- a/completion/classes/defaultedit_form.php +++ b/completion/classes/defaultedit_form.php @@ -138,16 +138,6 @@ public function definition() { } } - /** - * There is no course module for this form, because it is used to update default completion settings. So it will - * always return null. - * - * @return \stdClass|null - */ - protected function get_cm(): ?\stdClass { - return null; - } - /** * This method has been overridden because the form identifier must be unique for each module type. * Otherwise, the form will display the same data for each module type once it's submitted. diff --git a/completion/classes/edit_base_form.php b/completion/classes/edit_base_form.php index e0b9c948501f2..a9e7b0830777f 100644 --- a/completion/classes/edit_base_form.php +++ b/completion/classes/edit_base_form.php @@ -218,11 +218,20 @@ public function definition() { $this->add_action_buttons($displaycancel); } + /** + * Return the course module of the form, if any. + * + * @return cm_info|null + */ + protected function get_cm(): ?cm_info { + return null; + } + /** * Each module which defines definition_after_data() must call this method. */ public function definition_after_data() { - $this->definition_after_data_completion(); + $this->definition_after_data_completion($this->get_cm()); } /** diff --git a/completion/classes/form/form_trait.php b/completion/classes/form/form_trait.php index 3c66ad4c51df1..9c699b76e6002 100644 --- a/completion/classes/form/form_trait.php +++ b/completion/classes/form/form_trait.php @@ -17,6 +17,7 @@ namespace core_completion\form; use core_grades\component_gradeitems; +use cm_info; /** * Completion trait helper, with methods to add completion elements and validate them. @@ -80,21 +81,6 @@ public function get_suffix(): string { return $this->suffix; } - /** - * Get the cm (course module) associated to this class. - * This method must be overriden by the class using this trait if it doesn't include a _cm property. - * - * @return \stdClass|null - * @throws \coding_exception If the class does not have a _cm property. - */ - protected function get_cm(): ?\stdClass { - if (property_exists($this, '_cm')) { - return $this->_cm; - } - - throw new \coding_exception('This class does not have a _cm property. Please, add it or override the get_cm() method.'); - } - /** * Add completion elements to the form. * @@ -347,8 +333,10 @@ protected function validate_completion(array $data): array { /** * It should be called from the definition_after_data() to setup the completion settings in the form. + * + * @param cm_info|null $cm The course module associated to this form. */ - protected function definition_after_data_completion(): void { + protected function definition_after_data_completion(?cm_info $cm = null): void { global $COURSE, $SITE; $mform = $this->get_form(); @@ -359,7 +347,6 @@ protected function definition_after_data_completion(): void { $suffix = $this->get_suffix(); // If anybody has completed the activity, these options will be 'locked'. - $cm = $this->get_cm(); // We use $SITE course for site default activity completion, so we don't need any unlock button. $completedcount = (empty($cm) || $COURSE->id == $SITE->id) ? 0 : $completion->count_user_data($cm); $freeze = false; diff --git a/course/moodleform_mod.php b/course/moodleform_mod.php index bb1aaee3583fe..68c180df2950f 100644 --- a/course/moodleform_mod.php +++ b/course/moodleform_mod.php @@ -318,7 +318,11 @@ function definition_after_data() { } // Completion: If necessary, freeze fields. - $this->definition_after_data_completion(); + $cm = null; + if ($this->_cm) { + $cm = get_fast_modinfo($COURSE)->get_cm($this->_cm->id); + } + $this->definition_after_data_completion($cm); // Freeze admin defaults if required (and not different from default) $this->apply_admin_locked_flags();