From acd52a33ec2923dc804fa3a62c8fcb3d59c2a3ee Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Thu, 20 Nov 2014 18:12:02 +0800 Subject: [PATCH] MDL-43127 tool_uploadcourse: Add support for more enrolment methods --- admin/tool/uploadcourse/classes/course.php | 72 ++++++++-- admin/tool/uploadcourse/classes/helper.php | 15 ++- admin/tool/uploadcourse/classes/processor.php | 13 +- admin/tool/uploadcourse/classes/tracker.php | 18 ++- .../lang/en/tool_uploadcourse.php | 2 + admin/tool/uploadcourse/tests/course_test.php | 127 ++++++++++++++++++ admin/tool/uploadcourse/tests/helper_test.php | 4 +- 7 files changed, 233 insertions(+), 18 deletions(-) diff --git a/admin/tool/uploadcourse/classes/course.php b/admin/tool/uploadcourse/classes/course.php index ff2283304d0..f082eabd406 100644 --- a/admin/tool/uploadcourse/classes/course.php +++ b/admin/tool/uploadcourse/classes/course.php @@ -108,6 +108,9 @@ class tool_uploadcourse_course { static protected $importoptionsdefaults = array('canrename' => false, 'candelete' => false, 'canreset' => false, 'reset' => false, 'restoredir' => null, 'shortnametemplate' => null); + /** @var array special fields used when processing the enrolment methods. */ + static protected $enrolmentspecialfields = array('delete', 'disable', 'startdate', 'enddate', 'enrolperiod', 'role'); + /** * Constructor * @@ -655,9 +658,18 @@ class tool_uploadcourse_course { return false; } + // Getting the enrolment data. + $errors = array(); + $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata, $errors); + if (!empty($errors)) { + foreach ($errors as $key => $message) { + $this->error($key, $message); + } + return false; + } + // Saving data. $this->data = $coursedata; - $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata); // Restore data. // TODO Speed up things by not really extracting the backup just yet, but checking that @@ -769,10 +781,20 @@ class tool_uploadcourse_course { return; } + $cannotaddmethods = array(); $enrolmentplugins = tool_uploadcourse_helper::get_enrolment_plugins(); $instances = enrol_get_instances($course->id, false); foreach ($enrolmentdata as $enrolmethod => $method) { + $plugin = $enrolmentplugins[$enrolmethod]; + + // TODO MDL-48362 Abstract the logic to prevent it to be tied to the + // user interface. Ideally a plugin should have a method that returns + // whether or not a new instance can be added to the course rather than + // using enrol_plugin::get_newinstance_link() to figure that out. + $canadd = $plugin->get_newinstance_link($course->id); + + // TODO MDL-43820 Handle multiple instances of the same type. $instance = null; foreach ($instances as $i) { if ($i->enrol == $enrolmethod) { @@ -806,30 +828,57 @@ class tool_uploadcourse_course { } } } else { - $plugin = null; if (empty($instance)) { - $plugin = $enrolmentplugins[$enrolmethod]; + + // Check if we can create a new instance of this enrolment method. + if (!$canadd) { + $cannotaddmethods[] = $enrolmethod; + continue; + } + + // Some plugins do not implement enrol_plugin::add_default_instance(), + // but we will try anyway and call enrol_plugin::add_instance() if needed. + $id = $plugin->add_default_instance($course); + if (empty($id)) { + $id = $plugin->add_instance($course); + } + $instance = new stdClass(); - $instance->id = $plugin->add_default_instance($course); + $instance->id = $id; $instance->roleid = $plugin->get_config('roleid'); $instance->status = ENROL_INSTANCE_ENABLED; } else { - $plugin = $enrolmentplugins[$instance->enrol]; $plugin->update_status($instance, ENROL_INSTANCE_ENABLED); } // Now update values. foreach ($method as $k => $v) { + if (in_array($k, self::$enrolmentspecialfields)) { + // Skip the special import keys. + continue; + } $instance->{$k} = $v; } // Sort out the start, end and date. - $instance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0); - $instance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0); + if (isset($method['startdate'])) { + $instance->enrolstartdate = strtotime($method['startdate']); + } else if (!isset($instance->enrolstartdate)) { + $instance->enrolstartdate = 0; + } + if (isset($method['enddate'])) { + $instance->enrolenddate = strtotime($method['enddate']); + } else if (!isset($instance->enrolenddate)) { + $instance->enrolenddate = 0; + } // Is the enrolment period set? - if (isset($method['enrolperiod']) && ! empty($method['enrolperiod'])) { - if (preg_match('/^\d+$/', $method['enrolperiod'])) { + if (isset($method['enrolperiod'])) { + if (empty($method['enrolperiod'])) { + // Let's just make sure that it's 0. + $method['enrolperiod'] = 0; + } else if (preg_match('/^\d+$/', $method['enrolperiod'])) { + // Cast integers to integers. $method['enrolperiod'] = (int) $method['enrolperiod']; } else { // Try and convert period to seconds. @@ -859,6 +908,11 @@ class tool_uploadcourse_course { $DB->update_record('enrol', $instance); } } + + if (!empty($cannotaddmethods)) { + $this->error('cannotaddenrolmentmethods', new lang_string('cannotaddenrolmentmethods', 'tool_uploadcourse', + implode(', ', $cannotaddmethods))); + } } /** diff --git a/admin/tool/uploadcourse/classes/helper.php b/admin/tool/uploadcourse/classes/helper.php index 2873f3b783f..0a3e12e9698 100644 --- a/admin/tool/uploadcourse/classes/helper.php +++ b/admin/tool/uploadcourse/classes/helper.php @@ -134,9 +134,10 @@ class tool_uploadcourse_helper { * ) * * @param array $data data to extract the enrolment data from. + * @param array $errors will be populated with errors found. * @return array */ - public static function get_enrolment_data($data) { + public static function get_enrolment_data($data, &$errors = array()) { $enrolmethods = array(); $enroloptions = array(); foreach ($data as $field => $value) { @@ -158,16 +159,26 @@ class tool_uploadcourse_helper { // Combining enrolment methods and their options in a single array. $enrolmentdata = array(); + $unknownmethods = array(); + $methodsnotsupported = array(); if (!empty($enrolmethods)) { $enrolmentplugins = self::get_enrolment_plugins(); foreach ($enrolmethods as $key => $method) { if (!array_key_exists($method, $enrolmentplugins)) { - // Error! + // Unknown enrolment method. + $unknownmethods[] = $method; continue; } $enrolmentdata[$enrolmethods[$key]] = $enroloptions[$key]; } } + + // Logging errors related to enrolment methods. + if (!empty($unknownmethods)) { + $errors['unknownenrolmentmethods'] = new lang_string('unknownenrolmentmethods', 'tool_uploadcourse', + implode(', ', $unknownmethods)); + } + return $enrolmentdata; } diff --git a/admin/tool/uploadcourse/classes/processor.php b/admin/tool/uploadcourse/classes/processor.php index 8963f7eee5e..27438467f30 100644 --- a/admin/tool/uploadcourse/classes/processor.php +++ b/admin/tool/uploadcourse/classes/processor.php @@ -207,6 +207,7 @@ class tool_uploadcourse_processor { $course = $this->get_course($data); if ($course->prepare()) { $course->proceed(); + $outcome = 1; $status = $course->get_statuses(); if (array_key_exists('coursecreated', $status)) { @@ -217,11 +218,19 @@ class tool_uploadcourse_processor { $deleted++; } + // Errors can occur even though the course preparation returned true, often because + // some checks could not be done in course::prepare() because it requires the course to exist. + if ($course->has_errors()) { + $status += $course->get_errors(); + $errors++; + $outcome = 2; + } + $data = array_merge($data, $course->get_data(), array('id' => $course->get_id())); - $tracker->output($this->linenb, true, $status, $data); + $tracker->output($this->linenb, $outcome, $status, $data); } else { $errors++; - $tracker->output($this->linenb, false, $course->get_errors(), $data); + $tracker->output($this->linenb, 0, $course->get_errors(), $data); } } diff --git a/admin/tool/uploadcourse/classes/tracker.php b/admin/tool/uploadcourse/classes/tracker.php index 0daa84527c5..3ac78b4437f 100644 --- a/admin/tool/uploadcourse/classes/tracker.php +++ b/admin/tool/uploadcourse/classes/tracker.php @@ -136,7 +136,8 @@ class tool_uploadcourse_tracker { * Output one more line. * * @param int $line line number. - * @param bool $outcome success or not? + * @param int $outcome 0 for failure, 1 for success, 2 for success with errors. Use 2 when + * most of the process succeeded but there might have been outstanding errors. * @param array $status array of statuses. * @param array $data extra data to display. * @return void @@ -148,9 +149,16 @@ class tool_uploadcourse_tracker { } if ($this->outputmode == self::OUTPUT_PLAIN) { + if ($outcome == 1) { + $ok = 'OK'; + } else if (!$outcome) { + $ok = 'NOK'; // Not OK. + } else { + $ok = 'EOK'; // Errors, but OK. + } $message = array( $line, - $outcome ? 'OK' : 'NOK', + $ok, isset($data['id']) ? $data['id'] : '', isset($data['shortname']) ? $data['shortname'] : '', isset($data['fullname']) ? $data['fullname'] : '', @@ -168,10 +176,12 @@ class tool_uploadcourse_tracker { if (is_array($status)) { $status = implode(html_writer::empty_tag('br'), $status); } - if ($outcome) { + if ($outcome == 1) { $outcome = $OUTPUT->pix_icon('i/valid', ''); - } else { + } else if (!$outcome) { $outcome = $OUTPUT->pix_icon('i/invalid', ''); + } else { + $outcome = $OUTPUT->pix_icon('i/caution', ''); } echo html_writer::start_tag('tr', array('class' => 'r' . $this->rownb % 2)); echo html_writer::tag('td', $line, array('class' => 'c' . $ci++)); diff --git a/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php b/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php index 96b847c5320..64e96c50093 100644 --- a/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php +++ b/admin/tool/uploadcourse/lang/en/tool_uploadcourse.php @@ -29,6 +29,7 @@ $string['allowrenames_help'] = 'Whether the rename field is accepted or not.'; $string['allowresets'] = 'Allow resets'; $string['allowresets_help'] = 'Whether the reset field is accepted or not.'; $string['cachedef_helper'] = 'Helper caching'; +$string['cannotaddenrolmentmethods'] = 'Cannot add the enrolment methods: {$a}'; $string['cannotdeletecoursenotexist'] = 'Cannot delete a course that does not exist'; $string['cannotgenerateshortnameupdatemode'] = 'Cannot generate a shortname when updates are allowed'; $string['cannotreadbackupfile'] = 'Cannot read the backup file'; @@ -109,6 +110,7 @@ $string['shortnametemplate'] = 'Template to generate a shortname'; $string['shortnametemplate_help'] = 'The short name of the course is displayed in the navigation. You may use template syntax here (%f = fullname, %i = idnumber), or enter an initial value that is incremented.'; $string['templatefile'] = 'Restore from this file after upload'; $string['templatefile_help'] = 'Select a file to use as a template for the creation of all courses.'; +$string['unknownenrolmentmethods'] = 'Unknown enrolment methods: {$a}'; $string['unknownimportmode'] = 'Unknown import mode'; $string['updatemissing'] = 'Fill in missing items from CSV data and defaults'; $string['updatemode'] = 'Update mode'; diff --git a/admin/tool/uploadcourse/tests/course_test.php b/admin/tool/uploadcourse/tests/course_test.php index a173e721811..9d12b80e19c 100644 --- a/admin/tool/uploadcourse/tests/course_test.php +++ b/admin/tool/uploadcourse/tests/course_test.php @@ -900,8 +900,11 @@ class tool_uploadcourse_course_testcase extends advanced_testcase { } public function test_enrolment_data() { + global $DB; $this->resetAfterTest(true); + $this->setAdminUser(); + // Adding an enrolment method to a new course. $mode = tool_uploadcourse_processor::MODE_CREATE_NEW; $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; $data = array('shortname' => 'c1', 'summary' => 'S', 'fullname' => 'FN', 'category' => '1'); @@ -921,11 +924,135 @@ class tool_uploadcourse_course_testcase extends advanced_testcase { $enroldata[$instance->enrol] = $instance; } + $this->assertFalse($co->has_errors()); $this->assertNotEmpty($enroldata['manual']); $this->assertEquals(ENROL_INSTANCE_ENABLED, $enroldata['manual']->status); $this->assertEquals(strtotime($data['enrolment_1_startdate']), $enroldata['manual']->enrolstartdate); $this->assertEquals(strtotime('1970-01-01 GMT + ' . $data['enrolment_1_enrolperiod']), $enroldata['manual']->enrolperiod); $this->assertEquals(strtotime('12th July 2013'), $enroldata['manual']->enrolenddate); + + // Updating a course's enrolment method. + $c2 = $this->getDataGenerator()->create_course(array('shortname' => 'c2')); + $enroldata = array(); + $instances = enrol_get_instances($c2->id, false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + $manual = $enroldata['manual']; + $manual->enrolstartdate = strtotime('1st January 2000'); + $manual->enrolenddate = strtotime('2nd January 2001'); + $manual->status = ENROL_INSTANCE_DISABLED; + $DB->update_record('enrol', $manual); + + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'manual'; + $data['enrolment_1_role'] = 'teacher'; + $data['enrolment_1_enddate'] = '2nd August 2013'; + $data['enrolment_1_enrolperiod'] = '10 days'; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertTrue($co->prepare()); + $co->proceed(); + + // Get the enrolment methods data. + $enroldata = array(); + $instances = enrol_get_instances($co->get_id(), false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + + $this->assertFalse($co->has_errors()); + $this->assertNotEmpty($enroldata['manual']); + $this->assertEquals(ENROL_INSTANCE_ENABLED, $enroldata['manual']->status); + $this->assertEquals($manual->enrolstartdate, $enroldata['manual']->enrolstartdate); + $this->assertEquals(strtotime('1970-01-01 GMT + ' . $data['enrolment_1_enrolperiod']), + $enroldata['manual']->enrolperiod); + $this->assertEquals(strtotime('11th January 2000'), $enroldata['manual']->enrolenddate); + + // Disabling an enrolment method. + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'manual'; + $data['enrolment_1_disable'] = '1'; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertTrue($co->prepare()); + $co->proceed(); + + $enroldata = array(); + $instances = enrol_get_instances($co->get_id(), false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + $this->assertFalse($co->has_errors()); + $this->assertNotEmpty($enroldata['manual']); + $this->assertEquals(ENROL_INSTANCE_DISABLED, $enroldata['manual']->status); + + // Deleting an enrolment method. + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'manual'; + $data['enrolment_1_delete'] = '1'; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertTrue($co->prepare()); + $co->proceed(); + + $enroldata = array(); + $instances = enrol_get_instances($co->get_id(), false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + $this->assertFalse($co->has_errors()); + $this->assertArrayNotHasKey('manual', $enroldata); + + // Trying to add a non-existing enrolment method. + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'notexisting'; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertFalse($co->prepare()); + $this->assertArrayHasKey('unknownenrolmentmethods', $co->get_errors()); + + // Trying to add a non-compatible enrolment method. + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'database'; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertTrue($co->prepare()); + $co->proceed(); + + $enroldata = array(); + $instances = enrol_get_instances($co->get_id(), false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + $this->assertTrue($co->has_errors()); + $this->assertArrayNotHasKey('database', $enroldata); + $this->assertArrayHasKey('cannotaddenrolmentmethods', $co->get_errors()); + + // Testing cohort enrolment method. + $cohort = $this->getDataGenerator()->create_cohort(); + $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY; + $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY; + $data = array('shortname' => 'c2'); + $data['enrolment_1'] = 'cohort'; + $data['enrolment_1_customint1'] = $cohort->id; + $co = new tool_uploadcourse_course($mode, $updatemode, $data); + $this->assertTrue($co->prepare()); + $co->proceed(); + + $enroldata = array(); + $instances = enrol_get_instances($co->get_id(), false); + foreach ($instances as $instance) { + $enroldata[$instance->enrol] = $instance; + } + $this->assertFalse($co->has_errors()); + $this->assertArrayHasKey('cohort', $enroldata); + $this->assertEquals($cohort->id, $enroldata['cohort']->customint1); } public function test_idnumber_problems() { diff --git a/admin/tool/uploadcourse/tests/helper_test.php b/admin/tool/uploadcourse/tests/helper_test.php index 41a838c1753..22569e3a000 100644 --- a/admin/tool/uploadcourse/tests/helper_test.php +++ b/admin/tool/uploadcourse/tests/helper_test.php @@ -101,7 +101,9 @@ class tool_uploadcourse_helper_testcase extends advanced_testcase { 'test1' => 'test1', ) ); - $this->assertSame(tool_uploadcourse_helper::get_enrolment_data($data), $expected); + $errors = array(); + $this->assertSame(tool_uploadcourse_helper::get_enrolment_data($data, $errors), $expected); + $this->assertArrayHasKey('unknownenrolmentmethods', $errors); } public function test_get_enrolment_plugins() {