From 22129e914e2bb71d27b1056edfaf823dbb4d5ece Mon Sep 17 00:00:00 2001 From: vtos Date: Mon, 3 Jul 2023 11:19:32 +0200 Subject: [PATCH] MDL-61811 enrol_manual: fixed updating of 'notifyall' field When updating a manual enrolment instance, the value of 'notifyall' field must be calculated automatically based on the value of 'expirynotify' field. The relevant plugin's method missed that, thus, the value of 'notifyall' field was never updated when editing a manual enrolment instance and changing the 'expirynotify' setting. A functional test has been added to test the entire behaviour of the relevant plugin's method. --- enrol/manual/lib.php | 3 + enrol/manual/tests/lib_test.php | 114 ++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/enrol/manual/lib.php b/enrol/manual/lib.php index d894916f975..d5d7ec3d860 100644 --- a/enrol/manual/lib.php +++ b/enrol/manual/lib.php @@ -173,6 +173,9 @@ class enrol_manual_plugin extends enrol_plugin { } } } + + $data->notifyall = $data->expirynotify == 2 ? 1 : 0; + return parent::update_instance($instance, $data); } diff --git a/enrol/manual/tests/lib_test.php b/enrol/manual/tests/lib_test.php index a8a6e86a3bd..0f4b09bdc10 100644 --- a/enrol/manual/tests/lib_test.php +++ b/enrol/manual/tests/lib_test.php @@ -647,4 +647,118 @@ class lib_test extends \advanced_testcase { ], ]; } + + /** + * Tests an enrolment instance is updated properly. + * + * @covers \enrol_manual::update_instance + * @dataProvider update_enrolment_instance_data_provider + * + * @param stdClass $expectation + * @param stdClass $updatedata + */ + public function test_enrolment_instance_is_updated(stdClass $expectation, stdClass $updatedata): void { + global $DB; + + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + + $studentroles = get_archetype_roles('student'); + $studentrole = array_shift($studentroles); + + // Given the plugin is globally configured with the following settings. + $plugin = enrol_get_plugin('manual'); + $plugin->set_config('status', ENROL_INSTANCE_ENABLED); + $plugin->set_config('roleid', $studentrole->id); + $plugin->set_config('enrolperiod', 30 * DAYSECS); + $plugin->set_config('expirynotify', 1); + $plugin->set_config('expirythreshold', 2 * DAYSECS); + + // And a course is created with the default enrolment instance. + $course = $generator->create_course(); + + // When the enrolment instance is being updated. + $enrolinstance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual']); + $successfullyupdated = $plugin->update_instance($enrolinstance, $updatedata); + + // Then the update is successful. + $this->assertTrue($successfullyupdated); + + // And the updated enrolment instance contains the expected values. + $enrolinstance = $DB->get_record('enrol', ['id' => $enrolinstance->id]); + $this->assertEquals($expectation->status, $enrolinstance->status); + $this->assertEquals($expectation->roleid, $enrolinstance->roleid); + $this->assertEquals($expectation->enrolperiod, $enrolinstance->enrolperiod); + $this->assertEquals($expectation->expirynotify, $enrolinstance->expirynotify); + $this->assertEquals($expectation->notifyall, $enrolinstance->notifyall); + $this->assertEquals($expectation->expirythreshold, $enrolinstance->expirythreshold); + } + + /** + * Data provider for test_enrolment_instance_is_updated(). + * + * @return array + */ + public function update_enrolment_instance_data_provider(): array { + $studentroles = get_archetype_roles('student'); + $studentrole = array_shift($studentroles); + + $teacherroles = get_archetype_roles('teacher'); + $teacherrole = array_shift($teacherroles); + + return [ + 'disabled, all the others are default' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_DISABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 30 * DAYSECS, + 'expirynotify' => 1, + 'notifyall' => 0, + 'expirythreshold' => 2 * DAYSECS, + ], + 'update data' => (object) [ + 'status' => ENROL_INSTANCE_DISABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 30 * DAYSECS, + 'expirynotify' => 1, + 'expirythreshold' => 2 * DAYSECS, + ], + ], + 'enabled, teacher role, no duration set, notify no one on expiry, 0 notification threshold' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $teacherrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 0, + 'notifyall' => 0, + 'expirythreshold' => 0, + ], + 'update data' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $teacherrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 0, + 'expirythreshold' => 0, + ], + ], + 'notify enroller and enrolled on expiry, all the others are default' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 30 * DAYSECS, + 'expirynotify' => 2, + 'notifyall' => 1, + 'expirythreshold' => 2 * DAYSECS, + ], + 'update data' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 30 * DAYSECS, + 'expirynotify' => 2, + 'expirythreshold' => 2 * DAYSECS, + ], + ], + ]; + } }