From 93fdbb42925737a83053200a710d339df4832ae4 Mon Sep 17 00:00:00 2001 From: Brian Biggs Date: Tue, 24 Sep 2013 11:43:37 +0800 Subject: [PATCH 1/5] MDL-31356 enrol_imsenterprise: New Features * Feature 1: Allow nested categories when creating courses. * Feature 2: Allow updates to course Full Name and Short Name. * Feature 3: Allow setting authentication type for users. * Feature 4: Allow updates to users. Also added tests and updated course update routines so it doesn't muck with DB directly. --- .../lang/en/enrol_imsenterprise.php | 6 + enrol/imsenterprise/lib.php | 154 +++++++++--- enrol/imsenterprise/settings.php | 10 + .../tests/imsenterprise_test.php | 233 +++++++++++++++++- 4 files changed, 370 insertions(+), 33 deletions(-) diff --git a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php index 515a1f17a0ca8..fc38894202e76 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -26,6 +26,8 @@ $string['allowunenrol'] = 'Allow the IMS data to unenrol students/teachers'; $string['allowunenrol_desc'] = 'If enabled, course enrolments will be removed when specified in the Enterprise data.'; $string['basicsettings'] = 'Basic settings'; +$string['categoryseparator'] = 'Category Separator Character'; +$string['categoryseparator_desc'] = 'You can create nested categories (if you allow category creation). '; $string['coursesettings'] = 'Course data options'; $string['createnewcategories'] = 'Create new (hidden) course categories if not found in Moodle'; $string['createnewcategories_desc'] = 'If the element is present in a course\'s incoming data, its content will be used to specify a category if the course is to be created from scratch. The plugin will NOT re-categorise existing courses. @@ -75,6 +77,10 @@ Some student information systems fail to output the field. If this is the case, you should enable this setting to allow for using the as the Moodle user ID. Otherwise, leave this setting disabled.'; $string['truncatecoursecodes'] = 'Truncate course codes to this length'; $string['truncatecoursecodes_desc'] = 'In some situations you may have course codes which you wish to truncate to a specified length before processing. If so, enter the number of characters in this box. Otherwise, leave the box blank and no truncation will occur.'; +$string['updatecourses'] = 'Update course full names'; +$string['updatecourses_desc'] = 'If enabled, the IMS Enterprise enrolment plugin can update course full names (if the "recstatus" flag is set to 2, which represents an update).'; +$string['updateusers'] = 'Update user accounts when specified in IMS data'; +$string['updateusers_desc'] = 'If enabled, IMS Enterprise enrolment data can specify changes to user accounts (if the "recstatus" flag is set to 2, which represents an update).'; $string['usecapitafix'] = 'Tick this box if using "Capita" (their XML format is slightly wrong)'; $string['usecapitafix_desc'] = 'The student data system produced by Capita has been found to have one slight error in its XML output. If you are using Capita you should enable this setting - otherwise leave it un-ticked.'; $string['usersettings'] = 'User data options'; diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php index 56c4d06ae7edc..a91f42cc31b55 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -39,6 +39,21 @@ */ class enrol_imsenterprise_plugin extends enrol_plugin { + /** + * @var IMSENTERPRISE_ADD imsenterprise add action. + */ + const IMSENTERPRISE_ADD = 1; + + /** + * @var IMSENTERPRISE_UPDATE imsenterprise update action. + */ + const IMSENTERPRISE_UPDATE = 2; + + /** + * @var IMSENTERPRISE_DELETE imsenterprise delete action. + */ + const IMSENTERPRISE_DELETE = 3; + /** * @var $logfp resource file pointer for writing log data to. */ @@ -276,7 +291,14 @@ protected function process_group_tag($tagcontents) { // Get configs. $truncatecoursecodes = $this->get_config('truncatecoursecodes'); $createnewcourses = $this->get_config('createnewcourses'); + $updatecourses = $this->get_config('updatecourses'); $createnewcategories = $this->get_config('createnewcategories'); + $categoryseparator = trim($this->get_config('categoryseparator')); + + // Ensure a default is set for the category separator. + if (empty($categoryseparator)) { + $categoryseparator = '|'; + } if ($createnewcourses) { require_once("$CFG->dirroot/course/lib.php"); @@ -320,12 +342,13 @@ protected function process_group_tag($tagcontents) { // Third, check if the course(s) exist. foreach ($group->coursecode as $coursecode) { $coursecode = trim($coursecode); - if (!$DB->get_field('course', 'id', array('idnumber' => $coursecode))) { + $dbcourse = $DB->get_field('course', 'id', array('idnumber' => $coursecode)); + if (!$dbcourse) { if (!$createnewcourses) { $this->log_line("Course $coursecode not found in Moodle's course idnumbers."); } else { - // Create the (hidden) course(s) if not found + // Create the (hidden) course(s) if not found. $courseconfig = get_config('moodlecourse'); // Load Moodle Course shell defaults. // New course. @@ -361,27 +384,49 @@ protected function process_group_tag($tagcontents) { // Insert default names for teachers/students, from the current language. // Handle course categorisation (taken from the group.org.orgunit field if present). - if (!empty($group->category)) { - // If the category is defined and exists in Moodle, we want to store it in that one. - if ($catid = $DB->get_field('course_categories', 'id', array('name' => $group->category))) { - $course->category = $catid; - } else if ($createnewcategories) { - // Else if we're allowed to create new categories, let's create this one. - $newcat = new stdClass(); - $newcat->name = $group->category; - $newcat->visible = 0; - $catid = $DB->insert_record('course_categories', $newcat); - $course->category = $catid; - $this->log_line("Created new (hidden) category, #$catid: $newcat->name"); - } else { - // If not found and not allowed to create, stick with default. - $this->log_line('Category '.$group->category.' not found in Moodle database, so using '. - 'default category instead.'); - $course->category = $this->get_default_category_id(); + if (strlen($group->category) > 0) { + $sep = '{\\'.$categoryseparator.'}'; + $matches = preg_split($sep, $group->category, -1, PREG_SPLIT_NO_EMPTY); + + // Categories can be nested. + // For example: "Fall 2013|Biology" is the "Biology" category nested under the "Fall 2013" category. + // Iterate through each category and create it if necessary. + + $catid = 0; + $fullnestedcatname = ''; + foreach ($matches as $catname) { + $catname = trim($catname); + if (strlen($fullnestedcatname)) { + $fullnestedcatname .= ' / '; + } + $fullnestedcatname .= $catname; + $parentid = $catid; + if ($catid = $DB->get_field('course_categories', 'id', + array('name' => $catname, 'parent' => $parentid))) { + $course->category = $catid; + continue; // This category already exists. + } + if ($createnewcategories) { + // Else if we're allowed to create new categories, let's create this one. + $newcat = new stdClass(); + $newcat->name = $catname; + $newcat->visible = 0; + $newcat->parent = $parentid; + $catid = $DB->insert_record('course_categories', $newcat); + $this->log_line("Created new (hidden) category '$fullnestedcatname'"); + $course->category = $catid; + } else { + // If not found and not allowed to create, stick with default. + $this->log_line('Category '.$group->category.' not found in Moodle database, so using '. + 'default category instead.'); + $course->category = $this->get_default_category_id(); + break; + } } } else { $course->category = $this->get_default_category_id(); } + $course->startdate = time(); // Choose a sort order that puts us at the start of the list! $course->sortorder = 0; @@ -390,9 +435,41 @@ protected function process_group_tag($tagcontents) { $this->log_line("Created course $coursecode in Moodle (Moodle ID is $course->id)"); } - } else if ($recstatus == 3 && ($courseid = $DB->get_field('course', 'id', array('idnumber' => $coursecode)))) { + } else if (($recstatus == self::IMSENTERPRISE_UPDATE) && $dbcourse) { + if ($updatecourses) { + // Update course. Allowed fields to be updated are: + // Short Name, and Full Name. + $hasupdates = false; + if (!empty($group->short)) { + if ($group->short != $dbcourse->shortname) { + $dbcourse->shortname = $group->short; + $hasupdates = true; + } + } + if (!empty($group->full)) { + if ($group->full != $dbcourse->fullname) { + $dbcourse->fullname = $group->full; + $hasupdates = true; + } + } + if ($hasupdates) { + $DB->update_record('course', $dbcourse); + $courseid = $dbcourse->id; + add_to_log(SITEID, "course", "update", "view.php?id=$courseid", "ID $courseid"); + $this->log_line("Updated course $coursecode in Moodle (Moodle ID is $courseid)"); + } + } else { + // Update courses option is not enabled. Ignore. + $this->log_line("Ignoring update to course $coursecode"); + } + } else if (($recstatus == self::IMSENTERPRISE_DELETE) && $dbcourse) { // If course does exist, but recstatus==3 (delete), then set the course as hidden. - $DB->set_field('course', 'visible', '0', array('id' => $courseid)); + $courseid = $dbcourse->id; + $dbcourse->visible = 0; + $DB->update_record('course', $dbcourse); + add_to_log(SITEID, "course", "update", "view.php?id=$courseid", + "Updated (set to hidden) course $coursecode (Moodle ID is $courseid)"); + $this->log_line("Updated (set to hidden) course $coursecode in Moodle (Moodle ID is $courseid)"); } } } @@ -400,8 +477,7 @@ protected function process_group_tag($tagcontents) { /** * Process the person tag. This defines a Moodle user. - * - * @param string $tagcontents The raw contents of the XML element + * @param string $tagconents The raw contents of the XML element */ protected function process_person_tag($tagcontents) { global $CFG, $DB; @@ -412,6 +488,7 @@ protected function process_person_tag($tagcontents) { $fixcasepersonalnames = $this->get_config('fixcasepersonalnames'); $imsdeleteusers = $this->get_config('imsdeleteusers'); $createnewusers = $this->get_config('createnewusers'); + $imsupdateusers = $this->get_config('imsupdateusers'); $person = new stdClass(); if (preg_match('{.*?(.+?).*?}is', $tagcontents, $matches)) { @@ -423,11 +500,14 @@ protected function process_person_tag($tagcontents) { if (preg_match('{.*?.*?(.+?).*?.*?}is', $tagcontents, $matches)) { $person->lastname = trim($matches[1]); } - if (preg_match('{(.*?)}is', $tagcontents, $matches)) { + if (preg_match('{(.*?)}is', $tagcontents, $matches)) { $person->username = trim($matches[1]); } + if (preg_match('{.*?}is', $tagcontents, $matches)) { + $person->auth = trim($matches[1]); + } if ($imssourcedidfallback && trim($person->username) == '') { - // This is the point where we can fall back to useing the "sourcedid" if "userid" is not supplied + // This is the point where we can fall back to useing the "sourcedid" if "userid" is not supplied. // NB We don't use an "elseif" because the tag may be supplied-but-empty. $person->username = $person->idnumber; } @@ -460,7 +540,7 @@ protected function process_person_tag($tagcontents) { $recstatus = ($this->get_recstatus($tagcontents, 'person')); // Now if the recstatus is 3, we should delete the user if-and-only-if the setting for delete users is turned on. - if ($recstatus == 3) { + if ($recstatus == self::IMSENTERPRISE_DELETE) { if ($imsdeleteusers) { // If we're allowed to delete user records. // Do not dare to hack the user.deleted field directly in database!!! @@ -477,6 +557,18 @@ protected function process_person_tag($tagcontents) { } else { $this->log_line("Ignoring deletion request for user '$person->username' (ID number $person->idnumber)."); } + } else if ($recstatus == self::IMSENTERPRISE_UPDATE) { // Update user. + if ($imsupdateusers) { + if ($id = $DB->get_field('user', 'id', array('idnumber' => $person->idnumber))) { + $person->id = $id; + $DB->update_record('user', $person); + $this->log_line("Updated user $person->username"); + } else { + $this->log_line("Ignoring update request for non-existent user $person->username"); + } + } else { + $this->log_line("Ignoring update request for user $person->username"); + } } else { // Add or update record. @@ -494,9 +586,11 @@ protected function process_person_tag($tagcontents) { // If they don't exist and they have a defined username, and $createnewusers == true, we create them. $person->lang = $CFG->lang; // TODO: MDL-15863 this needs more work due to multiauth changes, use first auth for now. - $auth = explode(',', $CFG->auth); - $auth = reset($auth); - $person->auth = $auth; + if (empty($person->auth)) { + $auth = explode(',', $CFG->auth); + $auth = reset($auth); + $person->auth = $auth; + } $person->confirmed = 1; $person->timemodified = time(); $person->mnethostid = $CFG->mnet_localhost_id; @@ -568,7 +662,7 @@ protected function process_membership_tag($tagcontents) { } $recstatus = ($this->get_recstatus($mmatch[1], 'role')); - if ($recstatus == 3) { + if ($recstatus == self::IMSENTERPRISE_DELETE) { // See above - recstatus of 3 (==delete) is treated the same as status of 0. $member->status = 0; } diff --git a/enrol/imsenterprise/settings.php b/enrol/imsenterprise/settings.php index 8f7c734ee3383..a20d92a05f46b 100644 --- a/enrol/imsenterprise/settings.php +++ b/enrol/imsenterprise/settings.php @@ -50,6 +50,9 @@ $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/createnewusers', get_string('createnewusers', 'enrol_imsenterprise'), get_string('createnewusers_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/imsupdateusers', + get_string('updateusers', 'enrol_imsenterprise'), get_string('updateusers_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/imsdeleteusers', get_string('deleteusers', 'enrol_imsenterprise'), get_string('deleteusers_desc', 'enrol_imsenterprise'), 0)); @@ -88,10 +91,17 @@ $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/createnewcourses', get_string('createnewcourses', 'enrol_imsenterprise'), get_string('createnewcourses_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/updatecourses', + get_string('updatecourses', 'enrol_imsenterprise'), get_string('updatecourses_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/createnewcategories', get_string('createnewcategories', 'enrol_imsenterprise'), get_string('createnewcategories_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configtext('enrol_imsenterprise/categoryseparator', + get_string('categoryseparator', 'enrol_imsenterprise'), get_string('categoryseparator_desc', 'enrol_imsenterprise'), '|', + PARAM_TEXT, 3)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/imsunenrol', get_string('allowunenrol', 'enrol_imsenterprise'), get_string('allowunenrol_desc', 'enrol_imsenterprise'), 0)); diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index 113ef11e08167..f407a4d95f4cb 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -96,6 +96,7 @@ public function test_users_add() { $prevnusers = $DB->count_records('user'); $user1 = new StdClass(); + $user1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $user1->username = 'u1'; $user1->email = 'u1@example.com'; $user1->firstname = 'U'; @@ -108,6 +109,72 @@ public function test_users_add() { $this->assertEquals(($prevnusers + 1), $DB->count_records('user')); } + /** + * Add new users and set an auth type + */ + public function test_users_add_with_auth() { + global $DB; + + $prevnusers = $DB->count_records('user'); + + $user2 = new StdClass(); + $user2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $user2->username = 'u2'; + $user2->auth = 'cas'; + $user2->email = 'u2@u2.org'; + $user2->firstname = 'U'; + $user2->lastname = '2'; + + $users = array($user2); + $this->set_xml_file($users); + $this->imsplugin->cron(); + + $dbuser = $DB->get_record('user', array('username' => $user2->username)); + // TODO: MDL-15863 this needs more work due to multiauth changes, use first auth for now. + $dbauth = explode(',', $dbuser->auth); + $dbauth = reset($dbauth); + + $this->assertEquals(($prevnusers + 1), $DB->count_records('user')); + $this->assertEquals($dbauth, $user2->auth); + } + + + /** + * Update user + */ + public function test_user_update() { + global $DB; + + $user3 = new StdClass(); + $user3->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $user3->username = 'u3'; + $user3->email = 'u3@u3.org'; + $user3->firstname = 'U'; + $user3->lastname = '3'; + + $users = array($user3); + $this->set_xml_file($users); + $this->imsplugin->cron(); + + $user3u = $DB->get_record('user', array('username' => $user3->username)); + + $user3u->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; + $user3u->email = 'updated_u3@updated_u3.org'; + $user3u->firstname = 'updated_U'; + $user3u->lastname = 'updated_3'; + + $users = array($user3u); + $this->set_xml_file($users); + $this->imsplugin->cron(); + + $dbuser = $DB->get_record('user', array('username' => $user3->username)); + + $this->assertEquals($dbuser->email, $user3u->email); + $this->assertEquals($dbuser->firstname, $user3u->firstname); + $this->assertEquals($dbuser->lastname, $user3u->lastname); + } + + /** * Existing courses are not created again */ @@ -139,11 +206,13 @@ public function test_courses_add() { $prevncourses = $DB->count_records('course'); $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = 'id1'; $course1->imsshort = 'id1'; $course1->category = 'DEFAULT CATNAME'; $course2 = new StdClass(); + $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course2->idnumber = 'id2'; $course2->imsshort = 'id2'; $course2->category = 'DEFAULT CATNAME'; @@ -194,6 +263,7 @@ public function test_courses_attrmapping() { $this->imsplugin->set_config('imscoursemapsummary', 'coursecode'); $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = 'id1'; $course1->imsshort = 'description_short1'; $course1->imslong = 'description_long'; @@ -215,6 +285,7 @@ public function test_courses_attrmapping() { $this->imsplugin->set_config('imscoursemapsummary', 'full'); $course2 = new StdClass(); + $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course2->idnumber = 'id2'; $course2->imsshort = 'description_short2'; $course2->imslong = 'description_long'; @@ -236,6 +307,7 @@ public function test_courses_attrmapping() { $this->imsplugin->set_config('imscoursemapsummary', 'full'); $course3 = new StdClass(); + $course3->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course3->idnumber = 'id3'; $course3->imsshort = 'description_short3'; $course3->category = 'DEFAULT CATNAME'; @@ -251,6 +323,138 @@ public function test_courses_attrmapping() { } + /** + * Course updates + */ + public function test_course_update() { + global $DB; + + $course4 = new StdClass(); + $course4->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course4->idnumber = 'id4'; + $course4->imsshort = 'id4'; + $course4->imsfull = 'id4'; + $course4->category = 'DEFAULT CATNAME'; + + $this->set_xml_file(false, array($course4)); + $this->imsplugin->cron(); + + $course4u = $DB->get_record('course', array('idnumber' => $course4->idnumber)); + + $course4u->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; + $course4u->imsshort = 'description_short_updated'; + $course4u->imsfull = 'description_full_updated'; + + $this->set_xml_file(false, array($course4u)); + $this->imsplugin->cron(); + + $dbcourse = $DB->get_record('course', array('idnumber' => $course4->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->shortname, $course4u->imsshort); + $this->assertEquals($dbcourse->fullname, $course4u->imsfull); + } + + + /** + * Nested categories during course creation + */ + public function test_nested_categories() { + global $DB; + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcat = 'DEFAULT CATNAME'; + $subcat = 'DEFAULT SUB CATNAME'; + + $fullcat = $topcat.$catsep.$subcat; + + $course5 = new StdClass(); + $course5->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course5->idnumber = 'id5'; + $course5->imsshort = 'description_short'; + $course5->imslong = 'description_long'; + $course5->imsfull = 'description_full'; + $course5->category = $fullcat; + + $this->set_xml_file(false, array($course5)); + $this->imsplugin->cron(); + + $parentcatid = $DB->get_field('course_categories', 'id', array('name' => $topcat)); + $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat,'parent' => $parentcatid)); + + $this->assertTrue(isset($subcatid)); + $this->assertTrue($subcatid > 0); + + # Change the category separator character + $this->imsplugin->set_config('categoryseparator', ':'); + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcat = 'DEFAULT CATNAME'; + $subcat = 'DEFAULT SUB CATNAME TEST2'; + + $fullcat = $topcat.$catsep.$subcat; + + $course6 = new StdClass(); + $course6->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course6->idnumber = 'id6'; + $course6->imsshort = 'description_short'; + $course6->imslong = 'description_long'; + $course6->imsfull = 'description_full'; + $course6->category = $fullcat; + + $this->set_xml_file(false, array($course6)); + $this->imsplugin->cron(); + + $parentcatid = $DB->get_field('course_categories', 'id', array('name' => $topcat)); + $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat,'parent' => $parentcatid)); + + $this->assertTrue(isset($subcatid)); + $this->assertTrue($subcatid > 0); + } + + + /** + * Test that duplicate nested categories are not created + */ + public function test_nested_categories_for_dups() { + global $DB; + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcat = 'DEFAULT CATNAME'; + $subcat = 'DEFAULT SUB CATNAME DUPTEST'; + + $fullcat = $topcat.$catsep.$subcat; + + $course7 = new StdClass(); + $course7->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course7->idnumber = 'id7'; + $course7->imsshort = 'description_short'; + $course7->imslong = 'description_long'; + $course7->imsfull = 'description_full'; + $course7->category = $fullcat; + + $this->set_xml_file(false, array($course7)); + $this->imsplugin->cron(); + + $prevncategories = $DB->count_records('course_categories'); + + $course8 = new StdClass(); + $course8->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course8->idnumber = 'id8'; + $course8->imsshort = 'description_short'; + $course8->imslong = 'description_long'; + $course8->imsfull = 'description_full'; + $course8->category = $fullcat; + + $this->set_xml_file(false, array($course8)); + $this->imsplugin->cron(); + + $this->assertEquals($prevncategories, $DB->count_records('course_categories')); + } + + /** * Sets the plugin configuration for testing */ @@ -258,7 +462,9 @@ public function set_test_config() { $this->imsplugin->set_config('mailadmins', false); $this->imsplugin->set_config('prev_path', ''); $this->imsplugin->set_config('createnewusers', true); + $this->imsplugin->set_config('imsupdateusers', true); $this->imsplugin->set_config('createnewcourses', true); + $this->imsplugin->set_config('updatecourses', true); $this->imsplugin->set_config('createnewcategories', true); } @@ -276,12 +482,26 @@ public function set_xml_file($users = false, $courses = false) { if (!empty($users)) { foreach ($users as $user) { $xmlcontent .= ' - + recstatus)) { + $xmlcontent .= ' recstatus="'.$user->recstatus.'"'; + } + + $xmlcontent .= '> TestSource '.$user->username.' - '.$user->username.' + auth)) { + $xmlcontent .= ' authenticationtype="'.$user->auth.'"'; + } + + $xmlcontent .= '>'.$user->username.' '.$user->firstname.' '.$user->lastname.' @@ -300,7 +520,14 @@ public function set_xml_file($users = false, $courses = false) { foreach ($courses as $course) { $xmlcontent .= ' - + recstatus)) { + $xmlcontent .= ' recstatus="'.$course->recstatus.'"'; + } + + $xmlcontent .= '> TestSource '.$course->idnumber.' From ecf6a0ab5be0abded8a7815594ec042bf4a36de9 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Fri, 27 Sep 2013 11:44:35 +0800 Subject: [PATCH 2/5] MDL-31356 enrol_imsenterprise: Fix behaviour with course idnumber When not set we shouldn't throw a warning --- .../tests/imsenterprise_test.php | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index f407a4d95f4cb..baaddaf7d8c6a 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -174,6 +174,68 @@ public function test_user_update() { $this->assertEquals($dbuser->lastname, $user3u->lastname); } + /** + * Update user disabled + */ + public function test_user_update_disabled() { + global $DB; + + $this->imsplugin->set_config('imsupdateusers', false); + $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); + $imsuser ->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; + // THIS SHOULD WORK, surely?: $imsuser->username = $user->username; + // But this is required... + $imsuser->username = $user->idnumber; + $imsuser->email = 'u3@u3.org'; + $imsuser->firstname = 'U'; + $imsuser->lastname = '3'; + $this->set_xml_file(array($imsuser)); + $this->imsplugin->cron(); + // Verify no changes have been made. + $dbuser = $DB->get_record('user', array('id' => $user->id), '*', MUST_EXIST); + $this->assertEquals($user->email, $dbuser->email); + $this->assertEquals($user->firstname, $dbuser->firstname); + $this->assertEquals($user->lastname, $dbuser->lastname); + } + + /** + * Delete user + */ + public function test_user_delete() { + global $DB; + + $this->imsplugin->set_config('imsdeleteusers', true); + $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); + $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; + $imsuser->username = $user->username; + $imsuser->firstname = $user->firstname; + $imsuser->lastname = $user->lastname; + $imsuser->email = $user->email; + $this->set_xml_file(array($imsuser)); + $this->imsplugin->cron(); + $this->assertEquals(1, $DB->get_field('user', 'deleted', array('id' => $user->id), '*', MUST_EXIST)); + } + + /** + * Delete user disabled + */ + public function test_user_delete_disabled() { + global $DB; + $this->imsplugin->set_config('imsdeleteusers', false); + $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); + $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; + $imsuser->username = $user->username; + $imsuser->firstname = $user->firstname; + $imsuser->lastname = $user->lastname; + $imsuser->email = $user->email; + $this->set_xml_file(array($imsuser)); + $this->imsplugin->cron(); + $this->assertEquals(0, $DB->get_field('user', 'deleted', array('id' => $user->id), '*', MUST_EXIST)); + } + /** * Existing courses are not created again @@ -222,6 +284,81 @@ public function test_courses_add() { $this->imsplugin->cron(); $this->assertEquals(($prevncourses + 2), $DB->count_records('course')); + $this->assertTrue($DB->record_exists('course', array('idnumber' => $course1->idnumber))); + $this->assertTrue($DB->record_exists('course', array('idnumber' => $course2->idnumber))); + } + + /** + * Verify that courses are not created when createnewcourses + * option is diabled. + */ + public function test_courses_add_createnewcourses_disabled() { + global $DB; + + $this->imsplugin->set_config('createnewcourses', false); + $prevncourses = $DB->count_records('course'); + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = 'id1'; + $course1->imsshort = 'id1'; + $course1->category = 'DEFAULT CATNAME'; + $course2 = new StdClass(); + $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course2->idnumber = 'id2'; + $course2->imsshort = 'id2'; + $course2->category = 'DEFAULT CATNAME'; + $courses = array($course1, $course2); + $this->set_xml_file(false, $courses); + $this->imsplugin->cron(); + // Verify the courses have not ben creased. + $this->assertEquals($prevncourses , $DB->count_records('course')); + $this->assertFalse($DB->record_exists('course', array('idnumber' => $course1->idnumber))); + $this->assertFalse($DB->record_exists('course', array('idnumber' => $course2->idnumber))); + } + + /** + * Test adding a course with no idnumber. + */ + public function test_courses_no_idnumber() { + global $DB; + $prevncourses = $DB->count_records('course'); + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = ''; + $course1->imsshort = 'id1'; + $course1->category = 'DEFAULT CATNAME'; + $this->set_xml_file(false, array($course1)); + $this->imsplugin->cron(); + // Verify no action. + $this->assertEquals($prevncourses, $DB->count_records('course')); + } + + /** + * Add new course with the truncateidnumber setting. + */ + public function test_courses_add_truncate_idnumber() { + global $DB; + + $truncatelength = 4; + + $this->imsplugin->set_config('truncatecoursecodes', $truncatelength); + $prevncourses = $DB->count_records('course'); + + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = '123456789'; + $course1->imsshort = 'id1'; + $course1->category = 'DEFAULT CATNAME'; + + $this->set_xml_file(false, array($course1)); + $this->imsplugin->cron(); + + // Verify the new course has been added. + $this->assertEquals(($prevncourses + 1), $DB->count_records('course')); + + $truncatedidnumber = substr($course1->idnumber, 0, $truncatelength); + + $this->assertTrue($DB->record_exists('course', array('idnumber' => $truncatedidnumber))); } /** From 81e849bc71d9d3bf4494269429a580482fa27e52 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Fri, 27 Sep 2013 14:41:18 +0800 Subject: [PATCH 3/5] MDL-31356 enrol_imsenterprise: add additional test coverage --- .../tests/imsenterprise_test.php | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index baaddaf7d8c6a..4d386569076d2 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -145,42 +145,29 @@ public function test_users_add_with_auth() { public function test_user_update() { global $DB; - $user3 = new StdClass(); - $user3->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; - $user3->username = 'u3'; - $user3->email = 'u3@u3.org'; - $user3->firstname = 'U'; - $user3->lastname = '3'; - - $users = array($user3); - $this->set_xml_file($users); - $this->imsplugin->cron(); - - $user3u = $DB->get_record('user', array('username' => $user3->username)); - - $user3u->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; - $user3u->email = 'updated_u3@updated_u3.org'; - $user3u->firstname = 'updated_U'; - $user3u->lastname = 'updated_3'; + $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); + $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; + // THIS SHOULD WORK, surely?: $imsuser->username = $user->username; + // But this is required... + $imsuser->username = $user->idnumber; + $imsuser->email = 'u3@u3.org'; + $imsuser->firstname = 'U'; + $imsuser->lastname = '3'; - $users = array($user3u); - $this->set_xml_file($users); + $this->set_xml_file(array($imsuser)); $this->imsplugin->cron(); - - $dbuser = $DB->get_record('user', array('username' => $user3->username)); - - $this->assertEquals($dbuser->email, $user3u->email); - $this->assertEquals($dbuser->firstname, $user3u->firstname); - $this->assertEquals($dbuser->lastname, $user3u->lastname); + $dbuser = $DB->get_record('user', array('id' => $user->id), '*', MUST_EXIST); + $this->assertEquals($imsuser->email, $dbuser->email); + $this->assertEquals($imsuser->firstname, $dbuser->firstname); + $this->assertEquals($imsuser->lastname, $dbuser->lastname); } - /** - * Update user disabled - */ public function test_user_update_disabled() { global $DB; $this->imsplugin->set_config('imsupdateusers', false); + $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); $imsuser = new stdClass(); $imsuser ->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; @@ -190,8 +177,10 @@ public function test_user_update_disabled() { $imsuser->email = 'u3@u3.org'; $imsuser->firstname = 'U'; $imsuser->lastname = '3'; + $this->set_xml_file(array($imsuser)); $this->imsplugin->cron(); + // Verify no changes have been made. $dbuser = $DB->get_record('user', array('id' => $user->id), '*', MUST_EXIST); $this->assertEquals($user->email, $dbuser->email); @@ -207,6 +196,7 @@ public function test_user_delete() { $this->imsplugin->set_config('imsdeleteusers', true); $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; $imsuser->username = $user->username; @@ -214,6 +204,7 @@ public function test_user_delete() { $imsuser->lastname = $user->lastname; $imsuser->email = $user->email; $this->set_xml_file(array($imsuser)); + $this->imsplugin->cron(); $this->assertEquals(1, $DB->get_field('user', 'deleted', array('id' => $user->id), '*', MUST_EXIST)); } @@ -223,8 +214,10 @@ public function test_user_delete() { */ public function test_user_delete_disabled() { global $DB; + $this->imsplugin->set_config('imsdeleteusers', false); $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); + $imsuser = new stdClass(); $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; $imsuser->username = $user->username; @@ -232,11 +225,11 @@ public function test_user_delete_disabled() { $imsuser->lastname = $user->lastname; $imsuser->email = $user->email; $this->set_xml_file(array($imsuser)); + $this->imsplugin->cron(); $this->assertEquals(0, $DB->get_field('user', 'deleted', array('id' => $user->id), '*', MUST_EXIST)); } - /** * Existing courses are not created again */ @@ -297,19 +290,27 @@ public function test_courses_add_createnewcourses_disabled() { $this->imsplugin->set_config('createnewcourses', false); $prevncourses = $DB->count_records('course'); + $course1 = new StdClass(); $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = 'id1'; $course1->imsshort = 'id1'; $course1->category = 'DEFAULT CATNAME'; + $course2 = new StdClass(); $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course2->idnumber = 'id2'; $course2->imsshort = 'id2'; $course2->category = 'DEFAULT CATNAME'; + $courses = array($course1, $course2); $this->set_xml_file(false, $courses); $this->imsplugin->cron(); + + $courses = array($course1, $course2); + $this->set_xml_file(false, $courses); + $this->imsplugin->cron(); + // Verify the courses have not ben creased. $this->assertEquals($prevncourses , $DB->count_records('course')); $this->assertFalse($DB->record_exists('course', array('idnumber' => $course1->idnumber))); From 08b29336b9052bf6f6f04bc7133ce79d9fecc605 Mon Sep 17 00:00:00 2001 From: Serge Gauthier Date: Wed, 1 Jun 2016 14:10:47 -0400 Subject: [PATCH 4/5] MDL-31356 enrol_imsenterprise: merge with moodle3.2 --- .../lang/en/enrol_imsenterprise.php | 6 +- enrol/imsenterprise/lib.php | 49 ++++++++++++++--- .../tests/imsenterprise_test.php | 55 +++++++++++++++---- 3 files changed, 87 insertions(+), 23 deletions(-) diff --git a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php index fc38894202e76..c87ccd162b604 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -27,7 +27,7 @@ $string['allowunenrol_desc'] = 'If enabled, course enrolments will be removed when specified in the Enterprise data.'; $string['basicsettings'] = 'Basic settings'; $string['categoryseparator'] = 'Category Separator Character'; -$string['categoryseparator_desc'] = 'You can create nested categories (if you allow category creation). '; +$string['categoryseparator_desc'] = 'You can create nested categories (if you allow category creation).'; $string['coursesettings'] = 'Course data options'; $string['createnewcategories'] = 'Create new (hidden) course categories if not found in Moodle'; $string['createnewcategories_desc'] = 'If the element is present in a course\'s incoming data, its content will be used to specify a category if the course is to be created from scratch. The plugin will NOT re-categorise existing courses. @@ -77,8 +77,8 @@ Some student information systems fail to output the field. If this is the case, you should enable this setting to allow for using the as the Moodle user ID. Otherwise, leave this setting disabled.'; $string['truncatecoursecodes'] = 'Truncate course codes to this length'; $string['truncatecoursecodes_desc'] = 'In some situations you may have course codes which you wish to truncate to a specified length before processing. If so, enter the number of characters in this box. Otherwise, leave the box blank and no truncation will occur.'; -$string['updatecourses'] = 'Update course full names'; -$string['updatecourses_desc'] = 'If enabled, the IMS Enterprise enrolment plugin can update course full names (if the "recstatus" flag is set to 2, which represents an update).'; +$string['updatecourses'] = 'Update course'; +$string['updatecourses_desc'] = 'If enabled, the IMS Enterprise enrolment plugin can update course full and short names (if the "recstatus" flag is set to 2, which represents an update).'; $string['updateusers'] = 'Update user accounts when specified in IMS data'; $string['updateusers_desc'] = 'If enabled, IMS Enterprise enrolment data can specify changes to user accounts (if the "recstatus" flag is set to 2, which represents an update).'; $string['usecapitafix'] = 'Tick this box if using "Capita" (their XML format is slightly wrong)'; diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php index a91f42cc31b55..3dc11650c3136 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -310,16 +310,22 @@ protected function process_group_tag($tagcontents) { $group->coursecode = trim($matches[1]); } + $matches = array(); if (preg_match('{.*?(.*?).*?}is', $tagcontents, $matches)) { $group->long = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?(.*?).*?}is', $tagcontents, $matches)) { $group->short = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?(.*?).*?}is', $tagcontents, $matches)) { $group->full = trim($matches[1]); } + $matches = array(); if (preg_match('{.*?(.*?).*?}is', $tagcontents, $matches)) { $group->category = trim($matches[1]); } @@ -342,7 +348,7 @@ protected function process_group_tag($tagcontents) { // Third, check if the course(s) exist. foreach ($group->coursecode as $coursecode) { $coursecode = trim($coursecode); - $dbcourse = $DB->get_field('course', 'id', array('idnumber' => $coursecode)); + $dbcourse = $DB->get_record('course', array('idnumber' => $coursecode)); if (!$dbcourse) { if (!$createnewcourses) { $this->log_line("Course $coursecode not found in Moodle's course idnumbers."); @@ -453,9 +459,8 @@ protected function process_group_tag($tagcontents) { } } if ($hasupdates) { - $DB->update_record('course', $dbcourse); + update_course($dbcourse); $courseid = $dbcourse->id; - add_to_log(SITEID, "course", "update", "view.php?id=$courseid", "ID $courseid"); $this->log_line("Updated course $coursecode in Moodle (Moodle ID is $courseid)"); } } else { @@ -465,10 +470,8 @@ protected function process_group_tag($tagcontents) { } else if (($recstatus == self::IMSENTERPRISE_DELETE) && $dbcourse) { // If course does exist, but recstatus==3 (delete), then set the course as hidden. $courseid = $dbcourse->id; - $dbcourse->visible = 0; - $DB->update_record('course', $dbcourse); - add_to_log(SITEID, "course", "update", "view.php?id=$courseid", - "Updated (set to hidden) course $coursecode (Moodle ID is $courseid)"); + $show = false; + course_change_visibility($courseid, $show); $this->log_line("Updated (set to hidden) course $coursecode in Moodle (Moodle ID is $courseid)"); } } @@ -477,7 +480,8 @@ protected function process_group_tag($tagcontents) { /** * Process the person tag. This defines a Moodle user. - * @param string $tagconents The raw contents of the XML element + * + * @param string $tagcontents The raw contents of the XML element */ protected function process_person_tag($tagcontents) { global $CFG, $DB; @@ -494,32 +498,49 @@ protected function process_person_tag($tagcontents) { if (preg_match('{.*?(.+?).*?}is', $tagcontents, $matches)) { $person->idnumber = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?.*?(.+?).*?.*?}is', $tagcontents, $matches)) { $person->firstname = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?.*?(.+?).*?.*?}is', $tagcontents, $matches)) { $person->lastname = trim($matches[1]); } + + $matches = array(); if (preg_match('{(.*?)}is', $tagcontents, $matches)) { $person->username = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?}is', $tagcontents, $matches)) { $person->auth = trim($matches[1]); } + if ($imssourcedidfallback && trim($person->username) == '') { // This is the point where we can fall back to useing the "sourcedid" if "userid" is not supplied. // NB We don't use an "elseif" because the tag may be supplied-but-empty. $person->username = $person->idnumber; } + + $matches = array(); if (preg_match('{(.*?)}is', $tagcontents, $matches)) { $person->email = trim($matches[1]); } + + $matches = array(); if (preg_match('{(.*?)}is', $tagcontents, $matches)) { $person->url = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?(.+?).*?}is', $tagcontents, $matches)) { $person->city = trim($matches[1]); } + + $matches = array(); if (preg_match('{.*?(.+?).*?}is', $tagcontents, $matches)) { $person->country = trim($matches[1]); } @@ -570,7 +591,7 @@ protected function process_person_tag($tagcontents) { $this->log_line("Ignoring update request for user $person->username"); } - } else { // Add or update record. + } else { // Add record. // If the user exists (matching sourcedid) then we don't need to do anything. if (!$DB->get_field('user', 'id', array('idnumber' => $person->idnumber)) && $createnewusers) { @@ -644,9 +665,12 @@ protected function process_membership_tag($tagcontents) { foreach ($membermatches as $mmatch) { $member = new stdClass(); $memberstoreobj = new stdClass(); + $matches = array(); if (preg_match('{.*?(.+?).*?}is', $mmatch[1], $matches)) { $member->idnumber = trim($matches[1]); } + + $matches = array(); if (preg_match('{}is', $mmatch[1], $matches)) { // 01 means Student, 02 means Instructor, 3 means ContentDeveloper, and there are more besides. $member->roletype = trim($matches[1]); @@ -656,6 +680,8 @@ protected function process_membership_tag($tagcontents) { // and there are more besides. $member->roletype = trim($matches[1]); } + + $matches = array(); if (preg_match('{(.+?).*?}is', $mmatch[1], $matches)) { // 1 means active, 0 means inactive - treat this as enrol vs unenrol. $member->status = trim($matches[1]); @@ -670,9 +696,12 @@ protected function process_membership_tag($tagcontents) { $timeframe = new stdClass(); $timeframe->begin = 0; $timeframe->end = 0; + $matches = array(); if (preg_match('{(.+?).*?}is', $mmatch[1], $matches)) { $timeframe = $this->decode_timeframe($matches[1]); } + + $matches = array(); if (preg_match('{.*?(.+?).*?.*?}is', $mmatch[1], $matches)) { $member->groupname = trim($matches[1]); @@ -816,6 +845,8 @@ protected static function decode_timeframe($string) { if (preg_match('{(\d\d\d\d)-(\d\d)-(\d\d)}is', $string, $matches)) { $ret->begin = mktime(0, 0, 0, $matches[2], $matches[3], $matches[1]); } + + $matches = array(); if (preg_match('{(\d\d\d\d)-(\d\d)-(\d\d)}is', $string, $matches)) { $ret->end = mktime(0, 0, 0, $matches[2], $matches[3], $matches[1]); } diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index 4d386569076d2..32616cde61d94 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -170,7 +170,7 @@ public function test_user_update_disabled() { $user = $this->getDataGenerator()->create_user(array('idnumber' => 'test-update-user')); $imsuser = new stdClass(); - $imsuser ->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; + $imsuser->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; // THIS SHOULD WORK, surely?: $imsuser->username = $user->username; // But this is required... $imsuser->username = $user->idnumber; @@ -322,14 +322,18 @@ public function test_courses_add_createnewcourses_disabled() { */ public function test_courses_no_idnumber() { global $DB; + $prevncourses = $DB->count_records('course'); + $course1 = new StdClass(); $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = ''; $course1->imsshort = 'id1'; $course1->category = 'DEFAULT CATNAME'; + $this->set_xml_file(false, array($course1)); $this->imsplugin->cron(); + // Verify no action. $this->assertEquals($prevncourses, $DB->count_records('course')); } @@ -492,6 +496,35 @@ public function test_course_update() { $this->assertEquals($dbcourse->fullname, $course4u->imsfull); } + /** + * Course delete. Make it hidden. + */ + public function test_course_delete() { + global $DB; + + $course8 = new StdClass(); + $course8->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course8->idnumber = 'id8'; + $course8->imsshort = 'id8'; + $course8->imsfull = 'id8'; + $course8->category = 'DEFAULT CATNAME'; + + $this->set_xml_file(false, array($course8)); + $this->imsplugin->cron(); + + $course8d = $DB->get_record('course', array('idnumber' => $course8->idnumber)); + $this->assertEquals($course8d->visible, 1); + + $course8d->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; + + $this->set_xml_file(false, array($course8d)); + $this->imsplugin->cron(); + + $dbcourse = $DB->get_record('course', array('idnumber' => $course8d->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->visible, 0); + } + /** * Nested categories during course creation @@ -518,12 +551,12 @@ public function test_nested_categories() { $this->imsplugin->cron(); $parentcatid = $DB->get_field('course_categories', 'id', array('name' => $topcat)); - $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat,'parent' => $parentcatid)); + $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat, 'parent' => $parentcatid)); $this->assertTrue(isset($subcatid)); $this->assertTrue($subcatid > 0); - # Change the category separator character + // Change the category separator character. $this->imsplugin->set_config('categoryseparator', ':'); $catsep = trim($this->imsplugin->get_config('categoryseparator')); @@ -545,7 +578,7 @@ public function test_nested_categories() { $this->imsplugin->cron(); $parentcatid = $DB->get_field('course_categories', 'id', array('name' => $topcat)); - $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat,'parent' => $parentcatid)); + $subcatid = $DB->get_field('course_categories', 'id', array('name' => $subcat, 'parent' => $parentcatid)); $this->assertTrue(isset($subcatid)); $this->assertTrue($subcatid > 0); @@ -622,7 +655,7 @@ public function set_xml_file($users = false, $courses = false) { $xmlcontent .= ' recstatus)) { $xmlcontent .= ' recstatus="'.$user->recstatus.'"'; } @@ -634,12 +667,12 @@ public function set_xml_file($users = false, $courses = false) { auth)) { - $xmlcontent .= ' authenticationtype="'.$user->auth.'"'; - } + // Optional authentication type. + if (!empty($user->auth)) { + $xmlcontent .= ' authenticationtype="'.$user->auth.'"'; + } - $xmlcontent .= '>'.$user->username.' + $xmlcontent .= '>'.$user->username.' '.$user->firstname.' '.$user->lastname.' @@ -660,7 +693,7 @@ public function set_xml_file($users = false, $courses = false) { $xmlcontent .= ' recstatus)) { $xmlcontent .= ' recstatus="'.$course->recstatus.'"'; } From cc6b92483a8fc04ed86c1655c51c3e4505816385 Mon Sep 17 00:00:00 2001 From: Serge Gauthier Date: Thu, 7 Jul 2016 14:32:23 -0400 Subject: [PATCH 5/5] MDL-31356 enrol_imsenterprise: Continue development for added features * Add the possibility to make it works as before (with categrory name or idnumber). * Add possibility to specify nested categories by name or idnumber. * Fix error in core_course_courselib_testcase::test_course_created_event because dependency with imsenterprise --- course/tests/courselib_test.php | 9 +- .../lang/en/enrol_imsenterprise.php | 6 +- enrol/imsenterprise/lib.php | 176 +++++--- enrol/imsenterprise/settings.php | 8 +- .../tests/imsenterprise_test.php | 406 ++++++++++++++++-- enrol/imsenterprise/version.php | 2 +- 6 files changed, 508 insertions(+), 99 deletions(-) diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index f0d4869de70a8..4c9ed7396d6fa 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1622,6 +1622,7 @@ public function test_course_created_event() { ob_end_clean(); // Create the XML file we want to use. + $course->category = (array)$course->category; $imstestcase = new enrol_imsenterprise_testcase(); $imstestcase->imsplugin = enrol_get_plugin('imsenterprise'); $imstestcase->set_test_config(); @@ -1632,7 +1633,13 @@ public function test_course_created_event() { $imstestcase->imsplugin->cron(); $events = $sink->get_events(); $sink->close(); - $event = $events[0]; + $event = null; + foreach ($events as $eventinfo) { + if ($eventinfo instanceof \core\event\course_created ) { + $event = $eventinfo; + break; + } + } // Validate the event triggered is \core\event\course_created. There is no need to validate the other values // as they have already been validated in the previous steps. Here we only want to make sure that when the diff --git a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php index c87ccd162b604..4cebfe9b4bb5b 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -26,8 +26,10 @@ $string['allowunenrol'] = 'Allow the IMS data to unenrol students/teachers'; $string['allowunenrol_desc'] = 'If enabled, course enrolments will be removed when specified in the Enterprise data.'; $string['basicsettings'] = 'Basic settings'; +$string['categoryidnumber'] = 'Allow category idnumber'; +$string['categoryidnumber_desc'] = 'If enabled IMS Enterprise will create category with idnumber'; $string['categoryseparator'] = 'Category Separator Character'; -$string['categoryseparator_desc'] = 'You can create nested categories (if you allow category creation).'; +$string['categoryseparator_desc'] = 'Required when "Category idnumber" is enabled. Character to separate the category name and idnumber.'; $string['coursesettings'] = 'Course data options'; $string['createnewcategories'] = 'Create new (hidden) course categories if not found in Moodle'; $string['createnewcategories_desc'] = 'If the element is present in a course\'s incoming data, its content will be used to specify a category if the course is to be created from scratch. The plugin will NOT re-categorise existing courses. @@ -57,6 +59,8 @@ $string['mailusers'] = 'Notify users by email'; $string['messageprovider:imsenterprise_enrolment'] = 'IMS Enterprise enrolment messages'; $string['miscsettings'] = 'Miscellaneous'; +$string['nestedcategories'] = 'Allow nested categories'; +$string['nestedcategories_desc'] = 'If enabled IMS Enterprise will create nested categories'; $string['pluginname'] = 'IMS Enterprise file'; $string['pluginname_desc'] = 'This method will repeatedly check for and process a specially-formatted text file in the location that you specify. The file must follow the IMS Enterprise specifications containing person, group, and membership XML elements.'; $string['processphoto'] = 'Add user photo data to profile'; diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php index 3dc11650c3136..ede270077522e 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot.'/group/lib.php'); - +require_once($CFG->dirroot.'/lib/coursecatlib.php'); /** * IMS Enterprise file enrolment plugin. @@ -79,6 +79,11 @@ class enrol_imsenterprise_plugin extends enrol_plugin { */ protected $rolemappings; + /** + * @var $defaultcategoryid id of default category. + */ + protected $defaultcategoryid; + /** * Read in an IMS Enterprise file. * Originally designed to handle v1.1 files but should be able to handle @@ -108,6 +113,8 @@ public function cron() { $this->logfp = fopen($logtolocation, 'a'); } + $this->defaultcategoryid = null; + $fileisnew = false; if ( file_exists($filename) ) { core_php_time_limit::raise(); @@ -118,6 +125,9 @@ public function cron() { $this->log_line('Found file '.$filename); $this->xmlcache = ''; + $categoryseparator = trim($this->get_config('categoryseparator')); + $categoryidnumber = $this->get_config('categoryidnumber'); + // Make sure we understand how to map the IMS-E roles to Moodle roles. $this->load_role_mappings(); // Make sure we understand how to map the IMS-E course names to Moodle course names. @@ -128,7 +138,9 @@ public function cron() { // Decide if we want to process the file (based on filepath, modification time, and MD5 hash) // This is so we avoid wasting the server's efforts processing a file unnecessarily. - if (empty($prevpath) || ($filename != $prevpath)) { + if ($categoryidnumber && empty($categoryseparator)) { + $this->log_line('Category idnumber is enabled but category separator is not defined - skipping processing.'); + } else if (empty($prevpath) || ($filename != $prevpath)) { $fileisnew = true; } else if (isset($prevtime) && ($filemtime <= $prevtime)) { $this->log_line('File modification time is not more recent than last update - skipping processing.'); @@ -292,13 +304,6 @@ protected function process_group_tag($tagcontents) { $truncatecoursecodes = $this->get_config('truncatecoursecodes'); $createnewcourses = $this->get_config('createnewcourses'); $updatecourses = $this->get_config('updatecourses'); - $createnewcategories = $this->get_config('createnewcategories'); - $categoryseparator = trim($this->get_config('categoryseparator')); - - // Ensure a default is set for the category separator. - if (empty($categoryseparator)) { - $categoryseparator = '|'; - } if ($createnewcourses) { require_once("$CFG->dirroot/course/lib.php"); @@ -325,9 +330,10 @@ protected function process_group_tag($tagcontents) { $group->full = trim($matches[1]); } - $matches = array(); - if (preg_match('{.*?(.*?).*?}is', $tagcontents, $matches)) { - $group->category = trim($matches[1]); + if (preg_match('{(.*?)}is', $tagcontents, $matchesorg)) { + if (preg_match_all('{(.*?)}is', $matchesorg[1], $matchesorgunit)) { + $group->categories = array_map('trim', $matchesorgunit[1]); + } } $recstatus = ($this->get_recstatus($tagcontents, 'group')); @@ -389,49 +395,8 @@ protected function process_group_tag($tagcontents) { $course->enablecompletion = $courseconfig->enablecompletion; // Insert default names for teachers/students, from the current language. - // Handle course categorisation (taken from the group.org.orgunit field if present). - if (strlen($group->category) > 0) { - $sep = '{\\'.$categoryseparator.'}'; - $matches = preg_split($sep, $group->category, -1, PREG_SPLIT_NO_EMPTY); - - // Categories can be nested. - // For example: "Fall 2013|Biology" is the "Biology" category nested under the "Fall 2013" category. - // Iterate through each category and create it if necessary. - - $catid = 0; - $fullnestedcatname = ''; - foreach ($matches as $catname) { - $catname = trim($catname); - if (strlen($fullnestedcatname)) { - $fullnestedcatname .= ' / '; - } - $fullnestedcatname .= $catname; - $parentid = $catid; - if ($catid = $DB->get_field('course_categories', 'id', - array('name' => $catname, 'parent' => $parentid))) { - $course->category = $catid; - continue; // This category already exists. - } - if ($createnewcategories) { - // Else if we're allowed to create new categories, let's create this one. - $newcat = new stdClass(); - $newcat->name = $catname; - $newcat->visible = 0; - $newcat->parent = $parentid; - $catid = $DB->insert_record('course_categories', $newcat); - $this->log_line("Created new (hidden) category '$fullnestedcatname'"); - $course->category = $catid; - } else { - // If not found and not allowed to create, stick with default. - $this->log_line('Category '.$group->category.' not found in Moodle database, so using '. - 'default category instead.'); - $course->category = $this->get_default_category_id(); - break; - } - } - } else { - $course->category = $this->get_default_category_id(); - } + // Handle course categorisation (taken from the group.org.orgunit or group.org.id fields if present). + $course->category = $this->get_category_from_group($group->categories); $course->startdate = time(); // Choose a sort order that puts us at the start of the list! @@ -480,7 +445,7 @@ protected function process_group_tag($tagcontents) { /** * Process the person tag. This defines a Moodle user. - * + * * @param string $tagcontents The raw contents of the XML element */ protected function process_person_tag($tagcontents) { @@ -591,7 +556,7 @@ protected function process_person_tag($tagcontents) { $this->log_line("Ignoring update request for user $person->username"); } - } else { // Add record. + } else { // Add or update record. // If the user exists (matching sourcedid) then we don't need to do anything. if (!$DB->get_field('user', 'id', array('idnumber' => $person->idnumber)) && $createnewusers) { @@ -909,14 +874,101 @@ private function get_default_category_id() { global $CFG; require_once($CFG->libdir.'/coursecatlib.php'); - static $defaultcategoryid = null; - - if ($defaultcategoryid === null) { + if ($this->defaultcategoryid === null) { $category = coursecat::get_default(); - $defaultcategoryid = $category->id; + $this->defaultcategoryid = $category->id; + } + + return $this->defaultcategoryid; + } + + /** + * Find the category using idnumber or name. + * + * @param array $categories List of categories + * + * @return int id of category found. + */ + private function get_category_from_group($categories) { + global $DB; + + if (empty($categories)) { + $catid = $this->get_default_category_id(); + } else { + $createnewcategories = $this->get_config('createnewcategories'); + $categoryseparator = trim($this->get_config('categoryseparator')); + $nestedcategories = trim($this->get_config('nestedcategories')); + $searchbyidnumber = trim($this->get_config('categoryidnumber')); + + if (!empty($categoryseparator)) { + $sep = '{\\'.$categoryseparator.'}'; + } + + $catid = 0; + $fullnestedcatname = ''; + + foreach ($categories as $categoryinfo) { + if ($searchbyidnumber) { + $values = preg_split($sep, $categoryinfo, -1, PREG_SPLIT_NO_EMPTY); + if (count($values) < 2) { + $this->log_line('Category ' . $categoryinfo . ' missing name or idnumber. Using default category instead.'); + $catid = $this->get_default_category_id(); + break; + } + $categoryname = $values[0]; + $categoryidnumber = $values[1]; + } else { + $categoryname = $categoryinfo; + $categoryidnumber = null; + if (empty($categoryname)) { + $this->log_line('Category ' . $categoryinfo . ' missing name. Using default category instead.'); + $catid = $this->get_default_category_id(); + break; + } + } + + if (!empty($fullnestedcatname)) { + $fullnestedcatname .= ' / '; + } + + $fullnestedcatname .= $categoryname; + $parentid = $catid; + + // Check if category exist. + $params = array(); + if ($searchbyidnumber) { + $params['idnumber'] = $categoryidnumber; + } else { + $params['name'] = $categoryname; + } + if ($nestedcategories) { + $params['parent'] = $parentid; + } + + if ($catid = $DB->get_field('course_categories', 'id', $params)) { + continue; // This category already exists. + } + + // If we're allowed to create new categories, let's create this one. + if ($createnewcategories) { + $newcat = new stdClass(); + $newcat->name = $categoryname; + $newcat->visible = 0; + $newcat->parent = $parentid; + $newcat->idnumber = $categoryidnumber; + $newcat = coursecat::create($newcat); + $catid = $newcat->id; + $this->log_line("Created new (hidden) category '$fullnestedcatname'"); + } else { + // If not found and not allowed to create, stick with default. + $this->log_line('Category ' . $categoryinfo . ' not found in Moodle database. Using default category instead.'); + $catid = $this->get_default_category_id(); + break; + } + } } - return $defaultcategoryid; + return $catid; } /** diff --git a/enrol/imsenterprise/settings.php b/enrol/imsenterprise/settings.php index a20d92a05f46b..d55b8ee5499f5 100644 --- a/enrol/imsenterprise/settings.php +++ b/enrol/imsenterprise/settings.php @@ -98,8 +98,14 @@ get_string('createnewcategories', 'enrol_imsenterprise'), get_string('createnewcategories_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/nestedcategories', + get_string('nestedcategories', 'enrol_imsenterprise'), get_string('nestedcategories_desc', 'enrol_imsenterprise'), 0)); + + $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/categoryidnumber', + get_string('categoryidnumber', 'enrol_imsenterprise'), get_string('categoryidnumber_desc', 'enrol_imsenterprise'), 0)); + $settings->add(new admin_setting_configtext('enrol_imsenterprise/categoryseparator', - get_string('categoryseparator', 'enrol_imsenterprise'), get_string('categoryseparator_desc', 'enrol_imsenterprise'), '|', + get_string('categoryseparator', 'enrol_imsenterprise'), get_string('categoryseparator_desc', 'enrol_imsenterprise'), '', PARAM_TEXT, 3)); $settings->add(new admin_setting_configcheckbox('enrol_imsenterprise/imsunenrol', diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index 32616cde61d94..e3ed1128d4099 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -242,6 +242,8 @@ public function test_courses_existing() { // Default mapping according to default course attributes - IMS description tags mapping. $course1->imsshort = $course1->fullname; $course2->imsshort = $course2->fullname; + unset($course1->category); + unset($course2->category); $prevncourses = $DB->count_records('course'); @@ -264,13 +266,13 @@ public function test_courses_add() { $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = 'id1'; $course1->imsshort = 'id1'; - $course1->category = 'DEFAULT CATNAME'; + $course1->category[] = 'DEFAULT CATNAME'; $course2 = new StdClass(); $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course2->idnumber = 'id2'; $course2->imsshort = 'id2'; - $course2->category = 'DEFAULT CATNAME'; + $course2->category[] = 'DEFAULT CATNAME'; $courses = array($course1, $course2); $this->set_xml_file(false, $courses); @@ -295,13 +297,13 @@ public function test_courses_add_createnewcourses_disabled() { $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = 'id1'; $course1->imsshort = 'id1'; - $course1->category = 'DEFAULT CATNAME'; + $course1->category[] = 'DEFAULT CATNAME'; $course2 = new StdClass(); $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course2->idnumber = 'id2'; $course2->imsshort = 'id2'; - $course2->category = 'DEFAULT CATNAME'; + $course2->category[] = 'DEFAULT CATNAME'; $courses = array($course1, $course2); $this->set_xml_file(false, $courses); @@ -329,7 +331,7 @@ public function test_courses_no_idnumber() { $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = ''; $course1->imsshort = 'id1'; - $course1->category = 'DEFAULT CATNAME'; + $course1->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course1)); $this->imsplugin->cron(); @@ -353,7 +355,7 @@ public function test_courses_add_truncate_idnumber() { $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course1->idnumber = '123456789'; $course1->imsshort = 'id1'; - $course1->category = 'DEFAULT CATNAME'; + $course1->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course1)); $this->imsplugin->cron(); @@ -383,7 +385,7 @@ public function test_course_add_default_category() { $course1 = new stdClass(); $course1->idnumber = 'id1'; $course1->imsshort = 'id1'; - $course1->category = ''; + $course1->category[] = ''; $this->set_xml_file(false, array($course1)); $this->imsplugin->cron(); @@ -410,7 +412,7 @@ public function test_courses_attrmapping() { $course1->imsshort = 'description_short1'; $course1->imslong = 'description_long'; $course1->imsfull = 'description_full'; - $course1->category = 'DEFAULT CATNAME'; + $course1->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course1)); $this->imsplugin->cron(); @@ -432,7 +434,7 @@ public function test_courses_attrmapping() { $course2->imsshort = 'description_short2'; $course2->imslong = 'description_long'; $course2->imsfull = 'description_full'; - $course2->category = 'DEFAULT CATNAME'; + $course2->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course2)); $this->imsplugin->cron(); @@ -452,7 +454,7 @@ public function test_courses_attrmapping() { $course3->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course3->idnumber = 'id3'; $course3->imsshort = 'description_short3'; - $course3->category = 'DEFAULT CATNAME'; + $course3->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course3)); $this->imsplugin->cron(); @@ -476,7 +478,7 @@ public function test_course_update() { $course4->idnumber = 'id4'; $course4->imsshort = 'id4'; $course4->imsfull = 'id4'; - $course4->category = 'DEFAULT CATNAME'; + $course4->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course4)); $this->imsplugin->cron(); @@ -486,6 +488,7 @@ public function test_course_update() { $course4u->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_UPDATE; $course4u->imsshort = 'description_short_updated'; $course4u->imsfull = 'description_full_updated'; + unset($course4u->category); $this->set_xml_file(false, array($course4u)); $this->imsplugin->cron(); @@ -507,7 +510,7 @@ public function test_course_delete() { $course8->idnumber = 'id8'; $course8->imsshort = 'id8'; $course8->imsfull = 'id8'; - $course8->category = 'DEFAULT CATNAME'; + $course8->category[] = 'DEFAULT CATNAME'; $this->set_xml_file(false, array($course8)); $this->imsplugin->cron(); @@ -516,6 +519,7 @@ public function test_course_delete() { $this->assertEquals($course8d->visible, 1); $course8d->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_DELETE; + unset($course8d->category); $this->set_xml_file(false, array($course8d)); $this->imsplugin->cron(); @@ -527,25 +531,25 @@ public function test_course_delete() { /** - * Nested categories during course creation + * Nested categories with name during course creation */ public function test_nested_categories() { global $DB; - $catsep = trim($this->imsplugin->get_config('categoryseparator')); + $this->imsplugin->set_config('nestedcategories', true); $topcat = 'DEFAULT CATNAME'; $subcat = 'DEFAULT SUB CATNAME'; - $fullcat = $topcat.$catsep.$subcat; - $course5 = new StdClass(); $course5->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course5->idnumber = 'id5'; $course5->imsshort = 'description_short'; $course5->imslong = 'description_long'; $course5->imsfull = 'description_full'; - $course5->category = $fullcat; + $course5->category = array(); + $course5->category[] = $topcat; + $course5->category[] = $subcat; $this->set_xml_file(false, array($course5)); $this->imsplugin->cron(); @@ -556,23 +560,18 @@ public function test_nested_categories() { $this->assertTrue(isset($subcatid)); $this->assertTrue($subcatid > 0); - // Change the category separator character. - $this->imsplugin->set_config('categoryseparator', ':'); - - $catsep = trim($this->imsplugin->get_config('categoryseparator')); - $topcat = 'DEFAULT CATNAME'; $subcat = 'DEFAULT SUB CATNAME TEST2'; - $fullcat = $topcat.$catsep.$subcat; - $course6 = new StdClass(); $course6->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course6->idnumber = 'id6'; $course6->imsshort = 'description_short'; $course6->imslong = 'description_long'; $course6->imsfull = 'description_full'; - $course6->category = $fullcat; + $course6->category = array(); + $course6->category[] = $topcat; + $course6->category[] = $subcat; $this->set_xml_file(false, array($course6)); $this->imsplugin->cron(); @@ -586,25 +585,24 @@ public function test_nested_categories() { /** - * Test that duplicate nested categories are not created + * Test that duplicate nested categories with name are not created */ public function test_nested_categories_for_dups() { global $DB; - $catsep = trim($this->imsplugin->get_config('categoryseparator')); + $this->imsplugin->set_config('nestedcategories', true); $topcat = 'DEFAULT CATNAME'; $subcat = 'DEFAULT SUB CATNAME DUPTEST'; - $fullcat = $topcat.$catsep.$subcat; - $course7 = new StdClass(); $course7->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; $course7->idnumber = 'id7'; $course7->imsshort = 'description_short'; $course7->imslong = 'description_long'; $course7->imsfull = 'description_full'; - $course7->category = $fullcat; + $course7->category[] = $topcat; + $course7->category[] = $subcat; $this->set_xml_file(false, array($course7)); $this->imsplugin->cron(); @@ -617,7 +615,8 @@ public function test_nested_categories_for_dups() { $course8->imsshort = 'description_short'; $course8->imslong = 'description_long'; $course8->imsfull = 'description_full'; - $course8->category = $fullcat; + $course8->category[] = $topcat; + $course8->category[] = $subcat; $this->set_xml_file(false, array($course8)); $this->imsplugin->cron(); @@ -625,6 +624,336 @@ public function test_nested_categories_for_dups() { $this->assertEquals($prevncategories, $DB->count_records('course_categories')); } + /** + * Nested categories with idnumber during course creation + */ + public function test_nested_categories_idnumber() { + global $DB; + + $this->imsplugin->set_config('nestedcategories', true); + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcatname = 'DEFAULT CATNAME'; + $subcatname = 'DEFAULT SUB CATNAME'; + $topcatidnumber = '01'; + $subcatidnumber = '0101'; + + $topcat = $topcatname.$catsep.$topcatidnumber; + $subcat = $subcatname.$catsep.$subcatidnumber; + + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = 'id5'; + $course1->imsshort = 'description_short'; + $course1->imslong = 'description_long'; + $course1->imsfull = 'description_full'; + $course1->category[] = $topcat; + $course1->category[] = $subcat; + + $this->set_xml_file(false, array($course1)); + $this->imsplugin->cron(); + + $parentcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $topcatidnumber)); + $subcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $subcatidnumber, 'parent' => $parentcatid)); + + $this->assertTrue(isset($subcatid)); + $this->assertTrue($subcatid > 0); + + // Change the category separator character. + $this->imsplugin->set_config('categoryseparator', ':'); + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcatname = 'DEFAULT CATNAME'; + $subcatname = 'DEFAULT SUB CATNAME TEST2'; + $topcatidnumber = '01'; + $subcatidnumber = '0102'; + + $topcat = $topcatname.$catsep.$topcatidnumber; + $subcat = $subcatname.$catsep.$subcatidnumber; + + $course2 = new StdClass(); + $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course2->idnumber = 'id6'; + $course2->imsshort = 'description_short'; + $course2->imslong = 'description_long'; + $course2->imsfull = 'description_full'; + $course2->category[] = $topcat; + $course2->category[] = $subcat; + + $this->set_xml_file(false, array($course2)); + $this->imsplugin->cron(); + + $parentcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $topcatidnumber)); + $subcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $subcatidnumber, 'parent' => $parentcatid)); + + $this->assertTrue(isset($subcatid)); + $this->assertTrue($subcatid > 0); + } + + /** + * Test that duplicate nested categories with idnumber are not created + */ + public function test_nested_categories_idnumber_for_dups() { + global $DB; + + $this->imsplugin->set_config('nestedcategories', true); + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcatname = 'DEFAULT CATNAME'; + $subcatname = 'DEFAULT SUB CATNAME'; + $topcatidnumber = '01'; + $subcatidnumber = '0101'; + + $topcat = $topcatname.$catsep.$topcatidnumber; + $subcat = $subcatname.$catsep.$subcatidnumber; + + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = 'id1'; + $course1->imsshort = 'description_short'; + $course1->imslong = 'description_long'; + $course1->imsfull = 'description_full'; + $course1->category[] = $topcat; + $course1->category[] = $subcat; + + $this->set_xml_file(false, array($course1)); + $this->imsplugin->cron(); + + $prevncategories = $DB->count_records('course_categories'); + + $course2 = new StdClass(); + $course2->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course2->idnumber = 'id2'; + $course2->imsshort = 'description_short'; + $course2->imslong = 'description_long'; + $course2->imsfull = 'description_full'; + $course2->category[] = $topcat; + $course2->category[] = $subcat; + + $this->set_xml_file(false, array($course2)); + $this->imsplugin->cron(); + + $this->assertEquals($prevncategories, $DB->count_records('course_categories')); + } + + /** + * Test that nested categories with idnumber is not created if name is missing + */ + public function test_categories_idnumber_missing_name() { + global $DB, $CFG; + + $this->imsplugin->set_config('nestedcategories', true); + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcatname = 'DEFAULT CATNAME'; + $subcatname = ''; + $topcatidnumber = '01'; + $subcatidnumber = '0101'; + + $topcat = $topcatname.$catsep.$topcatidnumber; + $subcat = $subcatname.$catsep.$subcatidnumber; + + $course1 = new StdClass(); + $course1->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course1->idnumber = 'id1'; + $course1->imsshort = 'description_short'; + $course1->imslong = 'description_long'; + $course1->imsfull = 'description_full'; + $course1->category[] = $topcat; + $course1->category[] = $subcat; + + $this->set_xml_file(false, array($course1)); + $this->imsplugin->cron(); + + // Check all categories except the last subcategory was created. + $parentcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $topcatidnumber)); + $this->assertTrue((boolean)$parentcatid); + $subcatid = $DB->get_field('course_categories', 'id', array('idnumber' => $subcatidnumber, 'parent' => $parentcatid)); + $this->assertFalse((boolean)$subcatid); + + // Check course was put in default category. + $defaultcat = coursecat::get_default(); + $dbcourse = $DB->get_record('course', array('idnumber' => $course1->idnumber), '*', MUST_EXIST); + $this->assertEquals($dbcourse->category, $defaultcat->id); + + } + + /** + * Create category with name (nested categories not activated). + */ + public function test_create_category_name_no_nested() { + global $DB; + + $course = new StdClass(); + $course->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course->idnumber = 'id'; + $course->imsshort = 'description_short'; + $course->imslong = 'description_long'; + $course->imsfull = 'description_full'; + $course->category[] = 'CATNAME'; + + $this->set_xml_file(false, array($course)); + $this->imsplugin->cron(); + + $dbcat = $DB->get_record('course_categories', array('name' => $course->category[0])); + $this->assertFalse(!$dbcat); + $this->assertEquals($dbcat->parent, 0); + + $dbcourse = $DB->get_record('course', array('idnumber' => $course->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->category, $dbcat->id); + + } + + /** + * Find a category with name (nested categories not activated). + */ + public function test_find_category_name_no_nested() { + global $DB; + + $cattop = $this->getDataGenerator()->create_category(array('name' => 'CAT-TOP')); + $catsub = $this->getDataGenerator()->create_category(array('name' => 'CAT-SUB', 'parent' => $cattop->id)); + $prevcats = $DB->count_records('course_categories'); + + $course = new StdClass(); + $course->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course->idnumber = 'id'; + $course->imsshort = 'description_short'; + $course->imslong = 'description_long'; + $course->imsfull = 'description_full'; + $course->category[] = 'CAT-SUB'; + + $this->set_xml_file(false, array($course)); + $this->imsplugin->cron(); + + $newcats = $DB->count_records('course_categories'); + + // Check that no new category was not created. + $this->assertEquals($prevcats, $newcats); + + // Check course is associated to CAT-SUB. + $dbcourse = $DB->get_record('course', array('idnumber' => $course->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->category, $catsub->id); + + } + + /** + * Create category with idnumber (nested categories not activated). + */ + public function test_create_category_idnumber_no_nested() { + global $DB; + + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $course = new StdClass(); + $course->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course->idnumber = 'id'; + $course->imsshort = 'description_short'; + $course->imslong = 'description_long'; + $course->imsfull = 'description_full'; + $course->category[] = 'CATNAME'. $catsep . 'CATIDNUMBER'; + + $this->set_xml_file(false, array($course)); + $this->imsplugin->cron(); + + $dbcat = $DB->get_record('course_categories', array('idnumber' => 'CATIDNUMBER')); + $this->assertFalse(!$dbcat); + $this->assertEquals($dbcat->parent, 0); + $this->assertEquals($dbcat->name, 'CATNAME'); + + $dbcourse = $DB->get_record('course', array('idnumber' => $course->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->category, $dbcat->id); + + } + + /** + * Find a category with idnumber (nested categories not activated). + */ + public function test_find_category_idnumber_no_nested() { + global $DB; + + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $topcatname = 'CAT-TOP'; + $subcatname = 'CAT-SUB'; + $topcatidnumber = 'ID-TOP'; + $subcatidnumber = 'ID-SUB'; + + $cattop = $this->getDataGenerator()->create_category(array('name' => $topcatname, 'idnumber' => $topcatidnumber)); + $catsub = $this->getDataGenerator()->create_category(array('name' => $subcatname, 'idnumber' => $subcatidnumber, + 'parent' => $cattop->id)); + $prevcats = $DB->count_records('course_categories'); + + $course = new StdClass(); + $course->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course->idnumber = 'id'; + $course->imsshort = 'description_short'; + $course->imslong = 'description_long'; + $course->imsfull = 'description_full'; + $course->category[] = $subcatname . $catsep . $subcatidnumber; + + $this->set_xml_file(false, array($course)); + $this->imsplugin->cron(); + + $newcats = $DB->count_records('course_categories'); + + // Check that no new category was not created. + $this->assertEquals($prevcats, $newcats); + + $dbcourse = $DB->get_record('course', array('idnumber' => $course->idnumber)); + $this->assertFalse(!$dbcourse); + $this->assertEquals($dbcourse->category, $catsub->id); + + } + + /** + * Test that category with idnumber is not created if name is missing (nested categories not activated). + */ + public function test_category_idnumber_missing_name_no_nested() { + global $DB; + + $this->imsplugin->set_config('categoryidnumber', true); + $this->imsplugin->set_config('categoryseparator', '|'); + $catsep = trim($this->imsplugin->get_config('categoryseparator')); + + $catidnumber = '01'; + + $course = new StdClass(); + $course->recstatus = enrol_imsenterprise_plugin::IMSENTERPRISE_ADD; + $course->idnumber = 'id1'; + $course->imsshort = 'description_short'; + $course->imslong = 'description_long'; + $course->imsfull = 'description_full'; + $course->category[] = '' . $catsep . $catidnumber; + + $this->set_xml_file(false, array($course)); + $this->imsplugin->cron(); + + // Check category was not created. + $catid = $DB->get_record('course_categories', array('idnumber' => $catidnumber)); + $this->assertFalse($catid); + + // Check course was put in default category. + $defaultcat = coursecat::get_default(); + $dbcourse = $DB->get_record('course', array('idnumber' => $course->idnumber), '*', MUST_EXIST); + $this->assertEquals($dbcourse->category, $defaultcat->id); + + } /** * Sets the plugin configuration for testing @@ -637,6 +966,9 @@ public function set_test_config() { $this->imsplugin->set_config('createnewcourses', true); $this->imsplugin->set_config('updatecourses', true); $this->imsplugin->set_config('createnewcategories', true); + $this->imsplugin->set_config('categoryseparator', ''); + $this->imsplugin->set_config('categoryidnumber', false); + $this->imsplugin->set_config('nestedcategories', false); } /** @@ -726,8 +1058,16 @@ public function set_xml_file($users = false, $courses = false) { // The orgunit tag value is used by moodle as category name. $xmlcontent .= ' - - '.$course->category.' + '; + // Optional category name. + if (isset($course->category) && !empty($course->category)) { + foreach ($course->category as $category) { + $xmlcontent .= ' + '.$category.''; + } + } + + $xmlcontent .= ' '; } diff --git a/enrol/imsenterprise/version.php b/enrol/imsenterprise/version.php index ca746c73fb7bc..4b95292f858a5 100644 --- a/enrol/imsenterprise/version.php +++ b/enrol/imsenterprise/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016052300; // The current plugin version (Date: YYYYMMDDXX) +$plugin->version = 2016070600; // The current plugin version (Date: YYYYMMDDXX) $plugin->requires = 2016051900; // Requires this Moodle version. $plugin->component = 'enrol_imsenterprise';