Skip to content

Commit

Permalink
MDL-79351 completion: fix form_trait code smells
Browse files Browse the repository at this point in the history
  • Loading branch information
ferranrecio committed Sep 14, 2023
1 parent 9b2c445 commit f652b76
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 32 deletions.
6 changes: 3 additions & 3 deletions completion/classes/bulkedit_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 0 additions & 10 deletions completion/classes/defaultedit_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 10 additions & 1 deletion completion/classes/edit_base_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
21 changes: 4 additions & 17 deletions completion/classes/form/form_trait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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();

Expand All @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion course/moodleform_mod.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit f652b76

Please sign in to comment.