From 50ac4392f356887286c1657240be5794f8452fdb Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 13 Jan 2014 16:16:22 -0800 Subject: [PATCH 1/4] MDL-40909 mod_forum: removed duplicated test (duplicated in enrol/meta/tests/plugin_test.php) --- mod/forum/tests/lib_test.php | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index dd724ddab42..3d03583b0e6 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -169,39 +169,6 @@ class mod_forum_lib_testcase extends advanced_testcase { } } - /** - * Test user_enrolment_deleted observer. - */ - public function test_user_enrolment_deleted_observer() { - global $DB; - - $this->resetAfterTest(); - - $metaplugin = enrol_get_plugin('meta'); - $user1 = $this->getDataGenerator()->create_user(); - $course1 = $this->getDataGenerator()->create_course(); - $course2 = $this->getDataGenerator()->create_course(); - $student = $DB->get_record('role', array('shortname' => 'student')); - - $e1 = $metaplugin->add_instance($course2, array('customint1' => $course1->id)); - $enrol1 = $DB->get_record('enrol', array('id' => $e1)); - - // Enrol user. - $metaplugin->enrol_user($enrol1, $user1->id, $student->id); - $this->assertEquals(1, $DB->count_records('user_enrolments')); - - // Unenrol user and capture event. - $sink = $this->redirectEvents(); - $metaplugin->unenrol_user($enrol1, $user1->id); - $events = $sink->get_events(); - $sink->close(); - $event = array_pop($events); - - $this->assertEquals(0, $DB->count_records('user_enrolments')); - $this->assertInstanceOf('\core\event\user_enrolment_deleted', $event); - $this->assertEquals('user_unenrolled', $event->get_legacy_eventname()); - } - /** * Test the logic in the forum_tp_can_track_forums() function. */ From 6324517209f805ab4da99effdb51a6c9e5a1075a Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 13 Jan 2014 15:46:34 -0800 Subject: [PATCH 2/4] MDL-40909 core_enrol: removed 'unenrol' add_to_log calls Before each of these calls a call to unenrol_user() is made, which triggers the event 'user_enrolment_deleted'. Nothing more is needed. --- enrol/manual/manage.php | 1 - enrol/manual/unenrolself.php | 2 +- enrol/paypal/unenrolself.php | 2 +- enrol/self/unenrolself.php | 2 +- enrol/tests/enrollib_test.php | 41 ++++++++++++++++++++ lib/classes/event/user_enrolment_deleted.php | 8 ++++ 6 files changed, 52 insertions(+), 4 deletions(-) diff --git a/enrol/manual/manage.php b/enrol/manual/manage.php index 7d91e4b6017..64b542fbba1 100644 --- a/enrol/manual/manage.php +++ b/enrol/manual/manage.php @@ -139,7 +139,6 @@ if ($canunenrol && optional_param('remove', false, PARAM_BOOL) && confirm_sesske if (!empty($userstounassign)) { foreach($userstounassign as $removeuser) { $enrol_manual->unenrol_user($instance, $removeuser->id); - add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //there should be userid somewhere! } $potentialuserselector->invalidate_selected_users(); diff --git a/enrol/manual/unenrolself.php b/enrol/manual/unenrolself.php index c7304fb5f58..9dd3bc517f6 100644 --- a/enrol/manual/unenrolself.php +++ b/enrol/manual/unenrolself.php @@ -49,7 +49,7 @@ $PAGE->set_title($plugin->get_instance_name($instance)); if ($confirm and confirm_sesskey()) { $plugin->unenrol_user($instance, $USER->id); - add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //TODO: there should be userid somewhere! + redirect(new moodle_url('/index.php')); } diff --git a/enrol/paypal/unenrolself.php b/enrol/paypal/unenrolself.php index 8fd16d264c2..61155c8af86 100644 --- a/enrol/paypal/unenrolself.php +++ b/enrol/paypal/unenrolself.php @@ -49,7 +49,7 @@ $PAGE->set_title($plugin->get_instance_name($instance)); if ($confirm and confirm_sesskey()) { $plugin->unenrol_user($instance, $USER->id); - add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //there should be userid somewhere! + redirect(new moodle_url('/index.php')); } diff --git a/enrol/self/unenrolself.php b/enrol/self/unenrolself.php index 7e7d269042b..2da62ee2ffe 100644 --- a/enrol/self/unenrolself.php +++ b/enrol/self/unenrolself.php @@ -49,7 +49,7 @@ $PAGE->set_title($plugin->get_instance_name($instance)); if ($confirm and confirm_sesskey()) { $plugin->unenrol_user($instance, $USER->id); - add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //TODO: there should be userid somewhere! + redirect(new moodle_url('/index.php')); } diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php index fcf9cad560c..af2960e8b03 100644 --- a/enrol/tests/enrollib_test.php +++ b/enrol/tests/enrollib_test.php @@ -316,4 +316,45 @@ class core_enrollib_testcase extends advanced_testcase { $expectedlegacyeventdata->courseid = $course1->id; $this->assertEventLegacyData($expectedlegacyeventdata, $event); } + + /** + * Test user_enrolment_deleted event. + */ + public function test_user_enrolment_deleted_event() { + global $DB; + + $this->resetAfterTest(true); + + $manualplugin = enrol_get_plugin('manual'); + $user = $this->getDataGenerator()->create_user(); + $course = $this->getDataGenerator()->create_course(); + $student = $DB->get_record('role', array('shortname' => 'student')); + + $enrol = $DB->get_record('enrol', array('courseid' => $course->id, 'enrol' => 'manual'), '*', MUST_EXIST); + + // Enrol user. + $manualplugin->enrol_user($enrol, $user->id, $student->id); + + // Get the user enrolment information, used to validate legacy event data. + $dbuserenrolled = $DB->get_record('user_enrolments', array('userid' => $user->id)); + + // Unenrol user and capture event. + $sink = $this->redirectEvents(); + $manualplugin->unenrol_user($enrol, $user->id); + $events = $sink->get_events(); + $sink->close(); + $event = array_pop($events); + + // Validate the event. + $this->assertInstanceOf('\core\event\user_enrolment_deleted', $event); + $this->assertEquals(context_course::instance($course->id), $event->get_context()); + $this->assertEquals('user_unenrolled', $event->get_legacy_eventname()); + $expectedlegacyeventdata = $dbuserenrolled; + $expectedlegacyeventdata->enrol = $manualplugin->get_name(); + $expectedlegacyeventdata->courseid = $course->id; + $expectedlegacyeventdata->lastenrol = true; + $this->assertEventLegacyData($expectedlegacyeventdata, $event); + $expected = array($course->id, 'course', 'unenrol', '../enrol/users.php?id=' . $course->id, $course->id); + $this->assertEventLegacyLogData($expected, $event); + } } diff --git a/lib/classes/event/user_enrolment_deleted.php b/lib/classes/event/user_enrolment_deleted.php index e178aab8c1d..4c7a905b1ef 100644 --- a/lib/classes/event/user_enrolment_deleted.php +++ b/lib/classes/event/user_enrolment_deleted.php @@ -95,6 +95,14 @@ class user_enrolment_deleted extends base { return (object)$this->other['userenrolment']; } + /** + * Return legacy data for add_to_log(). + * + * @return array + */ + protected function get_legacy_logdata() { + return array($this->courseid, 'course', 'unenrol', '../enrol/users.php?id=' . $this->courseid, $this->courseid); + } /** * Custom validation. From f1f4f4fd7be51bfe60493eba27e097fd89f61173 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 13 Jan 2014 16:31:55 -0800 Subject: [PATCH 3/4] MDL-40909 core_enrol: removed 'enrol' add_to_log calls Before each of these calls a call to enrol_user() is made, which triggers the event 'user_enrolment_created'. Nothing more is needed. --- enrol/manual/manage.php | 1 - enrol/self/lib.php | 1 - enrol/tests/enrollib_test.php | 3 +++ lib/classes/event/user_enrolment_created.php | 9 +++++++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/enrol/manual/manage.php b/enrol/manual/manage.php index 64b542fbba1..6c86e6fb45d 100644 --- a/enrol/manual/manage.php +++ b/enrol/manual/manage.php @@ -123,7 +123,6 @@ if ($canenrol && optional_param('add', false, PARAM_BOOL) && confirm_sesskey()) $timeend = $timestart + $extendperiod; } $enrol_manual->enrol_user($instance, $adduser->id, $roleid, $timestart, $timeend); - add_to_log($course->id, 'course', 'enrol', '../enrol/users.php?id='.$course->id, $course->id); //there should be userid somewhere! } $potentialuserselector->invalidate_selected_users(); diff --git a/enrol/self/lib.php b/enrol/self/lib.php index d64281ab9a6..edc114f7bfb 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -195,7 +195,6 @@ class enrol_self_plugin extends enrol_plugin { } $this->enrol_user($instance, $USER->id, $instance->roleid, $timestart, $timeend); - add_to_log($instance->courseid, 'course', 'enrol', '../enrol/users.php?id='.$instance->courseid, $instance->courseid); //TODO: There should be userid somewhere! if ($instance->password and $instance->customint1 and $data->enrolpassword !== $instance->password) { // It must be a group enrolment, let's assign group too. diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php index af2960e8b03..394eb6cb66f 100644 --- a/enrol/tests/enrollib_test.php +++ b/enrol/tests/enrollib_test.php @@ -310,11 +310,14 @@ class core_enrollib_testcase extends advanced_testcase { $dbuserenrolled = $DB->get_record('user_enrolments', array('userid' => $admin->id)); $this->assertInstanceOf('\core\event\user_enrolment_created', $event); $this->assertEquals($dbuserenrolled->id, $event->objectid); + $this->assertEquals(context_course::instance($course1->id), $event->get_context()); $this->assertEquals('user_enrolled', $event->get_legacy_eventname()); $expectedlegacyeventdata = $dbuserenrolled; $expectedlegacyeventdata->enrol = $manual->get_name(); $expectedlegacyeventdata->courseid = $course1->id; $this->assertEventLegacyData($expectedlegacyeventdata, $event); + $expected = array($course1->id, 'course', 'enrol', '../enrol/users.php?id=' . $course1->id, $course1->id); + $this->assertEventLegacyLogData($expected, $event); } /** diff --git a/lib/classes/event/user_enrolment_created.php b/lib/classes/event/user_enrolment_created.php index 23539ca3123..825eb5321b6 100644 --- a/lib/classes/event/user_enrolment_created.php +++ b/lib/classes/event/user_enrolment_created.php @@ -97,6 +97,15 @@ class user_enrolment_created extends base { return $legacyeventdata; } + /** + * Return legacy data for add_to_log(). + * + * @return array + */ + protected function get_legacy_logdata() { + return array($this->courseid, 'course', 'enrol', '../enrol/users.php?id=' . $this->courseid, $this->courseid); + } + /** * Custom validation. * From 7cbb5070f5d1e59865185ad4a37c9872c904d044 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Mon, 13 Jan 2014 17:00:56 -0800 Subject: [PATCH 4/4] MDL-40909 enrol_imsenterprise: removed 'new' add_to_log call Here we are replacing the unnecessary code used to create a course with a call to create_course(), which triggers the event 'course_created'. Nothing more is needed. --- course/tests/courselib_test.php | 37 +++++++++++++++++-- enrol/imsenterprise/lib.php | 15 +------- .../tests/imsenterprise_test.php | 6 +-- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 3f8d5b897a1..56ad60f3058 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -26,8 +26,9 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; -require_once($CFG->dirroot.'/course/lib.php'); -require_once($CFG->dirroot.'/course/tests/fixtures/course_capability_assignment.php'); +require_once($CFG->dirroot . '/course/lib.php'); +require_once($CFG->dirroot . '/course/tests/fixtures/course_capability_assignment.php'); +require_once($CFG->dirroot . '/enrol/imsenterprise/tests/imsenterprise_test.php'); class core_course_courselib_testcase extends advanced_testcase { @@ -1400,8 +1401,10 @@ class core_course_courselib_testcase extends advanced_testcase { // Catch the events. $sink = $this->redirectEvents(); - // Create the course. - $course = $this->getDataGenerator()->create_course(); + // Create the course with an id number which is used later when generating a course via the imsenterprise plugin. + $data = new stdClass(); + $data->idnumber = 'idnumber'; + $course = $this->getDataGenerator()->create_course($data); // Get course from DB for comparison. $course = $DB->get_record('course', array('id' => $course->id)); @@ -1420,6 +1423,32 @@ class core_course_courselib_testcase extends advanced_testcase { $this->assertEventLegacyData($course, $event); $expectedlog = array(SITEID, 'course', 'new', 'view.php?id=' . $course->id, $course->fullname . ' (ID ' . $course->id . ')'); $this->assertEventLegacyLogData($expectedlog, $event); + + // Now we want to trigger creating a course via the imsenterprise. + // Delete the course we created earlier, as we want the imsenterprise plugin to create this. + // We do not want print out any of the text this function generates while doing this, which is why + // we are using ob_start() and ob_end_clean(). + ob_start(); + delete_course($course); + ob_end_clean(); + + // Create the XML file we want to use. + $imstestcase = new enrol_imsenterprise_testcase(); + $imstestcase->imsplugin = enrol_get_plugin('imsenterprise'); + $imstestcase->set_test_config(); + $imstestcase->set_xml_file(false, array($course)); + + // Capture the event. + $sink = $this->redirectEvents(); + $imstestcase->imsplugin->cron(); + $events = $sink->get_events(); + $sink->close(); + $event = $events[0]; + + // 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 + // imsenterprise plugin creates a course an event is triggered. + $this->assertInstanceOf('\core\event\course_created', $event); } /** diff --git a/enrol/imsenterprise/lib.php b/enrol/imsenterprise/lib.php index 34a04429ce1..6db08519456 100644 --- a/enrol/imsenterprise/lib.php +++ b/enrol/imsenterprise/lib.php @@ -377,24 +377,11 @@ class enrol_imsenterprise_plugin extends enrol_plugin { } else { $course->category = $this->get_default_category_id(); } - $course->timecreated = time(); $course->startdate = time(); // Choose a sort order that puts us at the start of the list! $course->sortorder = 0; - $courseid = $DB->insert_record('course', $course); - // Setup default enrolment plugins. - $course->id = $courseid; - enrol_course_updated(true, $course, null); - - // Setup the blocks. - $course = $DB->get_record('course', array('id' => $courseid)); - blocks_add_default_course_blocks($course); - - // Create default 0-section. - course_create_sections_if_missing($course, 0); - - add_to_log(SITEID, "course", "new", "view.php?id=$course->id", "$course->fullname (ID $course->id)"); + $course = create_course($course); $this->log_line("Created course $coursecode in Moodle (Moodle ID is $course->id)"); } diff --git a/enrol/imsenterprise/tests/imsenterprise_test.php b/enrol/imsenterprise/tests/imsenterprise_test.php index 390f276836a..a86006ee94f 100644 --- a/enrol/imsenterprise/tests/imsenterprise_test.php +++ b/enrol/imsenterprise/tests/imsenterprise_test.php @@ -42,7 +42,7 @@ class enrol_imsenterprise_testcase extends advanced_testcase { /** * @var $imsplugin enrol_imsenterprise_plugin IMS plugin instance. */ - protected $imsplugin; + public $imsplugin; /** * Setup required for all tests. @@ -254,7 +254,7 @@ class enrol_imsenterprise_testcase extends advanced_testcase { /** * Sets the plugin configuration for testing */ - protected function set_test_config() { + public function set_test_config() { $this->imsplugin->set_config('mailadmins', false); $this->imsplugin->set_config('prev_path', ''); $this->imsplugin->set_config('createnewusers', true); @@ -268,7 +268,7 @@ class enrol_imsenterprise_testcase extends advanced_testcase { * @param bool|array $users false or array of users StdClass * @param bool|array $courses false or of courses StdClass */ - protected function set_xml_file($users = false, $courses = false) { + public function set_xml_file($users = false, $courses = false) { $xmlcontent = '';