From 5dc60d9f94a4ff772e5e4e3c12a4a996d9bba42c Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Fri, 4 Aug 2023 17:01:59 +0800 Subject: [PATCH] MDL-78855 tool_uploadcourse: Create instance only if plugin supports it --- admin/tool/uploadcourse/classes/course.php | 174 ++++++++++-------- .../lang/en/tool_uploadcourse.php | 1 + .../uploadcourse/tests/behat/create.feature | 15 ++ .../uploadcourse/tests/behat/update.feature | 30 +++ .../fixtures/unsupported_enrol_method.csv | 3 + enrol/cohort/lib.php | 25 +++ enrol/guest/lib.php | 9 +- enrol/manual/lib.php | 9 + enrol/self/lib.php | 9 + enrol/upgrade.txt | 6 + lib/enrollib.php | 33 +++- 11 files changed, 234 insertions(+), 80 deletions(-) create mode 100644 admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv diff --git a/admin/tool/uploadcourse/classes/course.php b/admin/tool/uploadcourse/classes/course.php index 1316b0a398e..4a785114232 100644 --- a/admin/tool/uploadcourse/classes/course.php +++ b/admin/tool/uploadcourse/classes/course.php @@ -1044,7 +1044,7 @@ class tool_uploadcourse_course { $plugin = $enrolmentplugins[$instance->enrol]; // Ensure user is able to delete the instance. - if ($plugin->can_delete_instance($instance)) { + if ($plugin->can_delete_instance($instance) && $plugin->is_csv_upload_supported()) { $plugin->delete_instance($instance); } else { $this->error('errorcannotdeleteenrolment', @@ -1056,92 +1056,110 @@ class tool_uploadcourse_course { // Create/update enrolment. $plugin = $enrolmentplugins[$enrolmethod]; - $status = ($todisable) ? ENROL_INSTANCE_DISABLED : ENROL_INSTANCE_ENABLED; - $method = $plugin->fill_enrol_custom_fields($method, $course->id); + if ($plugin->is_csv_upload_supported()) { + $status = ($todisable) ? ENROL_INSTANCE_DISABLED : ENROL_INSTANCE_ENABLED; + $method = $plugin->fill_enrol_custom_fields($method, $course->id); - // Create a new instance if necessary. - if (empty($instance) && $plugin->can_add_instance($course->id)) { - $error = $plugin->validate_plugin_data_context($method, $course->id); - if ($error) { - $this->error('contextnotallowed', $error); - break; + // Create a new instance if necessary. + if (empty($instance) && $plugin->can_add_instance($course->id)) { + $error = $plugin->validate_plugin_data_context($method, $course->id); + if ($error) { + $this->error('contextnotallowed', $error); + break; + } + $instanceid = $plugin->add_default_instance($course); + if (!$instanceid) { + // Add instance with provided fields if plugin supports it. + $instanceid = $plugin->add_custom_instance($course, $method); + } + + $instance = $DB->get_record('enrol', ['id' => $instanceid]); + if ($instance) { + $instance->roleid = $plugin->get_config('roleid'); + // On creation the user can decide the status. + $plugin->update_status($instance, $status); + } } - $instanceid = $plugin->add_instance($course, $method); - $instance = $DB->get_record('enrol', ['id' => $instanceid]); - $instance->roleid = $plugin->get_config('roleid'); - // On creation the user can decide the status. - $plugin->update_status($instance, $status); - } - // Check if the we need to update the instance status. - if ($instance && $status != $instance->status) { - if ($plugin->can_hide_show_instance($instance)) { - $plugin->update_status($instance, $status); - } else { - $this->error('errorcannotdisableenrolment', - new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse', + // Check if the we need to update the instance status. + if ($instance && $status != $instance->status) { + if ($plugin->can_hide_show_instance($instance)) { + $plugin->update_status($instance, $status); + } else { + $this->error('errorcannotdisableenrolment', + new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse', + $plugin->get_instance_name($instance))); + break; + } + } + + if (empty($instance) || !$plugin->can_edit_instance($instance)) { + $this->error('errorcannotcreateorupdateenrolment', + new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse', $plugin->get_instance_name($instance))); - break; - } - } - if (empty($instance) || !$plugin->can_edit_instance($instance)) { - $this->error('errorcannotcreateorupdateenrolment', - new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse', - $plugin->get_instance_name($instance))); - - break; - } - - // Now update values. - $modifiedinstance = $instance; - - // Sort out the start, end and date. - $modifiedinstance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0); - $modifiedinstance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0); - - // Is the enrolment period set? - if (isset($method['enrolperiod']) && ! empty($method['enrolperiod'])) { - if (preg_match('/^\d+$/', $method['enrolperiod'])) { - $method['enrolperiod'] = (int) $method['enrolperiod']; - } else { - // Try and convert period to seconds. - $method['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $method['enrolperiod']); - } - $modifiedinstance->enrolperiod = $method['enrolperiod']; - } - if ($instance->enrolstartdate > 0 && isset($method['enrolperiod'])) { - $modifiedinstance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod']; - } - if ($instance->enrolenddate > 0) { - $modifiedinstance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate; - } - if ($instance->enrolenddate < $instance->enrolstartdate) { - $modifiedinstance->enrolenddate = $instance->enrolstartdate; - } - - // Sort out the given role. - if (isset($method['role']) || isset($method['roleid'])) { - if (isset($method['role'])) { - $role = $method['role']; - $roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST); - } else { - $roleid = $method['roleid']; - $role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST); - } - if (!$this->validate_role_context($course->id, $roleid)) { - $this->error('contextrolenotallowed', - new lang_string('contextrolenotallowed', 'core_role', $role)); break; } - $roleids = tool_uploadcourse_helper::get_role_ids(); - if (in_array($roleid, $roleids)) { - $modifiedinstance->roleid = $roleid; - } - } + // Now update values. + $modifiedinstance = $instance; - $plugin->update_instance($instance, $modifiedinstance); + // Sort out the start, end and date. + $modifiedinstance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0); + $modifiedinstance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0); + + // Is the enrolment period set? + if (isset($method['enrolperiod']) && !empty($method['enrolperiod'])) { + if (preg_match('/^\d+$/', $method['enrolperiod'])) { + $method['enrolperiod'] = (int)$method['enrolperiod']; + } else { + // Try and convert period to seconds. + $method['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $method['enrolperiod']); + } + $modifiedinstance->enrolperiod = $method['enrolperiod']; + } + if ($instance->enrolstartdate > 0 && isset($method['enrolperiod'])) { + $modifiedinstance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod']; + } + if ($instance->enrolenddate > 0) { + $modifiedinstance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate; + } + if ($instance->enrolenddate < $instance->enrolstartdate) { + $modifiedinstance->enrolenddate = $instance->enrolstartdate; + } + + // Sort out the given role. + if (isset($method['role']) || isset($method['roleid'])) { + if (isset($method['role'])) { + $role = $method['role']; + $roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST); + } else { + $roleid = $method['roleid']; + $role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST); + } + if (!$this->validate_role_context($course->id, $roleid)) { + $this->error('contextrolenotallowed', + new lang_string('contextrolenotallowed', 'core_role', $role)); + break; + } + + $roleids = tool_uploadcourse_helper::get_role_ids(); + if (in_array($roleid, $roleids)) { + $modifiedinstance->roleid = $roleid; + } + } + + // Sort out custom instance name. + if (isset($method['name'])) { + $modifiedinstance->name = $method['name']; + } + + $plugin->update_instance($instance, $modifiedinstance); + } else { + $this->error('errorunsupportedmethod', + new lang_string('errorunsupportedmethod', 'tool_uploadcourse', + $enrolmethod)); + } } } } diff --git a/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php b/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php index 1db81b0ab3b..7000b2f39a6 100644 --- a/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php +++ b/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php @@ -84,6 +84,7 @@ $string['errorcannotdeleteenrolment'] = 'Cannot delete enrolment method \'{$a}\' $string['errorcannotdisableenrolment'] = 'Cannot disable enrolment method \'{$a}\''; $string['errorwhilerestoringcourse'] = 'Error while restoring the course'; $string['errorwhiledeletingcourse'] = 'Error while deleting the course'; +$string['errorunsupportedmethod'] = 'Enrolment method \'{$a}\' is not supported in csv upload'; $string['generatedshortnameinvalid'] = 'The generated shortname is invalid'; $string['generatedshortnamealreadyinuse'] = 'The generated shortname is already in use'; $string['id'] = 'ID'; diff --git a/admin/tool/uploadcourse/tests/behat/create.feature b/admin/tool/uploadcourse/tests/behat/create.feature index b42e3e949c6..5bde8621cec 100644 --- a/admin/tool/uploadcourse/tests/behat/create.feature +++ b/admin/tool/uploadcourse/tests/behat/create.feature @@ -130,3 +130,18 @@ Feature: An admin can create courses using a CSV file And I should see "Role notallowed not allowed in this context." And I am on site homepage And I should see "coursez" + + @javascript + Scenario: Unsupported enrol methods are not created + Given the following config values are set as admin: + | enrol_plugins_enabled | manual,guest,lti | + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv" file to "File" filemanager + And I click on "Preview" "button" + When I click on "Upload courses" "button" + Then I should see "Course created" + And I should see "Enrolment method 'enrol_lti_plugin' is not supported in csv upload" + And I am on the "C2" "enrolment methods" page + And I should see "manualtest" + And I should not see "ltitest" diff --git a/admin/tool/uploadcourse/tests/behat/update.feature b/admin/tool/uploadcourse/tests/behat/update.feature index 3d86dc6924b..310108095d4 100644 --- a/admin/tool/uploadcourse/tests/behat/update.feature +++ b/admin/tool/uploadcourse/tests/behat/update.feature @@ -57,3 +57,33 @@ Feature: An admin can update courses using a CSV file And I should see "Field 3: b" And I should see "Field 4: Hello" And I should see "Field 5: Goodbye" + + @javascript + Scenario: Unsupported enrol methods are not updated + Given the following config values are set as admin: + | enrol_plugins_enabled | manual,lti | + And the following "courses" exist: + | fullname | shortname | category | + | Course 2 | C2 | 0 | + And I am on the "C2" "enrolment methods" page + When I select "Publish as LTI tool" from the "Add method" singleselect + And the following fields match these values: + | LTI version | LTI Advantage | + And I set the following fields to these values: + | Custom instance name | Published course | + | Tool to be published | Course | + And I press "Add method" + And I should see "Published course" + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv" file to "File" filemanager + And I click on "Preview" "button" + And I click on "Upload courses" "button" + Then I should see "Enrolment method 'enrol_lti_plugin' is not supported in csv upload" + And I should see "Courses errors: 1" + And I am on the "C2" "enrolment methods" page + And I should see "manualtest" + And I should not see "Manual enrolments" + And I should see "Published course" + And I should not see "ltitest" diff --git a/admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv b/admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv new file mode 100644 index 00000000000..7892c5cc7ee --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/unsupported_enrol_method.csv @@ -0,0 +1,3 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_name +C2,Course 2,0,manual,student,manualtest +C2,Course 2,0,lti,student,ltitest diff --git a/enrol/cohort/lib.php b/enrol/cohort/lib.php index 5e59a9aa0f5..a24701a96d8 100644 --- a/enrol/cohort/lib.php +++ b/enrol/cohort/lib.php @@ -609,6 +609,31 @@ class enrol_cohort_plugin extends enrol_plugin { } return $error; } + + /** + * Add new instance of enrol plugin with custom settings, + * called when adding new instance manually or when adding new course. + * Used for example on course upload. + * + * Not all plugins support this. + * + * @param stdClass $course Course object + * @param array|null $fields instance fields + * @return int|null id of new instance or null if not supported + */ + public function add_custom_instance(stdClass $course, ?array $fields = null): ?int { + return $this->add_instance($course, $fields); + } + + + /** + * Check if enrolment plugin is supported in csv course upload. + * + * @return bool + */ + public function is_csv_upload_supported(): bool { + return true; + } } /** diff --git a/enrol/guest/lib.php b/enrol/guest/lib.php index aba166b9dfb..2e6ddf8f2b8 100644 --- a/enrol/guest/lib.php +++ b/enrol/guest/lib.php @@ -489,7 +489,14 @@ class enrol_guest_plugin extends enrol_plugin { return $errors; } - + /** + * Check if enrolment plugin is supported in csv course upload. + * + * @return bool + */ + public function is_csv_upload_supported(): bool { + return true; + } } /** diff --git a/enrol/manual/lib.php b/enrol/manual/lib.php index d894916f975..f1129ae6452 100644 --- a/enrol/manual/lib.php +++ b/enrol/manual/lib.php @@ -643,6 +643,15 @@ class enrol_manual_plugin extends enrol_plugin { return $errors; } + /** + * Check if enrolment plugin is supported in csv course upload. + * + * @return bool + */ + public function is_csv_upload_supported(): bool { + return true; + } + } /** diff --git a/enrol/self/lib.php b/enrol/self/lib.php index 1c931762e7f..28aea94d647 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -1067,6 +1067,15 @@ class enrol_self_plugin extends enrol_plugin { return $contact; } + + /** + * Check if enrolment plugin is supported in csv course upload. + * + * @return bool + */ + public function is_csv_upload_supported(): bool { + return true; + } } /** diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index 5bf66904d68..8d193cc614e 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -1,6 +1,12 @@ This files describes API changes in /enrol/* - plugins, information provided here is intended especially for developers. +=== 4.3 === + +* New is_csv_upload_supported() function has been created. It checks whether enrolment plugin is supported + in CSV course upload. Defaults to false. Override this function in your enrolment plugin if you want it to + be supported in CSV course upload. + === 4.2 === * New is_self_enrol_available() function has been created. Similar to can_self_enrol but without checking user capabilities. diff --git a/lib/enrollib.php b/lib/enrollib.php index f6de9ee76f4..1210b368a6c 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -2673,6 +2673,30 @@ abstract class enrol_plugin { return null; } + /** + * Add new instance of enrol plugin with custom settings, + * called when adding new instance manually or when adding new course. + * Used for example on course upload. + * + * Not all plugins support this. + * + * @param stdClass $course Course object + * @param array|null $fields instance fields + * @return int|null id of new instance or null if not supported + */ + public function add_custom_instance(stdClass $course, ?array $fields = null): ?int { + return null; + } + + /** + * Check if enrolment plugin is supported in csv course upload. + * + * @return bool + */ + public function is_csv_upload_supported(): bool { + return false; + } + /** * Update instance status * @@ -3460,7 +3484,14 @@ abstract class enrol_plugin { * @return array Errors */ public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null) : array { - return []; + $errors = []; + if (!$this->is_csv_upload_supported()) { + $errors['errorunsupportedmethod'] = + new lang_string('errorunsupportedmethod', 'tool_uploadcourse', + get_class($this)); + + } + return $errors; } /**