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 515a1f17a0c..fc38894202e 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -26,6 +26,8 @@ $string['aftersaving...'] = 'Once you have saved your settings, you may wish to' $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 @@ $string['sourcedidfallback_desc'] = 'In IMS data, the field represen 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 56c4d06ae7e..a91f42cc31b 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -39,6 +39,21 @@ require_once($CFG->dirroot.'/group/lib.php'); */ 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { /** * 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { } 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { } $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 8f7c734ee33..a20d92a05f4 100644 --- a/enrol/imsenterprise/settings.php +++ b/enrol/imsenterprise/settings.php @@ -50,6 +50,9 @@ if ($ADMIN->fulltree) { $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 @@ if ($ADMIN->fulltree) { $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 113ef11e081..f407a4d95f4 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -96,6 +96,7 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { } + /** + * 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { 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 f407a4d95f4..baaddaf7d8c 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -174,6 +174,68 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 baaddaf7d8c..4d386569076 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -145,42 +145,29 @@ class enrol_imsenterprise_testcase extends advanced_testcase { 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'; + $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($user3); - $this->set_xml_file($users); + $this->set_xml_file(array($imsuser)); $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); + $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { */ 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 fc38894202e..c87ccd162b6 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -27,7 +27,7 @@ $string['allowunenrol'] = 'Allow the IMS data to unenrol studen $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 @@ $string['sourcedidfallback_desc'] = 'In IMS data, the field represen 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 a91f42cc31b..3dc11650c31 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -310,16 +310,22 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { } } 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { } 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { /** * 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { 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 4d386569076..32616cde61d 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -170,7 +170,7 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { */ 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $xmlcontent .= ' recstatus)) { $xmlcontent .= ' recstatus="'.$user->recstatus.'"'; } @@ -634,12 +667,12 @@ class enrol_imsenterprise_testcase extends advanced_testcase { 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 f0d4869de70..4c9ed7396d6 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -1622,6 +1622,7 @@ class core_course_courselib_testcase extends advanced_testcase { 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 @@ class core_course_courselib_testcase extends advanced_testcase { $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 c87ccd162b6..4cebfe9b4bb 100644 --- a/enrol/imsenterprise/lang/en/enrol_imsenterprise.php +++ b/enrol/imsenterprise/lang/en/enrol_imsenterprise.php @@ -26,8 +26,10 @@ $string['aftersaving...'] = 'Once you have saved your settings, you may wish to' $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['mailadmins'] = 'Notify admin by email'; $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 3dc11650c31..ede27007752 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $this->logfp = fopen($logtolocation, 'a'); } + $this->defaultcategoryid = null; + $fileisnew = false; if ( file_exists($filename) ) { core_php_time_limit::raise(); @@ -118,6 +125,9 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { // 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { /** * 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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { $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 @@ class enrol_imsenterprise_plugin extends enrol_plugin { 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 $defaultcategoryid; + 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 $catid; } /** diff --git a/enrol/imsenterprise/settings.php b/enrol/imsenterprise/settings.php index a20d92a05f4..d55b8ee5499 100644 --- a/enrol/imsenterprise/settings.php +++ b/enrol/imsenterprise/settings.php @@ -98,8 +98,14 @@ if ($ADMIN->fulltree) { 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 32616cde61d..e3ed1128d40 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -242,6 +242,8 @@ class enrol_imsenterprise_testcase extends advanced_testcase { // 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { /** - * 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { /** - * 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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { $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 @@ class enrol_imsenterprise_testcase extends advanced_testcase { // 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 ca746c73fb7..4b95292f858 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';