From e0145522c903c60c863b53764319dcaa8f5d2506 Mon Sep 17 00:00:00 2001 From: vtos Date: Tue, 13 Jun 2023 13:25:47 +0200 Subject: [PATCH] MDL-64648 enrol_manual: fixed default setting of 'expirynotify' When creating a course, a manual enrolment instance is added by default. The instance settings should inherit the values of those for the manual enrolment plugin and properly calculate its extra settings. The 'expirynotify' setting wasn't inherited correctly in case it had 'Enroller + Enrolled' value. A functional test was added to test the behaviour of settings inheritance. --- enrol/manual/lib.php | 9 +-- enrol/manual/tests/lib_test.php | 104 ++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/enrol/manual/lib.php b/enrol/manual/lib.php index b4221d005f9..3ae5d0fd176 100644 --- a/enrol/manual/lib.php +++ b/enrol/manual/lib.php @@ -127,18 +127,13 @@ class enrol_manual_plugin extends enrol_plugin { */ public function add_default_instance($course) { $expirynotify = $this->get_config('expirynotify', 0); - if ($expirynotify == 2) { - $expirynotify = 1; - $notifyall = 1; - } else { - $notifyall = 0; - } + $fields = array( 'status' => $this->get_config('status'), 'roleid' => $this->get_config('roleid', 0), 'enrolperiod' => $this->get_config('enrolperiod', 0), 'expirynotify' => $expirynotify, - 'notifyall' => $notifyall, + 'notifyall' => $expirynotify == 2 ? 1 : 0, 'expirythreshold' => $this->get_config('expirythreshold', 86400), ); return $this->add_instance($course, $fields); diff --git a/enrol/manual/tests/lib_test.php b/enrol/manual/tests/lib_test.php index c5ba4f69a85..a8a6e86a3bd 100644 --- a/enrol/manual/tests/lib_test.php +++ b/enrol/manual/tests/lib_test.php @@ -25,6 +25,7 @@ namespace enrol_manual; use course_enrolment_manager; +use stdClass; defined('MOODLE_INTERNAL') || die(); @@ -543,4 +544,107 @@ class lib_test extends \advanced_testcase { // Manual enrol has 2 enrol actions -- edit and unenrol. $this->assertCount(2, $actions); } + + /** + * Test how the default enrolment instance inherits its settings from the global plugin settings. + * + * @dataProvider default_enrolment_instance_data_provider + * @param stdClass $expectation + * @param stdClass $globalsettings + * @covers \enrol_manual::add_default_instance + */ + public function test_default_enrolment_instance_acquires_correct_settings(stdClass $expectation, stdClass $globalsettings) { + global $DB; + + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + + // Given the plugin is globally configured with the following settings. + $plugin = enrol_get_plugin('manual'); + $plugin->set_config('status', $globalsettings->status); + $plugin->set_config('roleid', $globalsettings->roleid); + $plugin->set_config('enrolperiod', $globalsettings->enrolperiod); + $plugin->set_config('expirynotify', $globalsettings->expirynotify); + $plugin->set_config('expirythreshold', $globalsettings->expirythreshold); + + // When creating a course. + $course = $generator->create_course(); + + // Then the default manual enrolment instance being created is properly configured. + $enrolinstance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual']); + $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_default_enrolment_instance_acquires_correct_settings(). + * + * @return array + */ + public function default_enrolment_instance_data_provider(): array { + $studentroles = get_archetype_roles('student'); + $studentrole = array_shift($studentroles); + + $teacherroles = get_archetype_roles('teacher'); + $teacherrole = array_shift($teacherroles); + + return [ + 'enabled, student role, no duration set, notify no one on expiry, 12 hours notification threshold' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 0, + 'notifyall' => 0, + 'expirythreshold' => 12 * HOURSECS, + ], + 'global settings' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 0, + 'expirythreshold' => 12 * HOURSECS, + ], + ], + 'enabled, student role, 72 hours duration, notify enroller only on expiry, 1 day notification threshold' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 72 * HOURSECS, + 'expirynotify' => 1, + 'notifyall' => 0, + 'expirythreshold' => DAYSECS, + ], + 'global settings' => (object) [ + 'status' => ENROL_INSTANCE_ENABLED, + 'roleid' => $studentrole->id, + 'enrolperiod' => 72 * HOURSECS, + 'expirynotify' => 1, + 'expirythreshold' => DAYSECS, + ], + ], + 'disabled, teacher role, no duration set, notify enroller and enrolled on expiry, 0 notification threshold' => [ + 'expectation' => (object) [ + 'status' => ENROL_INSTANCE_DISABLED, + 'roleid' => $teacherrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 2, + 'notifyall' => 1, + 'expirythreshold' => 0 + ], + 'global settings' => (object) [ + 'status' => ENROL_INSTANCE_DISABLED, + 'roleid' => $teacherrole->id, + 'enrolperiod' => 0, + 'expirynotify' => 2, + 'expirythreshold' => 0, + ], + ], + ]; + } }