From 44ae08383ba4c2866a2b7e25f97e0df8c3683c8d Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 15 Nov 2017 17:16:21 +0100 Subject: [PATCH] MDL-60813 calendar: Fix core_calendar_get_calendar_events for categories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users couldn’t see category events when filtering by categories in the core_calendar_get_calendar_events WS. --- .../local/event/mappers/event_mapper.php | 1 + calendar/externallib.php | 54 +++++++++---------- calendar/tests/container_test.php | 1 + calendar/tests/event_factory_test.php | 1 + calendar/tests/event_mapper_test.php | 1 + calendar/tests/externallib_test.php | 35 ++++++++++-- calendar/tests/helpers.php | 1 + .../tests/repeat_event_collection_test.php | 1 + lib/testing/generator/data_generator.php | 1 + 9 files changed, 63 insertions(+), 33 deletions(-) diff --git a/calendar/classes/local/event/mappers/event_mapper.php b/calendar/classes/local/event/mappers/event_mapper.php index 2e338699066..80ef5eda313 100644 --- a/calendar/classes/local/event/mappers/event_mapper.php +++ b/calendar/classes/local/event/mappers/event_mapper.php @@ -119,6 +119,7 @@ class event_mapper implements event_mapper_interface { 'description' => $event->get_description()->get_value(), 'format' => $event->get_description()->get_format(), 'courseid' => $event->get_course() ? $event->get_course()->get('id') : null, + 'categoryid' => $event->get_category() ? $event->get_category()->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(), diff --git a/calendar/externallib.php b/calendar/externallib.php index 213930f7ffc..b7735b51159 100644 --- a/calendar/externallib.php +++ b/calendar/externallib.php @@ -226,25 +226,36 @@ class core_calendar_external extends external_api { } $categories = array(); - if (empty($params['events']['categoryids']) && !empty($courses)) { - list($wheresql, $sqlparams) = $DB->get_in_or_equal($courses); - $wheresql = "id $wheresql"; - $courseswithcategory = $DB->get_records_select('course', $wheresql, $sqlparams); + if ($hassystemcap || !empty($courses)) { - // Grab the list of course categories for the requested course list. $coursecategories = array(); - foreach ($courseswithcategory as $course) { - if (empty($course->visible)) { - if (!has_capability('moodle/course:viewhidden', context_course::instance($course->id))) { - continue; + if (!empty($courses)) { + list($wheresql, $sqlparams) = $DB->get_in_or_equal($courses); + $wheresql = "id $wheresql"; + $courseswithcategory = $DB->get_records_select('course', $wheresql, $sqlparams); + + // Grab the list of course categories for the requested course list. + foreach ($courseswithcategory as $course) { + if (empty($course->visible)) { + if (!has_capability('moodle/course:viewhidden', context_course::instance($course->id))) { + continue; + } } + $category = \coursecat::get($course->category); + // Fetch parent categories. + $coursecategories = array_merge($coursecategories, [$category->id], $category->get_parents()); } - $category = \coursecat::get($course->category); - $coursecategories[] = $category; } foreach (\coursecat::get_all() as $category) { - if (has_capability('moodle/category:manage', $category->get_context(), $USER, false)) { + // Skip categories not requested. + if (!empty($params['events']['categoryids'])) { + if (!in_array($category->id, $params['events']['categoryids'])) { + continue; + } + } + + if (has_capability('moodle/category:manage', $category->get_context())) { // If a user can manage a category, then they can see all child categories. as well as all parent categories. $categories[] = $category->id; @@ -254,29 +265,14 @@ class core_calendar_external extends external_api { } } $categories = array_merge($categories, $category->get_parents()); - } else if (isset($coursecategories[$category->id])) { + } else if (in_array($category->id, $coursecategories)) { + // The user has access to a course in this category. // Fetch all of the parents too. $categories = array_merge($categories, [$category->id], $category->get_parents()); $categories[] = $category->id; } } - } else { - // Build the category list. - // This includes the current category. - foreach ($params['events']['categoryids'] as $categoryid) { - $category = \coursecat::get($categoryid); - $categories = [$category->id]; - // All of its descendants. - foreach (\coursecat::get_all() as $cat) { - if (array_search($categoryid, $cat->get_parents()) !== false) { - $categories[] = $cat->id; - } - } - - // And all of its parents. - $categories = array_merge($categories, $category->get_parents()); - } } $funcparam['categories'] = array_unique($categories); diff --git a/calendar/tests/container_test.php b/calendar/tests/container_test.php index 3298187ce8b..3323e1fe3e5 100644 --- a/calendar/tests/container_test.php +++ b/calendar/tests/container_test.php @@ -533,6 +533,7 @@ class core_calendar_container_testcase extends advanced_testcase { $record->timesort = 0; $record->type = 1; $record->courseid = 0; + $record->categoryid = 0; foreach ($properties as $name => $value) { $record->$name = $value; diff --git a/calendar/tests/event_factory_test.php b/calendar/tests/event_factory_test.php index 93751263e2e..e2bf0bffd7b 100644 --- a/calendar/tests/event_factory_test.php +++ b/calendar/tests/event_factory_test.php @@ -465,6 +465,7 @@ class core_calendar_event_factory_testcase extends advanced_testcase { $record->timesort = 0; $record->type = 1; $record->courseid = 0; + $record->categoryid = 0; foreach ($properties as $name => $value) { $record->$name = $value; diff --git a/calendar/tests/event_mapper_test.php b/calendar/tests/event_mapper_test.php index 829465165be..47f3f4bb458 100644 --- a/calendar/tests/event_mapper_test.php +++ b/calendar/tests/event_mapper_test.php @@ -147,6 +147,7 @@ class core_calendar_event_mapper_testcase extends advanced_testcase { $record->timesort = 0; $record->type = 1; $record->courseid = 0; + $record->categoryid = 0; foreach ($properties as $name => $value) { $record->$name = $value; diff --git a/calendar/tests/externallib_test.php b/calendar/tests/externallib_test.php index 2fe4bbbc8f8..178bfff6eed 100644 --- a/calendar/tests/externallib_test.php +++ b/calendar/tests/externallib_test.php @@ -485,31 +485,58 @@ class core_calendar_externallib_testcase extends externallib_advanced_testcase { // Create some category events. $this->setAdminUser(); $record = new stdClass(); + $record->courseid = 0; $record->categoryid = $category->id; - $this->create_calendar_event('category a', $USER->id, 'category', 0, time(), $record); + $record->timestart = time() - DAYSECS; + $catevent1 = $this->create_calendar_event('category a', $USER->id, 'category', 0, time(), $record); + $record = new stdClass(); + $record->courseid = 0; $record->categoryid = $category2->id; - $this->create_calendar_event('category b', $USER->id, 'category', 0, time(), $record); + $record->timestart = time() + DAYSECS; + $catevent2 = $this->create_calendar_event('category b', $USER->id, 'category', 0, time(), $record); // Now as student, make sure we get the events of the courses I am enrolled. $this->setUser($user2); $paramevents = array('categoryids' => array($category2b->id)); - $options = array('timeend' => time() + 7 * WEEKSECS); + $options = array('timeend' => time() + 7 * WEEKSECS, 'userevents' => false, 'siteevents' => false); $events = core_calendar_external::get_calendar_events($paramevents, $options); $events = external_api::clean_returnvalue(core_calendar_external::get_calendar_events_returns(), $events); // Should be just one, since there's just one category event of the course I am enrolled (course3 - cat2b). $this->assertEquals(1, count($events['events'])); + $this->assertEquals($catevent2->id, $events['events'][0]['id']); + $this->assertEquals(0, count($events['warnings'])); + + // Now get category events but by course (there aren't course events in the course). + $paramevents = array('courseids' => array($course3->id)); + $options = array('timeend' => time() + 7 * WEEKSECS, 'userevents' => false, 'siteevents' => false); + $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($catevent2->id, $events['events'][0]['id']); + $this->assertEquals(0, count($events['warnings'])); + + // Empty events in one where I'm not enrolled and one parent category + // (parent of a category where this is a course where the user is enrolled). + $paramevents = array('categoryids' => array($category2->id, $category->id)); + $options = array('timeend' => time() + 7 * WEEKSECS, 'userevents' => false, 'siteevents' => false); + $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($catevent2->id, $events['events'][0]['id']); $this->assertEquals(0, count($events['warnings'])); // Admin can see all category events. $this->setAdminUser(); $paramevents = array('categoryids' => array($category->id, $category2->id, $category2b->id)); - $options = array('timeend' => time() + 7 * WEEKSECS); + $options = array('timeend' => time() + 7 * WEEKSECS, 'userevents' => false, 'siteevents' => false); $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($catevent1->id, $events['events'][0]['id']); + $this->assertEquals($catevent2->id, $events['events'][1]['id']); } /** diff --git a/calendar/tests/helpers.php b/calendar/tests/helpers.php index 2c3b461c70d..014ae60a5e3 100644 --- a/calendar/tests/helpers.php +++ b/calendar/tests/helpers.php @@ -56,6 +56,7 @@ function create_event($properties) { $record->timesort = 0; $record->type = CALENDAR_EVENT_TYPE_STANDARD; $record->courseid = 0; + $record->categoryid = 0; foreach ($properties as $name => $value) { $record->$name = $value; diff --git a/calendar/tests/repeat_event_collection_test.php b/calendar/tests/repeat_event_collection_test.php index ec6320c8de8..0b935afd6d5 100644 --- a/calendar/tests/repeat_event_collection_test.php +++ b/calendar/tests/repeat_event_collection_test.php @@ -136,6 +136,7 @@ class core_calendar_repeat_event_collection_testcase extends advanced_testcase { $record->timesort = 0; $record->type = 1; $record->courseid = 0; + $record->categoryid = 0; foreach ($properties as $name => $value) { $record->$name = $value; diff --git a/lib/testing/generator/data_generator.php b/lib/testing/generator/data_generator.php index e3d42a83531..4bf2e8b42bf 100644 --- a/lib/testing/generator/data_generator.php +++ b/lib/testing/generator/data_generator.php @@ -1124,6 +1124,7 @@ EOD; $record->timesort = 0; $record->eventtype = 'user'; $record->courseid = 0; + $record->categoryid = 0; foreach ($data as $key => $value) { $record->$key = $value;