From 687112af223293ab3ee108c07ab0eb4a636d5392 Mon Sep 17 00:00:00 2001 From: Simey Lameze Date: Wed, 25 Jul 2018 08:40:28 +0800 Subject: [PATCH] MDL-60826 calendar: further performance improvements This commit does few tweaks and other performance improvements to calendar_get_allowed_event_types to be more efficient. Also some minor queries optimizations on the event form to fetch the list of courses without groups. --- .../classes/local/event/forms/eventtype.php | 15 +-- calendar/lib.php | 121 ++++++++++++------ 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/calendar/classes/local/event/forms/eventtype.php b/calendar/classes/local/event/forms/eventtype.php index da0fcf4db28..f8486875da7 100644 --- a/calendar/classes/local/event/forms/eventtype.php +++ b/calendar/classes/local/event/forms/eventtype.php @@ -53,7 +53,7 @@ trait eventtype { * @param array $eventtypes The available event types for the user */ protected function add_event_type_elements($mform, $eventtypes) { - global $DB; + global $CFG, $DB; $options = []; if (!empty($eventtypes['user'])) { @@ -98,23 +98,22 @@ trait eventtype { $mform->hideIf('categoryid', 'eventtype', 'noteq', 'category'); } + $showall = $CFG->calendar_adminseesall && !has_capability('moodle/calendar:manageentries', \context_system::instance()); if (!empty($eventtypes['course'])) { - $limit = !has_capability('moodle/calendar:manageentries', \context_system::instance()); - $mform->addElement('course', 'courseid', get_string('course'), ['limittoenrolled' => $limit]); + $mform->addElement('course', 'courseid', get_string('course'), ['limittoenrolled' => !$showall]); $mform->hideIf('courseid', 'eventtype', 'noteq', 'course'); } if (!empty($eventtypes['group'])) { $groups = !(empty($this->_customdata['groups'])) ? $this->_customdata['groups'] : null; - $limit = !has_capability('moodle/calendar:manageentries', \context_system::instance()); // Get the list of courses without groups to filter on the course selector. $sql = "SELECT c.id FROM {course} c - LEFT JOIN {groups} g ON g.courseid = c.id - GROUP BY c.id - HAVING COUNT(g.id) = 0"; + WHERE c.id NOT IN ( + SELECT DISTINCT courseid FROM {groups} + )"; $coursesnogroup = $DB->get_records_sql($sql); - $mform->addElement('course', 'groupcourseid', get_string('course'), ['limittoenrolled' => $limit, + $mform->addElement('course', 'groupcourseid', get_string('course'), ['limittoenrolled' => !$showall, 'exclude' => array_keys($coursesnogroup)]); $mform->hideIf('groupcourseid', 'eventtype', 'noteq', 'group'); diff --git a/calendar/lib.php b/calendar/lib.php index ebc81a87d39..981180202ee 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -3695,59 +3695,77 @@ function calendar_get_allowed_event_types(int $courseid = null) { // We still don't know if the user can create group and course events, so iterate over the courses to find out // if the user has capabilities in one of the courses. if ($types['course'] == false || $types['group'] == false) { - $courses = calendar_get_default_courses(null, 'id', true); + if ($CFG->calendar_adminseesall && has_capability('moodle/calendar:manageentries', context_system::instance())) { + $sql = "SELECT c.id, " . context_helper::get_preload_record_columns_sql('ctx') . " + FROM {course} c + JOIN {context} ctx ON ctx.contextlevel = ? AND ctx.instanceid = c.id + WHERE c.id IN ( + SELECT DISTINCT courseid FROM {groups} + )"; + $courseswithgroups = $DB->get_recordset_sql($sql, [CONTEXT_COURSE]); + foreach ($courseswithgroups as $course) { + context_helper::preload_from_record($course); + $context = context_course::instance($course->id); + + if (has_capability('moodle/calendar:manageentries', $context)) { + if (has_any_capability(['moodle/site:accessallgroups', 'moodle/calendar:managegroupentries'], $context)) { + // The user can manage group entries or access any group. + $types['group'] = true; + $types['course'] = true; + break; + } + } + } + $courseswithgroups->close(); + + if (false === $types['course']) { + // Course is still not confirmed. There may have been no courses with a group in them. + $ctxfields = context_helper::get_preload_record_columns_sql('ctx'); + $sql = "SELECT + c.id, c.visible, {$ctxfields} + FROM {course} + JOIN {context} ctx ON (ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel)"; + $params = [ + 'contextlevel' => CONTEXT_COURSE, + ]; + $courses = $DB->get_recordset_sql($sql, $params); + foreach ($courses as $course) { + context_helper::preload_from_record($course); + $context = context_course::instance($course->id); + if (has_capability('moodle/calendar:manageentries', $context)) { + $types['course'] = true; + break; + } + } + $courses->close(); + } + + } else { + $courses = calendar_get_default_courses(null, 'id'); + if (empty($courses)) { + return $types; + } - if (!empty($courses)) { $courseids = array_map(function($c) { return $c->id; }, $courses); - list($insql, $params) = $DB->get_in_or_equal($courseids); - $contextsql = "SELECT c.id, " . context_helper::get_preload_record_columns_sql('ctx') . " - FROM {course} c - JOIN {context} ctx ON ctx.contextlevel = ? AND ctx.instanceid = c.id - WHERE c.id $insql - GROUP BY c.id, ctx.id"; - array_unshift($params, CONTEXT_COURSE); - $contextrecords = $DB->get_records_sql($contextsql, $params); - foreach($courses as $course) { - context_helper::preload_from_record($contextrecords[$course->id]); - $coursecontext = context_course::instance($course->id); - if (has_capability('moodle/calendar:manageentries', $coursecontext) - && ($courseid == $course->id || empty($courseid))) { - $types['course'] = true; - break; - } - } - + list($insql, $params) = $DB->get_in_or_equal($courseids, SQL_PARAMS_NAMED); $sql = "SELECT c.id, " . context_helper::get_preload_record_columns_sql('ctx') . " FROM {course} c - JOIN {groups} g ON g.courseid = c.id - JOIN {context} ctx ON ctx.contextlevel = ? AND ctx.instanceid = c.id + JOIN {context} ctx ON ctx.contextlevel = :contextlevel AND ctx.instanceid = c.id WHERE c.id $insql - GROUP BY c.id, ctx.id - HAVING COUNT(g.id) > 0"; - $courseswithgroups = $DB->get_records_sql($sql, $params); + AND c.id IN (SELECT DISTINCT courseid FROM {groups})"; + $params['contextlevel'] = CONTEXT_COURSE; + $courseswithgroups = $DB->get_recordset_sql($sql, $params); foreach ($courseswithgroups as $coursewithgroup) { - $groups = groups_get_all_groups($coursewithgroup->id); context_helper::preload_from_record($coursewithgroup); $context = context_course::instance($coursewithgroup->id); if (has_capability('moodle/calendar:manageentries', $context)) { - if (!empty($groups)) { - $types['group'] = (!empty($groups) && has_capability('moodle/site:accessallgroups', - $context)) || array_filter($groups, function($group) use ($USER) { - return groups_is_member($group->id); - }); - } - } - - if (has_any_capability(['moodle/site:accessallgroups', 'moodle/calendar:managegroupentries'], - $context)) { - // The user can manage group entries or access any group. - if (!empty($groups)) { - $types['group'] = true; - } + $types['course'] = true; + $types['group'] = (!empty(groups_get_all_groups($coursewithgroup->id, $USER->id)) + && has_capability('moodle/calendar:managegroupentries', $context)); } // Okay, course and group event types are allowed, no need to keep the loop iteration. @@ -3755,6 +3773,29 @@ function calendar_get_allowed_event_types(int $courseid = null) { break; } } + $courseswithgroups->close(); + + if (false === $types['course']) { + list($insql, $params) = $DB->get_in_or_equal($courseids, SQL_PARAMS_NAMED); + $contextsql = "SELECT c.id, " . context_helper::get_preload_record_columns_sql('ctx') . " + FROM {course} c + JOIN {context} ctx ON ctx.contextlevel = :contextlevel AND ctx.instanceid = c.id + WHERE c.id $insql + GROUP BY c.id, ctx.id"; + $params['contextlevel'] = CONTEXT_COURSE; + $contextrecords = $DB->get_recordset_sql($contextsql, $params); + foreach ($contextrecords as $course) { + context_helper::preload_from_record($course); + $coursecontext = context_course::instance($course->id); + if (has_capability('moodle/calendar:manageentries', $coursecontext) + && ($courseid == $course->id || empty($courseid))) { + $types['course'] = true; + break; + } + } + $contextrecords->close(); + } + } }