From 5900513f755a2a5b9f790e9dc7ce926571d6af31 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Mon, 24 Apr 2017 13:40:54 +0800 Subject: [PATCH 1/2] MDL-58688 calendar: use modinfo to check module availability --- calendar/export_execute.php | 6 +-- calendar/externallib.php | 11 ++++- calendar/lib.php | 60 ++++++++++++-------------- calendar/renderer.php | 4 +- calendar/tests/externallib_test.php | 65 ++++++++++++++++++++++++++++- 5 files changed, 104 insertions(+), 42 deletions(-) diff --git a/calendar/export_execute.php b/calendar/export_execute.php index add05a58d1f..a711b766080 100644 --- a/calendar/export_execute.php +++ b/calendar/export_execute.php @@ -185,9 +185,9 @@ $events = calendar_get_legacy_events($timestart, $timeend, $users, $groups, arra $ical = new iCalendar; $ical->add_property('method', 'PUBLISH'); foreach($events as $event) { - if (!empty($event->modulename)) { - $cm = get_coursemodule_from_instance($event->modulename, $event->instance); - if (!\core_availability\info_module::is_user_visible($cm, $userid, false)) { + if (!empty($event->modulename)) { + $instances = get_fast_modinfo($event->courseid, $userid)->get_instances_of($event->modulename); + if (empty($instances[$event->instance]->uservisible)) { continue; } } diff --git a/calendar/externallib.php b/calendar/externallib.php index 074f0e06395..bf0855be580 100644 --- a/calendar/externallib.php +++ b/calendar/externallib.php @@ -254,8 +254,15 @@ class core_calendar_external extends external_api { // User can see everything, no further check is needed. $events[$eventid] = $event; } else if (!empty($eventobj->modulename)) { - $cm = get_coursemodule_from_instance($eventobj->modulename, $eventobj->instance); - if (\core_availability\info_module::is_user_visible($cm, 0, false)) { + $courseid = $eventobj->courseid; + if (!$courseid) { + if (!$calendareventobj->context || !($context = $calendareventobj->context->get_course_context(false))) { + continue; + } + $courseid = $context->instanceid; + } + $instances = get_fast_modinfo($courseid)->get_instances_of($eventobj->modulename); + if (!empty($instances[$eventobj->instance]->uservisible)) { $events[$eventid] = $event; } } else { diff --git a/calendar/lib.php b/calendar/lib.php index 5df2607af51..51088323022 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -1404,8 +1404,8 @@ function calendar_get_mini($courses, $groups, $users, $calmonth = false, $calyea if (!empty($events)) { foreach ($events as $eventid => $event) { if (!empty($event->modulename)) { - $cm = get_coursemodule_from_instance($event->modulename, $event->instance); - if (!\core_availability\info_module::is_user_visible($cm, 0, false)) { + $instances = get_fast_modinfo($event->courseid)->get_instances_of($event->modulename); + if (empty($instances[$event->instance]->uservisible)) { unset($events[$eventid]); } } @@ -1730,23 +1730,11 @@ function calendar_get_upcoming($courses, $groups, $users, $daysinfuture, $maxeve } if ($events !== false) { - $modinfo = get_fast_modinfo($COURSE); foreach ($events as $event) { if (!empty($event->modulename)) { - if ($event->courseid == $COURSE->id) { - if (isset($modinfo->instances[$event->modulename][$event->instance])) { - $cm = $modinfo->instances[$event->modulename][$event->instance]; - if (!$cm->uservisible) { - continue; - } - } - } else { - if (!$cm = get_coursemodule_from_instance($event->modulename, $event->instance)) { - continue; - } - if (!\core_availability\info_module::is_user_visible($cm, 0, false)) { - continue; - } + $instances = get_fast_modinfo($event->courseid)->get_instances_of($event->modulename); + if (empty($instances[$event->instance]->uservisible)) { + continue; } } @@ -1766,18 +1754,20 @@ function calendar_get_upcoming($courses, $groups, $users, $daysinfuture, $maxeve /** * Get a HTML link to a course. * - * @param int $courseid the course id + * @param int|stdClass $course the course id or course object * @return string a link to the course (as HTML); empty if the course id is invalid */ -function calendar_get_courselink($courseid) { - if (!$courseid) { +function calendar_get_courselink($course) { + if (!$course) { return ''; } - calendar_get_course_cached($coursecache, $courseid); - $context = \context_course::instance($courseid); - $fullname = format_string($coursecache[$courseid]->fullname, true, array('context' => $context)); - $url = new \moodle_url('/course/view.php', array('id' => $courseid)); + if (!is_object($course)) { + $course = calendar_get_course_cached($coursecache, $course); + } + $context = \context_course::instance($course->id); + $fullname = format_string($course->fullname, true, array('context' => $context)); + $url = new \moodle_url('/course/view.php', array('id' => $course->id)); $link = \html_writer::link($url, $fullname); return $link; @@ -1786,6 +1776,9 @@ function calendar_get_courselink($courseid) { /** * Get current module cache. * + * It is not recommended to use this method. If course is known use: + * get_fast_modinfo($courseid)->instances[$modulename][$instance] + * * @param array $modulecache in memory module cache * @param string $modulename name of the module * @param int $instance module instance number @@ -1842,26 +1835,25 @@ function calendar_add_event_metadata($event) { if (!empty($event->modulename)) { // Activity event. // The module name is set. I will assume that it has to be displayed, and // also that it is an automatically-generated event. And of course that the - // fields for get_coursemodule_from_instance are set correctly. - $module = calendar_get_module_cached($coursecache, $event->modulename, $event->instance); - - if ($module === false) { + // instace id and modulename are set correctly. + $instances = get_fast_modinfo($event->courseid)->get_instances_of($event->modulename); + if (!array_key_exists($event->instance, $instances)) { return; } + $module = $instances[$event->instance]; - $modulename = get_string('modulename', $event->modulename); + $modulename = $module->get_module_type_name(false); if (get_string_manager()->string_exists($event->eventtype, $event->modulename)) { // Will be used as alt text if the event icon. $eventtype = get_string($event->eventtype, $event->modulename); } else { $eventtype = ''; } - $icon = $OUTPUT->image_url('icon', $event->modulename) . ''; - $event->icon = '' . $eventtype . ''; - $event->referer = '' . $event->name . ''; - $event->courselink = calendar_get_courselink($module->course); + $event->icon = '' . s($eventtype) . + ''; + $event->referer = html_writer::link($module->url, $event->name); + $event->courselink = calendar_get_courselink($module->get_course()); $event->cmid = $module->id; } else if ($event->courseid == SITEID) { // Site event. $event->icon = '' . diff --git a/calendar/renderer.php b/calendar/renderer.php index ed4bcef715e..8abfb038ce4 100644 --- a/calendar/renderer.php +++ b/calendar/renderer.php @@ -381,8 +381,8 @@ class core_calendar_renderer extends plugin_renderer_base { foreach($events as $eventid => $event) { $event = new calendar_event($event); if (!empty($event->modulename)) { - $cm = get_coursemodule_from_instance($event->modulename, $event->instance); - if (!\core_availability\info_module::is_user_visible($cm, 0, false)) { + $instances = get_fast_modinfo($event->courseid)->get_instances_of($event->modulename); + if (empty($instances[$event->instance]->uservisible)) { unset($events[$eventid]); } } diff --git a/calendar/tests/externallib_test.php b/calendar/tests/externallib_test.php index 62f93363104..9912bcc1381 100644 --- a/calendar/tests/externallib_test.php +++ b/calendar/tests/externallib_test.php @@ -100,7 +100,7 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { if (!empty($userid)) { $prop->userid = $userid; } else { - return false; + $prop->userid = 0; } } if (!isset($prop->courseid)) { @@ -669,6 +669,69 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { $this->assertNull($result['lastid']); } + /** + * Test retrieving event that was overridden for a user + */ + public function test_get_calendar_events_override() { + $user = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + $anotheruser = $this->getDataGenerator()->create_user(); + $course = $this->getDataGenerator()->create_course(); + $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + $moduleinstance = $generator->create_instance(['course' => $course->id]); + + $this->getDataGenerator()->enrol_user($user->id, $course->id); + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, 'editingteacher'); + $this->resetAfterTest(true); + $this->setAdminUser(); + + $params = [ + 'type' => CALENDAR_EVENT_TYPE_ACTION, + 'modulename' => 'assign', + 'instance' => $moduleinstance->id, + ]; + + $now = time(); + $event1 = $this->create_calendar_event('Base event', 0, 'due', 0, $now + DAYSECS, $params + ['courseid' => $course->id]); + $event2 = $this->create_calendar_event('User event', $user->id, 'due', 0, $now + 2*DAYSECS, $params + ['courseid' => 0]); + + // Retrieve course events for teacher - only one setUser($teacher); + $paramevents = array('courseids' => array($course->id)); + $options = array ('siteevents' => true, 'userevents' => true); + $events = core_calendar_external::get_calendar_events($paramevents, $options); + $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); + $this->assertEquals(1, count($events['events'])); + $this->assertEquals(0, count($events['warnings'])); + $this->assertEquals('Base event', $events['events'][0]['name']); + + // Retrieve events for user - both events are returned. + $this->setUser($user); + $events = core_calendar_external::get_calendar_events($paramevents, $options); + $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); + $this->assertEquals(2, count($events['events'])); + $this->assertEquals(0, count($events['warnings'])); + $this->assertEquals('Base event', $events['events'][0]['name']); + $this->assertEquals('User event', $events['events'][1]['name']); + + // Retrieve events by id as a teacher, 'User event' should be returned since teacher has access to this course. + $this->setUser($teacher); + $paramevents = ['eventids' => [$event2->id]]; + $events = core_calendar_external::get_calendar_events($paramevents, $options); + $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); + $this->assertEquals(1, count($events['events'])); + $this->assertEquals(0, count($events['warnings'])); + $this->assertEquals('User event', $events['events'][0]['name']); + + // Retrieve events by id as another user, nothing should be returned. + $this->setUser($anotheruser); + $paramevents = ['eventids' => [$event2->id, $event1->id]]; + $events = core_calendar_external::get_calendar_events($paramevents, $options); + $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); + $this->assertEquals(0, count($events['events'])); + $this->assertEquals(0, count($events['warnings'])); + } + /** * Requesting calendar events within a given time range should return all events with * a sort time between the lower and upper time bound (inclusive). From 47a710177a1570b46eb0ab9b08f0f781d5afc681 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Mon, 24 Apr 2017 14:09:09 +0800 Subject: [PATCH 2/2] MDL-58688 calendar: retrieve event module without db queries --- .../local/event/data_access/event_vault.php | 7 +- .../local/event/entities/event_interface.php | 2 + .../factories/event_abstract_factory.php | 27 +-- .../local/event/mappers/event_mapper.php | 8 +- ...module_std_proxy.php => cm_info_proxy.php} | 53 +++-- .../local/event/proxies/proxy_interface.php | 17 -- .../classes/local/event/proxies/std_proxy.php | 14 +- calendar/lib.php | 2 +- calendar/tests/cm_info_proxy_test.php | 59 +++++ calendar/tests/container_test.php | 8 +- calendar/tests/event_factory_test.php | 2 +- calendar/tests/event_mapper_test.php | 10 +- calendar/tests/helpers.php | 10 +- calendar/tests/module_std_proxy_test.php | 219 ------------------ calendar/tests/std_proxy_test.php | 19 -- 15 files changed, 123 insertions(+), 334 deletions(-) rename calendar/classes/local/event/proxies/{module_std_proxy.php => cm_info_proxy.php} (53%) create mode 100644 calendar/tests/cm_info_proxy_test.php delete mode 100644 calendar/tests/module_std_proxy_test.php diff --git a/calendar/classes/local/event/data_access/event_vault.php b/calendar/classes/local/event/data_access/event_vault.php index eaed1187229..c24c5f889ce 100644 --- a/calendar/classes/local/event/data_access/event_vault.php +++ b/calendar/classes/local/event/data_access/event_vault.php @@ -252,7 +252,7 @@ class event_vault implements event_vault_interface { true, true, function ($event) use ($course) { - return $event instanceof action_event_interface && $event->get_course()->get_id() == $course->id; + return $event instanceof action_event_interface && $event->get_course()->get('id') == $course->id; } ) ); @@ -348,11 +348,6 @@ class event_vault implements event_vault_interface { * @return event_interface|null */ protected function transform_from_database_record(\stdClass $record) { - if ($record->courseid == 0 && $record->instance && $record->modulename) { - list($course, $cm) = get_course_and_cm_from_instance($record->instance, $record->modulename); - $record->courseid = $course->id; - } - return $this->factory->create_instance($record); } diff --git a/calendar/classes/local/event/entities/event_interface.php b/calendar/classes/local/event/entities/event_interface.php index a277901c3db..192e75d9840 100644 --- a/calendar/classes/local/event/entities/event_interface.php +++ b/calendar/classes/local/event/entities/event_interface.php @@ -24,6 +24,8 @@ namespace core_calendar\local\event\entities; +use core_calendar\local\event\proxies\proxy_interface; + defined('MOODLE_INTERNAL') || die(); /** diff --git a/calendar/classes/local/event/factories/event_abstract_factory.php b/calendar/classes/local/event/factories/event_abstract_factory.php index c3520f42b9b..2695d93cf39 100644 --- a/calendar/classes/local/event/factories/event_abstract_factory.php +++ b/calendar/classes/local/event/factories/event_abstract_factory.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); use core_calendar\local\event\entities\event; use core_calendar\local\event\entities\repeat_event_collection; use core_calendar\local\event\exceptions\invalid_callback_exception; -use core_calendar\local\event\proxies\module_std_proxy; +use core_calendar\local\event\proxies\cm_info_proxy; use core_calendar\local\event\proxies\std_proxy; use core_calendar\local\event\value_objects\event_description; use core_calendar\local\event\value_objects\event_times; @@ -107,6 +107,12 @@ abstract class event_abstract_factory implements event_factory_interface { } public function create_instance(\stdClass $dbrow) { + if ($dbrow->modulename && $dbrow->instance && $dbrow->courseid == 0) { + // Some events (for example user overrides) may contain module instance but not course id. Find course id. + $cm = calendar_get_module_cached($this->modulecachereference, $dbrow->modulename, $dbrow->instance); + $dbrow->courseid = $cm->course; + } + $bailcheck = $this->bailoutcheck; $bail = $bailcheck($dbrow); @@ -126,9 +132,8 @@ abstract class event_abstract_factory implements event_factory_interface { $module = null; $subscription = null; - if ($dbrow->courseid == 0 && !empty($dbrow->modulename)) { - $cm = get_coursemodule_from_instance($dbrow->modulename, $dbrow->instance); - $dbrow->courseid = get_course($cm->course)->id; + if ($dbrow->modulename && $dbrow->instance) { + $module = new cm_info_proxy($dbrow->modulename, $dbrow->instance, $dbrow->courseid); } $course = new std_proxy($dbrow->courseid, function($id) { @@ -148,20 +153,6 @@ abstract class event_abstract_factory implements event_factory_interface { }); } - if ($dbrow->instance && !empty($dbrow->modulename)) { - $module = new module_std_proxy( - $dbrow->modulename, - $dbrow->instance, - function($modulename, $instance) { - return calendar_get_module_cached( - $this->modulecachereference, - $modulename, - $instance - ); - } - ); - } - if ($dbrow->subscriptionid) { $subscription = new std_proxy($dbrow->subscriptionid, function($id) { return calendar_get_subscription($id); diff --git a/calendar/classes/local/event/mappers/event_mapper.php b/calendar/classes/local/event/mappers/event_mapper.php index ae50e7d3615..a4191f2aee3 100644 --- a/calendar/classes/local/event/mappers/event_mapper.php +++ b/calendar/classes/local/event/mappers/event_mapper.php @@ -103,9 +103,9 @@ class event_mapper implements event_mapper_interface { 'name' => $event->get_name(), 'description' => $event->get_description()->get_value(), 'format' => $event->get_description()->get_format(), - 'courseid' => $event->get_course() ? $event->get_course()->get_id() : null, - 'groupid' => $event->get_group() ? $event->get_group()->get_id() : null, - 'userid' => $event->get_user() ? $event->get_user()->get_id() : null, + 'courseid' => $event->get_course() ? $event->get_course()->get('id') : null, + 'groupid' => $event->get_group() ? $event->get_group()->get('id') : null, + 'userid' => $event->get_user() ? $event->get_user()->get('id') : null, 'repeatid' => $event->get_repeats()->get_id(), 'modulename' => $event->get_course_module() ? $event->get_course_module()->get('modname') : null, 'instance' => $event->get_course_module() ? $event->get_course_module()->get('instance') : null, @@ -115,7 +115,7 @@ class event_mapper implements event_mapper_interface { 'timesort' => $event->get_times()->get_sort_time()->getTimestamp(), 'visible' => $event->is_visible() ? 1 : 0, 'timemodified' => $event->get_times()->get_modified_time()->getTimestamp(), - 'subscriptionid' => $event->get_subscription() ? $event->get_subscription()->get_id() : null, + 'subscriptionid' => $event->get_subscription() ? $event->get_subscription()->get('id') : null, 'actionname' => $action ? $action->get_name() : null, 'actionurl' => $action ? $action->get_url() : null, 'actionnum' => $action ? $action->get_item_count() : null, diff --git a/calendar/classes/local/event/proxies/module_std_proxy.php b/calendar/classes/local/event/proxies/cm_info_proxy.php similarity index 53% rename from calendar/classes/local/event/proxies/module_std_proxy.php rename to calendar/classes/local/event/proxies/cm_info_proxy.php index 1ad48fbf174..7aee7b04f6b 100644 --- a/calendar/classes/local/event/proxies/module_std_proxy.php +++ b/calendar/classes/local/event/proxies/cm_info_proxy.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * Course module stdClass proxy. + * Course module cm_info proxy. * * @package core_calendar * @copyright 2017 Cameron Ball @@ -38,27 +38,46 @@ defined('MOODLE_INTERNAL') || die(); * @copyright 2017 Cameron Ball * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class module_std_proxy extends std_proxy implements proxy_interface { +class cm_info_proxy implements proxy_interface { + /** @var \stdClass */ + protected $base; + /** @var \cm_info */ + protected $cm; /** - * module_std_proxy constructor. + * cm_info_proxy constructor. * - * @param int $modulename The module name. - * @param callable $instance The module instance. - * @param callable $callback Callback to load the class. - * @param \stdClass $base Class containing base values. + * @param string $modname The module name. + * @param int $instance The module instance. + * @param int $courseid course id this module belongs to */ - public function __construct($modulename, $instance, callable $callback, \stdClass $base = null) { - $this->modulename = $modulename; - $this->instance = $instance; - $this->callbackargs = [$modulename, $instance]; - $this->callback = $callback; - $this->base = $base = is_null($base) ? new \stdClass() : $base; - $this->base->modulename = $modulename; - $this->base->instance = $instance; + public function __construct($modname, $instance, $courseid) { + $this->base = (object)['course' => $courseid, 'modname' => $modname, 'instance' => $instance]; } - public function get_id() { - return $this->get_proxied_instance()->id; + /** + * Retrieve a member of the proxied class. + * + * @param string $member The name of the member to retrieve + * @return mixed The member. + */ + public function get($member) { + if ($this->base && property_exists($this->base, $member)) { + return $this->base->{$member}; + } + + return $this->get_proxied_instance()->{$member}; + } + + /** + * Get the full instance of the proxied class. + * + * @return \stdClass + */ + public function get_proxied_instance() { + if (!$this->cm) { + $this->cm = get_fast_modinfo($this->base->course)->instances[$this->base->modname][$this->base->instance]; + } + return $this->cm; } } diff --git a/calendar/classes/local/event/proxies/proxy_interface.php b/calendar/classes/local/event/proxies/proxy_interface.php index 6869fd85966..d6ed0214621 100644 --- a/calendar/classes/local/event/proxies/proxy_interface.php +++ b/calendar/classes/local/event/proxies/proxy_interface.php @@ -43,23 +43,6 @@ interface proxy_interface { */ public function get($member); - /** - * Retrieve the ID of the proxied class. - * @return int The proxied class' ID. - */ - public function get_id(); - - /** - * Set a member of the proxied class. - * - * @param string $member The name of the member to set - * @param mixed $value The value to set the member to - * @throws \core_calendar\local\event\exceptions\member_does_not_exist_exception If the proxied class does not have the - * requested member. - * @return void - */ - public function set($member, $value); - /** * Get the full instance of the proxied class. * diff --git a/calendar/classes/local/event/proxies/std_proxy.php b/calendar/classes/local/event/proxies/std_proxy.php index e94746d65a2..04c892d2ff3 100644 --- a/calendar/classes/local/event/proxies/std_proxy.php +++ b/calendar/classes/local/event/proxies/std_proxy.php @@ -78,13 +78,9 @@ class std_proxy implements proxy_interface { $this->base = $base; } - public function get_id() { - return $this->id; - } - public function get($member) { if ($member === 'id') { - return $this->get_id(); + return $this->id; } if ($this->base && property_exists($this->base, $member)) { @@ -98,14 +94,6 @@ class std_proxy implements proxy_interface { return $this->get_proxied_instance()->{$member}; } - public function set($member, $value) { - if (!property_exists($this->get_proxied_instance(), $member)) { - throw new member_does_not_exist_exception(sprintf('Member %s does not exist', $member)); - } - - $this->get_proxied_instance()->{$member} = $value; - } - public function get_proxied_instance() { $callback = $this->callback; return $this->class = $this->class ? $this->class : $callback(...$this->callbackargs); diff --git a/calendar/lib.php b/calendar/lib.php index 51088323022..dc38031758f 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -1776,7 +1776,7 @@ function calendar_get_courselink($course) { /** * Get current module cache. * - * It is not recommended to use this method. If course is known use: + * Only use this method if you do not know courseid. Otherwise use: * get_fast_modinfo($courseid)->instances[$modulename][$instance] * * @param array $modulecache in memory module cache diff --git a/calendar/tests/cm_info_proxy_test.php b/calendar/tests/cm_info_proxy_test.php new file mode 100644 index 00000000000..95aba86cb79 --- /dev/null +++ b/calendar/tests/cm_info_proxy_test.php @@ -0,0 +1,59 @@ +. + +/** + * cm_info_proxy tests. + * + * @package core_calendar + * @copyright 2017 Cameron Ball + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +use core_calendar\local\event\proxies\cm_info_proxy; + +/** + * cm_info_proxy testcase. + * + * @copyright 2017 Cameron Ball + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_calendar_cm_info_proxy_testcase extends advanced_testcase { + + /** + * Test creating cm_info_std_proxy, using getter and setter. + */ + public function test_proxy() { + $this->resetAfterTest(true); + + $course = $this->getDataGenerator()->create_course(); + $module = $this->getDataGenerator()->create_module('forum', + ['course' => $course->id, 'idnumber' => '123456']); + $proxy = new cm_info_proxy( + 'forum', + $module->id, + $course->id + ); + + $this->assertEquals('forum', $proxy->get('modname')); + $this->assertEquals($module->id, $proxy->get('instance')); + $this->assertEquals($course->id, $proxy->get('course')); + $this->assertEquals('123456', $proxy->get('idnumber')); + $this->assertEquals($module->cmid, $proxy->get('id')); + $this->assertEquals('123456', $proxy->get_proxied_instance()->idnumber); + } +} diff --git a/calendar/tests/container_test.php b/calendar/tests/container_test.php index 8f4f5a7f47f..8d9c62fdd66 100644 --- a/calendar/tests/container_test.php +++ b/calendar/tests/container_test.php @@ -99,15 +99,15 @@ class core_calendar_container_testcase extends advanced_testcase { $this->assertEquals($legacyevent->id, $event->get_id()); $this->assertEquals($dbrow->description, $event->get_description()->get_value()); $this->assertEquals($dbrow->format, $event->get_description()->get_format()); - $this->assertEquals($dbrow->courseid, $event->get_course()->get_id()); + $this->assertEquals($dbrow->courseid, $event->get_course()->get('id')); if ($dbrow->groupid == 0) { $this->assertNull($event->get_group()); } else { - $this->assertEquals($dbrow->groupid, $event->get_group()->get_id()); + $this->assertEquals($dbrow->groupid, $event->get_group()->get('id')); } - $this->assertEquals($dbrow->userid, $event->get_user()->get_id()); + $this->assertEquals($dbrow->userid, $event->get_user()->get('id')); $this->assertEquals($legacyevent->id, $event->get_repeats()->get_id()); $this->assertEquals($dbrow->modulename, $event->get_course_module()->get('modname')); $this->assertEquals($dbrow->instance, $event->get_course_module()->get('instance')); @@ -124,7 +124,7 @@ class core_calendar_container_testcase extends advanced_testcase { if (!$dbrow->subscriptionid) { $this->assertNull($event->get_subscription()); } else { - $this->assertEquals($event->get_subscription()->get_id()); + $this->assertEquals($event->get_subscription()->get('id')); } } diff --git a/calendar/tests/event_factory_test.php b/calendar/tests/event_factory_test.php index b2f3039f764..2cd22ea0516 100644 --- a/calendar/tests/event_factory_test.php +++ b/calendar/tests/event_factory_test.php @@ -312,7 +312,7 @@ class core_calendar_event_factory_testcase extends advanced_testcase { 'name' => 'test', 'description' => 'Test description', 'format' => 2, - 'courseid' => $course->id, + 'courseid' => 0, 'groupid' => 1, 'userid' => 1, 'repeatid' => 0, diff --git a/calendar/tests/event_mapper_test.php b/calendar/tests/event_mapper_test.php index 8d99c57a09d..0071a500d9d 100644 --- a/calendar/tests/event_mapper_test.php +++ b/calendar/tests/event_mapper_test.php @@ -382,16 +382,12 @@ class event_mapper_test_proxy implements proxy_interface { } public function get($member) { + if ($member === 'id') { + return $this->id; + } return isset($params[$member]) ? $params[$member] : null; } - public function get_id() { - return $this->id; - } - - public function set($member, $value) { - } - public function get_proxied_instance() { } } diff --git a/calendar/tests/helpers.php b/calendar/tests/helpers.php index c990d4de37d..f8e08fbeeaf 100644 --- a/calendar/tests/helpers.php +++ b/calendar/tests/helpers.php @@ -32,6 +32,7 @@ use core_calendar\local\event\entities\action_event; use core_calendar\local\event\entities\event; use core_calendar\local\event\entities\repeat_event_collection; use core_calendar\local\event\proxies\std_proxy; +use core_calendar\local\event\proxies\cm_info_proxy; use core_calendar\local\event\value_objects\action; use core_calendar\local\event\value_objects\event_description; use core_calendar\local\event\value_objects\event_times; @@ -94,14 +95,7 @@ class action_event_test_factory implements event_factory_interface { $subscription = null; if ($record->instance && $record->modulename) { - $modulename = $record->modulename; - $module = new std_proxy($record->instance, function($id) use ($modulename) { - return get_coursemodule_from_instance($modulename, $id); - }, - (object)[ - 'modname' => $modulename, - 'instance' => $record->instance - ]); + $module = new cm_info_proxy($record->instance, $record->modulename, $record->courseid); } if ($record->subscriptionid) { diff --git a/calendar/tests/module_std_proxy_test.php b/calendar/tests/module_std_proxy_test.php deleted file mode 100644 index c279eec806b..00000000000 --- a/calendar/tests/module_std_proxy_test.php +++ /dev/null @@ -1,219 +0,0 @@ -. - -/** - * module_std_proxy tests. - * - * @package core_calendar - * @copyright 2017 Cameron Ball - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -use core_calendar\local\event\proxies\module_std_proxy; - -/** - * std_proxy testcase. - * - * @copyright 2017 Cameron Ball - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class core_calendar_module_std_proxy_testcase extends advanced_testcase { - /** - * @var \stdClass[] $objects Array of objects to proxy. - */ - public $objects; - - /** - * Sets up the fixture. This method is called before a test is executed. - */ - public function setUp() { - $this->objects = [ - 'somemodule_someinstance' => (object) [ - 'member1' => 'Hello', - 'member2' => 1729, - 'member3' => 'Something else' - ], - 'anothermodule_anotherinstance' => (object) [ - 'member1' => 'Hej', - 'member2' => 87539319, - 'member3' => 'nagot annat' - ] - ]; - } - - /** - * Test proxying. - * - * @dataProvider test_proxy_testcases() - * @param string $modulename Object module name. - * @param string $instance Object instance. - * @param string $member Object member to retrieve. - * @param mixed $expected Expected value of member. - */ - public function test_proxy($modulename, $instance, $member, $expected) { - $proxy = new module_std_proxy( - $modulename, - $instance, - function($modulename, $instance) { - return $this->objects[$modulename . '_' . $instance]; - } - ); - - $this->assertEquals($proxy->get($member), $expected); - - // Test changing the value. - $proxy->set($member, 'something even more else'); - $this->assertEquals($proxy->get($member), 'something even more else'); - } - - /** - * Test setting values with a base class. - * - * @dataProvider test_proxy_testcases() - * @param string $modulename Object module name. - * @param string $instance Object instance. - * @param string $member Object member to retrieve. - * @param mixed $storedvalue Value as would be stored externally. - */ - public function test_base_values($modulename, $instance, $member, $storedvalue) { - $proxy = new module_std_proxy( - $modulename, - $instance, - function($module, $instance) { - return $this->objects[$module . '_' . $instance]; - }, - (object)['member1' => 'should clobber 1'] - ); - - $expected = $member == 'member1' ? 'should clobber 1' : $storedvalue; - $this->assertEquals($proxy->get($member), $expected); - } - - /** - * Test getting a non existant member. - * - * @dataProvider test_get_set_testcases() - * @param string $modulename Object module name. - * @param string $instance Object instance. - */ - public function test_get_invalid_member($modulename, $instance) { - $proxy = new module_std_proxy( - $modulename, - $instance, - function($modulename, $instance) { - return $this->objects[$modulename . '_' . $instance]; - } - ); - - $this->expectException('\core_calendar\local\event\exceptions\member_does_not_exist_exception'); - $proxy->get('thisdoesnotexist'); - } - - /** - * Test setting a non existant member. - * - * @dataProvider test_get_set_testcases() - * @param string $modulename Object module name. - * @param string $instance Object instance. - */ - public function test_set_invalid_member($modulename, $instance) { - $proxy = new module_std_proxy( - $modulename, - $instance, - function($modulename, $instance) { - return $this->objects[$modulename . '_' . $instance]; - } - ); - - $this->expectException('\core_calendar\local\event\exceptions\member_does_not_exist_exception'); - $proxy->set('thisdoesnotexist', 'should break'); - } - - /** - * Test get proxied instance. - * - * @dataProvider test_get_set_testcases() - * @param string $modulename Object module name. - * @param string $instance Object instance. - */ - public function test_get_proxied_instance($modulename, $instance) { - $proxy = new module_std_proxy( - $modulename, - $instance, - function($modulename, $instance) { - return $this->objects[$modulename . '_' . $instance]; - } - ); - - $this->assertEquals($proxy->get_proxied_instance(), $this->objects[$modulename . '_' . $instance]); - } - - /** - * Test cases for proxying test. - */ - public function test_proxy_testcases() { - return [ - 'Object 1 member 1' => [ - 'somemodule', - 'someinstance', - 'member1', - 'Hello' - ], - 'Object 1 member 2' => [ - 'somemodule', - 'someinstance', - 'member2', - 1729 - ], - 'Object 1 member 3' => [ - 'somemodule', - 'someinstance', - 'member3', - 'Something else' - ], - 'Object 2 member 1' => [ - 'anothermodule', - 'anotherinstance', - 'member1', - 'Hej' - ], - 'Object 2 member 2' => [ - 'anothermodule', - 'anotherinstance', - 'member2', - 87539319 - ], - 'Object 3 member 3' => [ - 'anothermodule', - 'anotherinstance', - 'member3', - 'nagot annat' - ] - ]; - } - - /** - * Test cases for getting and setting tests. - */ - public function test_get_set_testcases() { - return [ - 'Object 1' => ['somemodule', 'someinstance'], - 'Object 2' => ['anothermodule', 'anotherinstance'] - ]; - } -} diff --git a/calendar/tests/std_proxy_test.php b/calendar/tests/std_proxy_test.php index cc912b3b7cf..dc9f25e3efb 100644 --- a/calendar/tests/std_proxy_test.php +++ b/calendar/tests/std_proxy_test.php @@ -67,10 +67,6 @@ class core_calendar_std_proxy_testcase extends advanced_testcase { }); $this->assertEquals($proxy->get($member), $expected); - - // Test changing the value. - $proxy->set($member, 'something even more else'); - $this->assertEquals($proxy->get($member), 'something even more else'); } /** @@ -109,21 +105,6 @@ class core_calendar_std_proxy_testcase extends advanced_testcase { $proxy->get('thisdoesnotexist'); } - /** - * Test setting a non existant member. - * - * @dataProvider test_get_set_testcases() - * @param int $id ID of the object being proxied. - */ - public function test_set_invalid_member($id) { - $proxy = new std_proxy($id, function($id) { - return $this->objects[$id]; - }); - - $this->expectException('\core_calendar\local\event\exceptions\member_does_not_exist_exception'); - $proxy->set('thisdoesnotexist', 'should break'); - } - /** * Test get proxied instance. *