diff --git a/admin/tool/dataprivacy/classes/output/data_registry_page.php b/admin/tool/dataprivacy/classes/output/data_registry_page.php index 7249adfc675ae..2f12f589ede91 100644 --- a/admin/tool/dataprivacy/classes/output/data_registry_page.php +++ b/admin/tool/dataprivacy/classes/output/data_registry_page.php @@ -234,10 +234,6 @@ public static function get_courses_branch(\context $catcontext) { $coursecontext = \context_course::instance($course->id); - if (!$course->visible && !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { - continue; - } - $coursenode = [ 'text' => shorten_text(format_string($course->shortname, true, ['context' => $coursecontext])), 'contextid' => $coursecontext->id, diff --git a/blocks/course_list/block_course_list.php b/blocks/course_list/block_course_list.php index 9d09dcbd18d99..b4aa2c1edaad9 100644 --- a/blocks/course_list/block_course_list.php +++ b/blocks/course_list/block_course_list.php @@ -54,6 +54,11 @@ function get_content() { } } + $allcourselink = + (has_capability('moodle/course:update', context_system::instance()) + || empty($CFG->block_course_list_hideallcourseslink)) && + core_course_category::user_top(); + if (empty($CFG->disablemycourses) and isloggedin() and !isguestuser() and !(has_capability('moodle/course:update', context_system::instance()) and $adminseesall)) { // Just print My Courses if ($courses = enrol_get_my_courses()) { @@ -65,7 +70,7 @@ function get_content() { } $this->title = get_string('mycourses'); /// If we can update any course of the view all isn't hidden, show the view all courses link - if (has_capability('moodle/course:update', context_system::instance()) || empty($CFG->block_course_list_hideallcourseslink)) { + if ($allcourselink) { $this->content->footer = "wwwroot/course/index.php\">".get_string("fulllistofcourses")." ..."; } } @@ -75,8 +80,9 @@ function get_content() { } } - $categories = core_course_category::get(0)->get_children(); // Parent = 0 ie top-level categories only - if ($categories) { //Check we have categories + // User is not enrolled in any courses, show list of available categories or courses (if there is only one category). + $topcategory = core_course_category::top(); + if ($topcategory->is_uservisible() && ($categories = $topcategory->get_children())) { // Check we have categories. if (count($categories) > 1 || (count($categories) == 1 && $DB->count_records('course') > 200)) { // Just print top level category links foreach ($categories as $category) { $categoryname = $category->get_formatted_name(); @@ -84,13 +90,13 @@ function get_content() { $this->content->items[]="wwwroot/course/index.php?categoryid=$category->id\">".$icon . $categoryname . ""; } /// If we can update any course of the view all isn't hidden, show the view all courses link - if (has_capability('moodle/course:update', context_system::instance()) || empty($CFG->block_course_list_hideallcourseslink)) { + if ($allcourselink) { $this->content->footer .= "wwwroot/course/index.php\">".get_string('fulllistofcourses').' ...'; } $this->title = get_string('categories'); } else { // Just print course names of single category $category = array_shift($categories); - $courses = get_courses($category->id); + $courses = $category->get_courses(); if ($courses) { foreach ($courses as $course) { @@ -98,12 +104,12 @@ function get_content() { $linkcss = $course->visible ? "" : " class=\"dimmed\" "; $this->content->items[]="shortname, true, array('context' => $coursecontext))."\" ". + . s($course->get_formatted_shortname())."\" ". "href=\"$CFG->wwwroot/course/view.php?id=$course->id\">" - .$icon. format_string(get_course_display_name_for_list($course), true, array('context' => context_course::instance($course->id))) . ""; + .$icon. $course->get_formatted_name() . ""; } /// If we can update any course of the view all isn't hidden, show the view all courses link - if (has_capability('moodle/course:update', context_system::instance()) || empty($CFG->block_course_list_hideallcourseslink)) { + if ($allcourselink) { $this->content->footer .= "wwwroot/course/index.php\">".get_string('fulllistofcourses').' ...'; } $this->get_remote_courses(); diff --git a/blocks/html/lib.php b/blocks/html/lib.php index a86b827efc732..5fc988ceba8c3 100644 --- a/blocks/html/lib.php +++ b/blocks/html/lib.php @@ -48,9 +48,8 @@ function block_html_pluginfile($course, $birecord_or_cm, $context, $filearea, $a $parentcontext = $context->get_parent_context(); if ($parentcontext->contextlevel === CONTEXT_COURSECAT) { // Check if category is visible and user can view this category. - $category = $DB->get_record('course_categories', array('id' => $parentcontext->instanceid), '*', MUST_EXIST); - if (!$category->visible) { - require_capability('moodle/category:viewhiddencategories', $parentcontext); + if (!core_course_category::get($parentcontext->instanceid, IGNORE_MISSING)) { + send_file_not_found(); } } else if ($parentcontext->contextlevel === CONTEXT_USER && $parentcontext->instanceid != $USER->id) { // The block is in the context of a user, it is only visible to the user who it belongs to. diff --git a/calendar/lib.php b/calendar/lib.php index 84b5ab3bfeeec..12bc80b6ecff2 100644 --- a/calendar/lib.php +++ b/calendar/lib.php @@ -2352,8 +2352,11 @@ function calendar_get_default_courses($courseid = null, $fields = '*', $canmanag $fieldlist = explode(',', $fields); $prefixedfields = array_map(function($value) { - return 'c.' . trim($value); + return 'c.' . trim(strtolower($value)); }, $fieldlist); + if (!in_array('c.visible', $prefixedfields) && !in_array('c.*', $prefixedfields)) { + $prefixedfields[] = 'c.visible'; + } $courses = get_courses('all', 'c.shortname', implode(',', $prefixedfields)); } else { $courses = enrol_get_users_courses($userid, true, $fields); diff --git a/calendar/view.php b/calendar/view.php index 3fb1509421828..96f39f649e928 100644 --- a/calendar/view.php +++ b/calendar/view.php @@ -83,6 +83,7 @@ if ($courseid != SITEID && !empty($courseid)) { navigation_node::override_active_url(new moodle_url('/course/view.php', array('id' => $course->id))); } else if (!empty($categoryid)) { + core_course_category::get($categoryid); // Check that category exists and can be accessed. $PAGE->set_category_by_id($categoryid); navigation_node::override_active_url(new moodle_url('/course/index.php', array('categoryid' => $categoryid))); } else { diff --git a/course/classes/category.php b/course/classes/category.php index c3658571c6902..fa5bc0cfa56ed 100644 --- a/course/classes/category.php +++ b/course/classes/category.php @@ -113,10 +113,7 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr protected $theme = false; /** @var bool */ - protected $fromcache; - - /** @var bool */ - protected $hasmanagecapability = null; + protected $fromcache = false; /** * Magic setter method, we do not want anybody to modify properties from the outside @@ -232,17 +229,17 @@ protected function __construct(stdClass $record, $fromcache = false) { */ public static function get($id, $strictness = MUST_EXIST, $alwaysreturnhidden = false, $user = null) { if (!$id) { - if (!isset(self::$coursecat0)) { - $record = new stdClass(); - $record->id = 0; - $record->visible = 1; - $record->depth = 0; - $record->path = ''; - $record->locked = 0; - self::$coursecat0 = new self($record); + // Top-level category. + if ($alwaysreturnhidden || self::top()->is_uservisible()) { + return self::top(); } - return self::$coursecat0; + if ($strictness == MUST_EXIST) { + throw new moodle_exception('cannotviewcategory'); + } + return null; } + + // Try to get category from cache or retrieve from the DB. $coursecatrecordcache = cache::make('core', 'coursecatrecords'); $coursecat = $coursecatrecordcache->get($id); if ($coursecat === false) { @@ -253,14 +250,66 @@ public static function get($id, $strictness = MUST_EXIST, $alwaysreturnhidden = $coursecatrecordcache->set($id, $coursecat); } } - if ($coursecat && ($alwaysreturnhidden || $coursecat->is_uservisible($user))) { - return $coursecat; - } else { + + if (!$coursecat) { + // Course category not found. if ($strictness == MUST_EXIST) { throw new moodle_exception('unknowncategory'); } + $coursecat = null; + } else if (!$alwaysreturnhidden && !$coursecat->is_uservisible($user)) { + // Course category is found but user can not access it. + if ($strictness == MUST_EXIST) { + throw new moodle_exception('cannotviewcategory'); + } + $coursecat = null; } - return null; + return $coursecat; + } + + /** + * Returns the pseudo-category representing the whole system (id=0, context_system) + * + * @return core_course_category + */ + public static function top() { + if (!isset(self::$coursecat0)) { + $record = new stdClass(); + $record->id = 0; + $record->visible = 1; + $record->depth = 0; + $record->path = ''; + $record->locked = 0; + self::$coursecat0 = new self($record); + } + return self::$coursecat0; + } + + /** + * Returns the top-most category for the current user + * + * Examples: + * 1. User can browse courses everywhere - return self::top() - pseudo-category with id=0 + * 2. User does not have capability to browse courses on the system level but + * has it in ONE course category - return this course category + * 3. User has capability to browse courses in two course categories - return self::top() + * + * @return core_course_category|null + */ + public static function user_top() { + $children = self::top()->get_children(); + if (count($children) == 1) { + // User has access to only one category on the top level. Return this category as "user top category". + return reset($children); + } + if (count($children) > 1) { + // User has access to more than one category on the top level. Return the top as "user top category". + // In this case user actually may not have capability 'moodle/course:browse' on the top level. + return self::top(); + } + // User can not access any categories on the top level. + // TODO MDL-10965 find ANY/ALL categories in the tree where user has access to. + return self::get(0, IGNORE_MISSING); } /** @@ -343,7 +392,7 @@ public static function get_all($options = []) { * @return core_course_category */ public static function get_default() { - if ($visiblechildren = self::get(0)->get_children()) { + if ($visiblechildren = self::top()->get_children()) { $defcategory = reset($visiblechildren); } else { $toplevelcategories = self::get_tree(0); @@ -358,6 +407,9 @@ public static function get_default() { * during {@link fix_course_sortorder()} */ protected function restore() { + if (!$this->id) { + return; + } // Update all fields in the current object. $newrecord = self::get($this->id, MUST_EXIST, true); foreach (self::$coursecatfields as $key => $unused) { @@ -419,7 +471,7 @@ public static function create($data, $editoroptions = null) { } if (empty($data->parent)) { - $parent = self::get(0); + $parent = self::top(); } else { $parent = self::get($data->parent, MUST_EXIST, true); } @@ -589,8 +641,48 @@ public function update($data, $editoroptions = null) { * @return bool */ public function is_uservisible($user = null) { - return !$this->id || $this->visible || - has_capability('moodle/category:viewhiddencategories', $this->get_context(), $user); + return self::can_view_category($this, $user); + } + + /** + * Checks if current user has access to the category + * + * @param stdClass|core_course_category $category + * @param int|stdClass $user The user id or object. By default (null) checks access for the current user. + * @return bool + */ + public static function can_view_category($category, $user = null) { + if (!$category->id) { + return has_capability('moodle/course:browse', context_system::instance(), $user); + } + $context = context_coursecat::instance($category->id); + if (!$category->visible && !has_capability('moodle/category:viewhiddencategories', $context, $user)) { + return false; + } + return has_capability('moodle/course:browse', $context, $user); + } + + /** + * Checks if current user can view course information or enrolment page. + * + * This method does not check if user is already enrolled in the course + * + * @param stdClass $course course object (must have 'id', 'visible' and 'category' fields) + * @param null|stdClass $user The user id or object. By default (null) checks access for the current user. + */ + public static function can_view_course_info($course, $user = null) { + if ($course->id == SITEID) { + return true; + } + if (!$course->visible) { + $coursecontext = context_course::instance($course->id); + if (!has_capability('moodle/course:viewhiddencourses', $coursecontext, $user)) { + return false; + } + } + $categorycontext = isset($course->category) ? context_coursecat::instance($course->category) : + context_course::instance($course->id)->get_parent_context(); + return has_capability('moodle/course:browse', $categorycontext, $user); } /** @@ -680,12 +772,29 @@ protected static function get_tree($id) { * Returns number of ALL categories in the system regardless if * they are visible to current user or not * + * @deprecated since Moodle 3.7 * @return int */ public static function count_all() { + debugging('Method core_course_category::count_all() is deprecated. Please use ' . + 'core_course_category::is_simple_site()', DEBUG_DEVELOPER); return self::get_tree('countall'); } + /** + * Checks if the site has only one category and it is visible and available. + * + * In many situations we won't show this category at all + * @return bool + */ + public static function is_simple_site() { + if (self::get_tree('countall') != 1) { + return false; + } + $default = self::get_default(); + return $default->visible && $default->is_uservisible(); + } + /** * Retrieves number of records from course_categories table * @@ -977,9 +1086,9 @@ protected static function ensure_users_enrolled($courseusers) { * * @param string $whereclause * @param array $params - * @param array $options may indicate that summary and/or coursecontacts need to be retrieved + * @param array $options may indicate that summary needs to be retrieved * @param bool $checkvisibility if true, capability 'moodle/course:viewhiddencourses' will be checked - * on not visible courses + * on not visible courses and 'moodle/course:browse' on all courses * @return array array of stdClass objects */ protected static function get_course_records($whereclause, $params, $options, $checkvisibility = false) { @@ -1002,29 +1111,21 @@ protected static function get_course_records($whereclause, $params, $options, $c array('contextcourse' => CONTEXT_COURSE) + $params); if ($checkvisibility) { + $mycourses = enrol_get_my_courses(); // Loop through all records and make sure we only return the courses accessible by user. foreach ($list as $course) { if (isset($list[$course->id]->hassummary)) { $list[$course->id]->hassummary = strlen($list[$course->id]->hassummary) > 0; } - if (empty($course->visible)) { - // Load context only if we need to check capability. - context_helper::preload_from_record($course); - if (!has_capability('moodle/course:viewhiddencourses', context_course::instance($course->id))) { - unset($list[$course->id]); - } + context_helper::preload_from_record($course); + $context = context_course::instance($course->id); + // Check that course is accessible by user. + if (!array_key_exists($course->id, $mycourses) && !self::can_view_course_info($course)) { + unset($list[$course->id]); } } } - // Preload course contacts if necessary. - if (!empty($options['coursecontacts'])) { - self::preload_course_contacts($list); - } - // Preload custom fields if necessary - saves DB queries later to do it for each course separately. - if (!empty($options['customfields'])) { - self::preload_custom_fields($list); - } return $list; } @@ -1041,10 +1142,11 @@ protected function get_not_visible_children_ids() { if (($invisibleids = $coursecatcache->get('ic'. $this->id)) === false) { // We never checked visible children before. $hidden = self::get_tree($this->id.'i'); + $catids = self::get_tree($this->id); $invisibleids = array(); - if ($hidden) { + if ($catids) { // Preload categories contexts. - list($sql, $params) = $DB->get_in_or_equal($hidden, SQL_PARAMS_NAMED, 'id'); + list($sql, $params) = $DB->get_in_or_equal($catids, SQL_PARAMS_NAMED, 'id'); $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); $contexts = $DB->get_records_sql("SELECT $ctxselect FROM {context} ctx WHERE ctx.contextlevel = :contextcoursecat AND ctx.instanceid ".$sql, @@ -1052,9 +1154,10 @@ protected function get_not_visible_children_ids() { foreach ($contexts as $record) { context_helper::preload_from_record($record); } - // Check that user has 'viewhiddencategories' capability for each hidden category. - foreach ($hidden as $id) { - if (!has_capability('moodle/category:viewhiddencategories', context_coursecat::instance($id))) { + // Check access for each category. + foreach ($catids as $id) { + $cat = (object)['id' => $id, 'visible' => in_array($id, $hidden) ? 0 : 1]; + if (!self::can_view_category($cat)) { $invisibleids[] = $id; } } @@ -1602,10 +1705,9 @@ public function get_courses($options = array()) { $limit = !empty($options['limit']) ? $options['limit'] : null; $sortfields = !empty($options['sort']) ? $options['sort'] : array('sortorder' => 1); - // Check if this category is hidden. - // Also 0-category never has courses unless this is recursive call. - if (!$this->is_uservisible() || (!$this->id && !$recursive)) { - return array(); + if (!$this->id && !$recursive) { + // There are no courses on system level unless we need recursive list. + return []; } $coursecatcache = cache::make('core', 'coursecat'); @@ -2402,7 +2504,9 @@ public static function make_categories_list($requiredcapability = '', $excludeid $thislist = preg_split('|,|', $thislist, -1, PREG_SPLIT_NO_EMPTY); } } else if ($baselist !== false) { - $thislist = array_keys($baselist); + $thislist = array_keys(array_filter($baselist, function($el) { + return $el['name'] !== false; + })); } if ($baselist === false) { @@ -2416,24 +2520,18 @@ public static function make_categories_list($requiredcapability = '', $excludeid $baselist = array(); $thislist = array(); foreach ($rs as $record) { - // If the category's parent is not visible to the user, it is not visible as well. - if (!$record->parent || isset($baselist[$record->parent])) { - context_helper::preload_from_record($record); - $context = context_coursecat::instance($record->id); - if (!$record->visible && !has_capability('moodle/category:viewhiddencategories', $context)) { - // No cap to view category, added to neither $baselist nor $thislist. - continue; - } - $baselist[$record->id] = array( - 'name' => format_string($record->name, true, array('context' => $context)), - 'path' => $record->path - ); - if (!empty($requiredcapability) && !has_all_capabilities($requiredcapability, $context)) { - // No required capability, added to $baselist but not to $thislist. - continue; - } - $thislist[] = $record->id; + context_helper::preload_from_record($record); + $context = context_coursecat::instance($record->id); + $canview = self::can_view_category($record); + $baselist[$record->id] = array( + 'name' => $canview ? format_string($record->name, true, array('context' => $context)) : false, + 'path' => $record->path + ); + if (!$canview || (!empty($requiredcapability) && !has_all_capabilities($requiredcapability, $context))) { + // No required capability, added to $baselist but not to $thislist. + continue; } + $thislist[] = $record->id; } $rs->close(); $coursecatcache->set($basecachekey, $baselist); @@ -2448,9 +2546,11 @@ public static function make_categories_list($requiredcapability = '', $excludeid $contexts = $DB->get_records_sql($sql, array('contextcoursecat' => CONTEXT_COURSECAT)); $thislist = array(); foreach (array_keys($baselist) as $id) { - context_helper::preload_from_record($contexts[$id]); - if (has_all_capabilities($requiredcapability, context_coursecat::instance($id))) { - $thislist[] = $id; + if ($baselist[$id]['name'] !== false) { + context_helper::preload_from_record($contexts[$id]); + if (has_all_capabilities($requiredcapability, context_coursecat::instance($id))) { + $thislist[] = $id; + } } } $coursecatcache->set($thiscachekey, join(',', $thislist)); @@ -2463,7 +2563,9 @@ public static function make_categories_list($requiredcapability = '', $excludeid if (!$excludeid || !in_array($excludeid, $path)) { $namechunks = array(); foreach ($path as $parentid) { - $namechunks[] = $baselist[$parentid]['name']; + if (array_key_exists($parentid, $baselist) && $baselist[$parentid]['name'] !== false) { + $namechunks[] = $baselist[$parentid]['name']; + } } $names[$id] = join($separator, $namechunks); } @@ -2529,7 +2631,7 @@ public static function wake_from_cache($a) { * @return bool */ public static function can_create_top_level_category() { - return has_capability('moodle/category:manage', context_system::instance()); + return self::top()->has_manage_capability(); } /** @@ -2550,10 +2652,10 @@ public function get_context() { * @return bool */ public function has_manage_capability() { - if ($this->hasmanagecapability === null) { - $this->hasmanagecapability = has_capability('moodle/category:manage', $this->get_context()); + if (!$this->is_uservisible()) { + return false; } - return $this->hasmanagecapability; + return has_capability('moodle/category:manage', $this->get_context()); } /** @@ -2561,7 +2663,7 @@ public function has_manage_capability() { * @return bool */ public function parent_has_manage_capability() { - return has_capability('moodle/category:manage', get_category_or_system_context($this->parent)); + return ($parent = $this->get_parent_coursecat()) && $parent->has_manage_capability(); } /** @@ -2595,7 +2697,7 @@ public function can_resort_courses() { * @return bool */ public function can_change_sortorder() { - return $this->id && $this->get_parent_coursecat()->can_resort_subcategories(); + return ($parent = $this->get_parent_coursecat()) && $parent->can_resort_subcategories(); } /** @@ -2603,7 +2705,7 @@ public function can_change_sortorder() { * @return bool */ public function can_create_course() { - return has_capability('moodle/course:create', $this->get_context()); + return $this->is_uservisible() && has_capability('moodle/course:create', $this->get_context()); } /** @@ -2619,7 +2721,7 @@ public function can_edit() { * @return bool */ public function can_review_roles() { - return has_capability('moodle/role:assign', $this->get_context()); + return $this->is_uservisible() && has_capability('moodle/role:assign', $this->get_context()); } /** @@ -2627,7 +2729,8 @@ public function can_review_roles() { * @return bool */ public function can_review_permissions() { - return has_any_capability(array( + return $this->is_uservisible() && + has_any_capability(array( 'moodle/role:assign', 'moodle/role:safeoverride', 'moodle/role:override', @@ -2640,7 +2743,8 @@ public function can_review_permissions() { * @return bool */ public function can_review_cohorts() { - return has_any_capability(array('moodle/cohort:view', 'moodle/cohort:manage'), $this->get_context()); + return $this->is_uservisible() && + has_any_capability(array('moodle/cohort:view', 'moodle/cohort:manage'), $this->get_context()); } /** @@ -2648,8 +2752,9 @@ public function can_review_cohorts() { * @return bool */ public function can_review_filters() { - return has_capability('moodle/filter:manage', $this->get_context()) && - count(filter_get_available_in_context($this->get_context())) > 0; + return $this->is_uservisible() && + has_capability('moodle/filter:manage', $this->get_context()) && + count(filter_get_available_in_context($this->get_context())) > 0; } /** @@ -2681,7 +2786,7 @@ public function can_move_courses_into() { * @return bool */ public function can_restore_courses_into() { - return has_capability('moodle/restore:restorecourse', $this->get_context()); + return $this->is_uservisible() && has_capability('moodle/restore:restorecourse', $this->get_context()); } /** @@ -2845,10 +2950,15 @@ public function change_sortorder_by_one($up) { /** * Returns the parent core_course_category object for this category. * - * @return core_course_category + * Only returns parent if it exists and is visible to the current user + * + * @return core_course_category|null */ public function get_parent_coursecat() { - return self::get($this->parent); + if (!$this->id) { + return null; + } + return self::get($this->parent, IGNORE_MISSING); } diff --git a/course/classes/list_element.php b/course/classes/list_element.php index 7d5eab4ce5abd..5acafff2053c0 100644 --- a/course/classes/list_element.php +++ b/course/classes/list_element.php @@ -415,14 +415,6 @@ public function get_context() { return context_course::instance($this->__get('id')); } - /** - * Returns true if this course is visible to the current user. - * @return bool - */ - public function is_uservisible() { - return $this->visible || has_capability('moodle/course:viewhiddencourses', $this->get_context()); - } - /** * Returns true if the current user can review enrolments for this course. * diff --git a/course/classes/management/helper.php b/course/classes/management/helper.php index 0befd6433dfa7..39c2c5e2929ae 100644 --- a/course/classes/management/helper.php +++ b/course/classes/management/helper.php @@ -403,12 +403,10 @@ public static function get_course_detail_actions(\core_course_list_element $cour $baseurl = new \moodle_url('/course/management.php', $params); $actions = array(); // View. - if ($course->is_uservisible()) { - $actions['view'] = array( - 'url' => new \moodle_url('/course/view.php', array('id' => $course->id)), - 'string' => \get_string('view') - ); - } + $actions['view'] = array( + 'url' => new \moodle_url('/course/view.php', array('id' => $course->id)), + 'string' => \get_string('view') + ); // Edit. if ($course->can_edit()) { $actions['edit'] = array( @@ -802,13 +800,14 @@ public static function search_courses($search, $blocklist, $modulelist, $page = $searchcriteria = array('modulelist' => $modulelist); } - $courses = \core_course_category::get(0)->search_courses($searchcriteria, array( + $topcat = \core_course_category::top(); + $courses = $topcat->search_courses($searchcriteria, array( 'recursive' => true, 'offset' => $page * $perpage, 'limit' => $perpage, 'sort' => array('fullname' => 1) )); - $totalcount = \core_course_category::get(0)->search_courses_count($searchcriteria, array('recursive' => true)); + $totalcount = $topcat->search_courses_count($searchcriteria, array('recursive' => true)); return array($courses, \count($courses), $totalcount); } diff --git a/course/classes/management_renderer.php b/course/classes/management_renderer.php index 83ba88a5d93b6..1f12b7f7146da 100644 --- a/course/classes/management_renderer.php +++ b/course/classes/management_renderer.php @@ -136,7 +136,7 @@ public function category_listing(core_course_category $category = null) { $catatlevel[] = array_shift($selectedparents); $catatlevel = array_unique($catatlevel); - $listing = core_course_category::get(0)->get_children(); + $listing = core_course_category::top()->get_children(); $attributes = array( 'class' => 'ml-1 list-unstyled', @@ -200,10 +200,10 @@ public function category_listitem(core_course_category $category, array $subcate 'aria-expanded' => $isexpanded ? 'true' : 'false' ); $text = $category->get_formatted_name(); - if ($category->parent) { + if (($parent = $category->get_parent_coursecat()) && $parent->id) { $a = new stdClass; $a->category = $text; - $a->parentcategory = $category->get_parent_coursecat()->get_formatted_name(); + $a->parentcategory = $parent->get_formatted_name(); $textlabel = get_string('categorysubcategoryof', 'moodle', $a); } $courseicon = $this->output->pix_icon('i/course', get_string('courses')); @@ -327,7 +327,7 @@ public function category_listing_actions(core_course_category $category = null) $cancreatecategory = $category && $category->can_create_subcategory(); $cancreatecategory = $cancreatecategory || core_course_category::can_create_top_level_category(); if ($category === null) { - $category = core_course_category::get(0); + $category = core_course_category::top(); } if ($cancreatecategory) { @@ -459,8 +459,8 @@ public function category_bulk_actions(core_course_category $category = null) { } if (core_course_category::can_change_parent_any()) { $options = array(); - if (has_capability('moodle/category:manage', context_system::instance())) { - $options[0] = core_course_category::get(0)->get_formatted_name(); + if (core_course_category::top()->has_manage_capability()) { + $options[0] = core_course_category::top()->get_formatted_name(); } $options += core_course_category::make_categories_list('moodle/category:manage'); $select = html_writer::select( diff --git a/course/classes/search/course.php b/course/classes/search/course.php index 27ba20189c90d..cce89ec3430c4 100644 --- a/course/classes/search/course.php +++ b/course/classes/search/course.php @@ -113,9 +113,7 @@ public function check_access($id) { return \core_search\manager::ACCESS_DELETED; } - $coursecontext = \context_course::instance($course->id); - - if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + if (\core_course_category::can_view_course_info($course)) { return \core_search\manager::ACCESS_GRANTED; } diff --git a/course/classes/search/customfield.php b/course/classes/search/customfield.php index 315281e6be55d..42bcf3d243236 100644 --- a/course/classes/search/customfield.php +++ b/course/classes/search/customfield.php @@ -135,8 +135,7 @@ public function check_access($id) { if (!$course) { return \core_search\manager::ACCESS_DELETED; } - $coursecontext = \context_course::instance($course->id); - if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + if (\core_course_category::can_view_course_info($course)) { return \core_search\manager::ACCESS_GRANTED; } return \core_search\manager::ACCESS_DENIED; diff --git a/course/externallib.php b/course/externallib.php index 97a42c233b716..1d8840fd68363 100644 --- a/course/externallib.php +++ b/course/externallib.php @@ -1870,7 +1870,7 @@ public static function get_categories($criteria = array(), $addsubcategories = t if (!isset($excludedcats[$category->id])) { // Final check to see if the category is visible to the user. - if ($category->visible or has_capability('moodle/category:viewhiddencategories', $context)) { + if (core_course_category::can_view_category($category)) { $categoryinfo = array(); $categoryinfo['id'] = $category->id; @@ -3235,14 +3235,18 @@ public static function get_courses_by_field($field = '', $value = '') { } // Get the public course information, even if we are not enrolled. $courseinlist = new core_course_list_element($course); - $coursesdata[$course->id] = self::get_course_public_information($courseinlist, $context); // Now, check if we have access to the course. try { self::validate_context($context); } catch (Exception $e) { + // User can not access the course, check if they can see the public information about the course and return it. + if (core_course_category::can_view_course_info($course)) { + $coursesdata[$course->id] = self::get_course_public_information($courseinlist, $context); + } continue; } + $coursesdata[$course->id] = self::get_course_public_information($courseinlist, $context); // Return information for any user that can access the course. $coursefields = array('format', 'showgrades', 'newsitems', 'startdate', 'enddate', 'maxbytes', 'showreports', 'visible', 'groupmode', 'groupmodeforce', 'defaultgroupingid', 'enablecompletion', 'completionnotify', 'lang', 'theme', diff --git a/course/index.php b/course/index.php index 536399ed92b82..28f66912aa3d6 100644 --- a/course/index.php +++ b/course/index.php @@ -29,39 +29,40 @@ $categoryid = optional_param('categoryid', 0, PARAM_INT); // Category id $site = get_site(); +if ($CFG->forcelogin) { + require_login(); +} + +$heading = $site->fullname; if ($categoryid) { + $category = core_course_category::get($categoryid); // This will validate access. $PAGE->set_category_by_id($categoryid); $PAGE->set_url(new moodle_url('/course/index.php', array('categoryid' => $categoryid))); $PAGE->set_pagetype('course-index-category'); - // And the object has been loaded for us no need for another DB call - $category = $PAGE->category; -} else { - // Check if there is only one category, if so use that. - if (core_course_category::count_all() == 1) { - $category = core_course_category::get_default(); - - $categoryid = $category->id; + $heading = $category->get_formatted_name(); +} else if ($category = core_course_category::user_top()) { + // Check if there is only one top-level category, if so use that. + $categoryid = $category->id; + $PAGE->set_url('/course/index.php'); + if ($category->is_uservisible() && $categoryid) { $PAGE->set_category_by_id($categoryid); - $PAGE->set_pagetype('course-index-category'); + $PAGE->set_context($category->get_context()); + if (!core_course_category::is_simple_site()) { + $PAGE->set_url(new moodle_url('/course/index.php', array('categoryid' => $categoryid))); + $heading = $category->get_formatted_name(); + } } else { $PAGE->set_context(context_system::instance()); } - - $PAGE->set_url('/course/index.php'); + $PAGE->set_pagetype('course-index-category'); +} else { + throw new moodle_exception('cannotviewcategory'); } $PAGE->set_pagelayout('coursecategory'); $courserenderer = $PAGE->get_renderer('core', 'course'); -if ($CFG->forcelogin) { - require_login(); -} - -if ($categoryid && !$category->visible && !has_capability('moodle/category:viewhiddencategories', $PAGE->context)) { - throw new moodle_exception('unknowncategory'); -} - -$PAGE->set_heading($site->fullname); +$PAGE->set_heading($heading); $content = $courserenderer->course_category($categoryid); echo $OUTPUT->header(); diff --git a/course/info.php b/course/info.php index 3eae7433ecaf3..6cb73c03ae126 100644 --- a/course/info.php +++ b/course/info.php @@ -48,8 +48,8 @@ } $context = context_course::instance($course->id); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) { - print_error('coursehidden', '', $CFG->wwwroot .'/'); + if (!core_course_category::can_view_course_info($course) && !is_enrolled($context, null, '', true)) { + print_error('cannotviewcategory', '', $CFG->wwwroot .'/'); } $PAGE->set_course($course); diff --git a/course/lib.php b/course/lib.php index 380a7625fd0b0..25af452c7fab7 100644 --- a/course/lib.php +++ b/course/lib.php @@ -762,7 +762,7 @@ function print_course_request_buttons($context) { echo $OUTPUT->single_button(new moodle_url('/course/request.php'), get_string('requestcourse'), 'get'); } /// Print a button to manage pending requests - if ($context->contextlevel == CONTEXT_SYSTEM && has_capability('moodle/site:approvecourse', $context)) { + if (has_capability('moodle/site:approvecourse', $context)) { $disabled = !$DB->record_exists('course_request', array()); echo $OUTPUT->single_button(new moodle_url('/course/pending.php'), get_string('coursespending'), 'get', array('disabled' => $disabled)); } diff --git a/course/management.php b/course/management.php index e104ac0163956..27784d152afdc 100644 --- a/course/management.php +++ b/course/management.php @@ -68,8 +68,9 @@ } else { $course = null; $courseid = null; - $category = core_course_category::get_default(); - $categoryid = $category->id; + $topchildren = core_course_category::top()->get_children(); + $category = reset($topchildren); + $categoryid = $category ? $category->id : 0; $context = context_coursecat::instance($category->id); $url->param('categoryid', $category->id); } @@ -378,9 +379,9 @@ } $categories = core_course_category::get_many($categoryids); } else if ($for === 'allcategories') { - if ($sortcategoriesby && core_course_category::get(0)->can_resort_subcategories()) { + if ($sortcategoriesby && core_course_category::top()->can_resort_subcategories()) { \core_course\management\helper::action_category_resort_subcategories( - core_course_category::get(0), $sortcategoriesby); + core_course_category::top(), $sortcategoriesby); } $categorieslist = core_course_category::make_categories_list('moodle/category:manage'); $categoryids = array_keys($categorieslist); diff --git a/course/renderer.php b/course/renderer.php index 186ff5cee1762..55f5ebed0b76a 100644 --- a/course/renderer.php +++ b/course/renderer.php @@ -1582,35 +1582,38 @@ protected function coursecat_tree(coursecat_helper $chelper, $coursecat) { */ public function course_category($category) { global $CFG; - $coursecat = core_course_category::get(is_object($category) ? $category->id : $category); + $usertop = core_course_category::user_top(); + if (empty($category)) { + $coursecat = $usertop; + } else if (is_object($category) && $category instanceof core_course_category) { + $coursecat = $category; + } else { + $coursecat = core_course_category::get(is_object($category) ? $category->id : $category); + } $site = get_site(); $output = ''; - if (can_edit_in_category($coursecat->id)) { + if ($coursecat->can_create_course() || $coursecat->has_manage_capability()) { // Add 'Manage' button if user has permissions to edit this category. $managebutton = $this->single_button(new moodle_url('/course/management.php', array('categoryid' => $coursecat->id)), get_string('managecourses'), 'get'); $this->page->set_button($managebutton); } - if (!$coursecat->id) { - if (core_course_category::count_all() == 1) { - // There exists only one category in the system, do not display link to it - $coursecat = core_course_category::get_default(); - $strfulllistofcourses = get_string('fulllistofcourses'); - $this->page->set_title("$site->shortname: $strfulllistofcourses"); - } else { - $strcategories = get_string('categories'); - $this->page->set_title("$site->shortname: $strcategories"); - } + + if (core_course_category::is_simple_site()) { + // There is only one category in the system, do not display link to it. + $strfulllistofcourses = get_string('fulllistofcourses'); + $this->page->set_title("$site->shortname: $strfulllistofcourses"); + } else if (!$coursecat->id || !$coursecat->is_uservisible()) { + $strcategories = get_string('categories'); + $this->page->set_title("$site->shortname: $strcategories"); } else { - $title = $site->shortname; - if (core_course_category::count_all() > 1) { - $title .= ": ". $coursecat->get_formatted_name(); - } - $this->page->set_title($title); + $strfulllistofcourses = get_string('fulllistofcourses'); + $this->page->set_title("$site->shortname: $strfulllistofcourses"); // Print the category selector - if (core_course_category::count_all() > 1) { + $categorieslist = core_course_category::make_categories_list(); + if (count($categorieslist) > 1) { $output .= html_writer::start_tag('div', array('class' => 'categorypicker')); $select = new single_select(new moodle_url('/course/index.php'), 'categoryid', core_course_category::make_categories_list(), $coursecat->id, null, 'switchcategory'); @@ -1644,13 +1647,13 @@ public function course_category($category) { } $coursedisplayoptions['limit'] = $perpage; $catdisplayoptions['limit'] = $perpage; - if ($browse === 'courses' || !$coursecat->has_children()) { + if ($browse === 'courses' || !$coursecat->get_children_count()) { $coursedisplayoptions['offset'] = $page * $perpage; $coursedisplayoptions['paginationurl'] = new moodle_url($baseurl, array('browse' => 'courses')); $catdisplayoptions['nodisplay'] = true; $catdisplayoptions['viewmoreurl'] = new moodle_url($baseurl, array('browse' => 'categories')); $catdisplayoptions['viewmoretext'] = new lang_string('viewallsubcategories'); - } else if ($browse === 'categories' || !$coursecat->has_courses()) { + } else if ($browse === 'categories' || !$coursecat->get_courses_count()) { $coursedisplayoptions['nodisplay'] = true; $catdisplayoptions['offset'] = $page * $perpage; $catdisplayoptions['paginationurl'] = new moodle_url($baseurl, array('browse' => 'categories')); @@ -1670,24 +1673,23 @@ public function course_category($category) { // Add action buttons $output .= $this->container_start('buttons'); - $context = get_category_or_system_context($coursecat->id); - if (has_capability('moodle/course:create', $context)) { - // Print link to create a new course, for the 1st available category. - if ($coursecat->id) { - $url = new moodle_url('/course/edit.php', array('category' => $coursecat->id, 'returnto' => 'category')); - } else { - $url = new moodle_url('/course/edit.php', array('category' => $CFG->defaultrequestcategory, 'returnto' => 'topcat')); + if ($coursecat->is_uservisible()) { + $context = get_category_or_system_context($coursecat->id); + if (has_capability('moodle/course:create', $context)) { + // Print link to create a new course, for the 1st available category. + if ($coursecat->id) { + $url = new moodle_url('/course/edit.php', array('category' => $coursecat->id, 'returnto' => 'category')); + } else { + $url = new moodle_url('/course/edit.php', + array('category' => $CFG->defaultrequestcategory, 'returnto' => 'topcat')); + } + $output .= $this->single_button($url, get_string('addnewcourse'), 'get'); } - $output .= $this->single_button($url, get_string('addnewcourse'), 'get'); - } - ob_start(); - if (core_course_category::count_all() == 1) { - print_course_request_buttons(context_system::instance()); - } else { + ob_start(); print_course_request_buttons($context); + $output .= ob_get_contents(); + ob_end_clean(); } - $output .= ob_get_contents(); - ob_end_clean(); $output .= $this->container_end(); return $output; @@ -1828,7 +1830,7 @@ public function tagged_courses($tagid, $exclusivemode = true, $ctx = 0, $rec = t if (empty($displayoptions)) { $displayoptions = array(); } - $showcategories = core_course_category::count_all() > 1; + $showcategories = !core_course_category::is_simple_site(); $displayoptions += array('limit' => $CFG->coursesperpage, 'offset' => 0); $chelper = new coursecat_helper(); $searchcriteria = array('tagid' => $tagid, 'ctx' => $ctx, 'rec' => $rec); @@ -1950,15 +1952,15 @@ public function frontpage_my_courses() { if (!empty($courses) || !empty($rcourses) || !empty($rhosts)) { $chelper = new coursecat_helper(); + $totalcount = count($courses); if (count($courses) > $CFG->frontpagecourselimit) { // There are more enrolled courses than we can display, display link to 'My courses'. - $totalcount = count($courses); $courses = array_slice($courses, 0, $CFG->frontpagecourselimit, true); $chelper->set_courses_display_options(array( 'viewmoreurl' => new moodle_url('/my/'), 'viewmoretext' => new lang_string('mycourses') )); - } else { + } else if (core_course_category::top()->is_uservisible()) { // All enrolled courses are displayed, display link to 'All courses' if there are more courses in system. $chelper->set_courses_display_options(array( 'viewmoreurl' => new moodle_url('/course/index.php'), @@ -2007,8 +2009,8 @@ public function frontpage_available_courses() { 'viewmoretext' => new lang_string('fulllistofcourses'))); $chelper->set_attributes(array('class' => 'frontpage-course-list-all')); - $courses = core_course_category::get(0)->get_courses($chelper->get_courses_display_options()); - $totalcount = core_course_category::get(0)->get_courses_count($chelper->get_courses_display_options()); + $courses = core_course_category::top()->get_courses($chelper->get_courses_display_options()); + $totalcount = core_course_category::top()->get_courses_count($chelper->get_courses_display_options()); if (!$totalcount && !$this->page->user_is_editing() && has_capability('moodle/course:create', context_system::instance())) { // Print link to create a new course, for the 1st available category. return $this->add_new_course_button(); @@ -2038,6 +2040,11 @@ public function add_new_course_button() { */ public function frontpage_combo_list() { global $CFG; + // TODO MDL-10965 improve. + $tree = core_course_category::top(); + if (!$tree->get_children_count()) { + return ''; + } $chelper = new coursecat_helper(); $chelper->set_subcat_depth($CFG->maxcategorydepth)-> set_categories_display_options(array( @@ -2051,7 +2058,7 @@ public function frontpage_combo_list() { array('browse' => 'courses', 'page' => 1)) ))-> set_attributes(array('class' => 'frontpage-category-combo')); - return $this->coursecat_tree($chelper, core_course_category::get(0)); + return $this->coursecat_tree($chelper, $tree); } /** @@ -2061,6 +2068,11 @@ public function frontpage_combo_list() { */ public function frontpage_categories_list() { global $CFG; + // TODO MDL-10965 improve. + $tree = core_course_category::top(); + if (!$tree->get_children_count()) { + return ''; + } $chelper = new coursecat_helper(); $chelper->set_subcat_depth($CFG->maxcategorydepth)-> set_show_courses(self::COURSECAT_SHOW_COURSES_COUNT)-> @@ -2070,7 +2082,7 @@ public function frontpage_categories_list() { array('browse' => 'categories', 'page' => 1)) ))-> set_attributes(array('class' => 'frontpage-category-names')); - return $this->coursecat_tree($chelper, core_course_category::get(0)); + return $this->coursecat_tree($chelper, $tree); } /** @@ -2379,6 +2391,9 @@ protected function frontpage_news($forum) { * @return string */ protected function frontpage_part($skipdivid, $contentsdivid, $header, $contents) { + if (strval($contents) === '') { + return ''; + } $output = html_writer::link('#' . $skipdivid, get_string('skipa', 'access', core_text::strtolower(strip_tags($header))), array('class' => 'skip-block skip')); @@ -2438,10 +2453,8 @@ public function frontpage() { case FRONTPAGEALLCOURSELIST: $availablecourseshtml = $this->frontpage_available_courses(); - if (!empty($availablecourseshtml)) { - $output .= $this->frontpage_part('skipavailablecourses', 'frontpage-available-course-list', - get_string('availablecourses'), $availablecourseshtml); - } + $output .= $this->frontpage_part('skipavailablecourses', 'frontpage-available-course-list', + get_string('availablecourses'), $availablecourseshtml); break; case FRONTPAGECATEGORYNAMES: @@ -2705,7 +2718,7 @@ public function set_search_criteria($searchcriteria) { * @return string|null */ public function get_category_formatted_description($coursecat, $options = null) { - if ($coursecat->id && !empty($coursecat->description)) { + if ($coursecat->id && $coursecat->is_uservisible() && !empty($coursecat->description)) { if (!isset($coursecat->descriptionformat)) { $descriptionformat = FORMAT_MOODLE; } else { diff --git a/course/search.php b/course/search.php index e02bd24060809..9dcd11292a2b1 100644 --- a/course/search.php +++ b/course/search.php @@ -71,7 +71,8 @@ $strsearchresults = new lang_string("searchresults"); $strnovalidcourses = new lang_string('novalidcourses'); -$PAGE->navbar->add($strcourses, new moodle_url('/course/index.php')); +$courseurl = core_course_category::user_top() ? new moodle_url('/course/index.php') : null; +$PAGE->navbar->add($strcourses, $courseurl); $PAGE->navbar->add($strsearch, new moodle_url('/course/search.php')); if (!empty($search)) { $PAGE->navbar->add(s($search)); diff --git a/course/tests/behat/course_browsing.feature b/course/tests/behat/course_browsing.feature new file mode 100644 index 0000000000000..f55e58fc6d4d4 --- /dev/null +++ b/course/tests/behat/course_browsing.feature @@ -0,0 +1,106 @@ +@core @core_course +Feature: Restricting access to course lists + In order to provide more targeted content + As a Moodle Administrator + I need to be able to give/revoke capabilities to view list of courses + + Background: + Given the following "categories" exist: + | name | category | idnumber | + | Science category | 0 | SCI | + | English category | 0 | ENG | + | Other category | 0 | MISC | + And the following "courses" exist: + | fullname | shortname | category | + | Biology Y1 | BIO1 | SCI | + | Biology Y2 | BI02 | SCI | + | English Y1 | ENG1 | ENG | + | English Y2 | ENG2 | ENG | + | Humanities Y1 | HUM2 | MISC | + Given the following "users" exist: + | username | firstname | lastname | email | + | user0 | User | Z | user0@example.com | + | userb | User | B | userb@example.com | + | usere | User | E | usere@example.com | + Given the following "roles" exist: + | name | shortname | description | archetype | + | Category viewer | coursebrowse | My custom role 1 | | + Given I log in as "admin" + And I set the following system permissions of "Authenticated user" role: + | capability | permission | + | moodle/course:browse | Prevent | + And I set the following system permissions of "Guest" role: + | capability | permission | + | moodle/course:browse | Prevent | + And I set the following system permissions of "Category viewer" role: + | capability | permission | + | moodle/course:browse | Allow | + And I am on site homepage + And I turn editing mode on + And I add the "Navigation" block if not present + And I log out + And the following "role assigns" exist: + | user | role | contextlevel | reference | + | usere | coursebrowse | Category | ENG | + | userb | coursebrowse | Category | ENG | + | userb | coursebrowse | Category | SCI | + + Scenario: Browse courses as a user without any browse capability + When I log in as "user0" + And I am on site homepage + Then I should not see "Available courses" + And "Courses" "link" should not exist in the "Navigation" "block" + And I log out + + Scenario: Browse own courses as a user without any browse capability + Given the following "course enrolments" exist: + | user | course | role | + | user0 | BIO1 | student | + When I log in as "user0" + And I am on site homepage + And I should see "Available courses" + And I should see "Biology Y1" + And "Courses" "link" should not exist in the "Navigation" "block" + And I log out + + Scenario: Browse courses as a user who has access to only one category + When I log in as "usere" + And I am on site homepage + Then I should see "Available courses" + And I should see "English Y1" + And I should see "English Y2" + And I should not see "Biology" + And I should not see "Humanities" + And I click on "Courses" "link" in the "Navigation" "block" + And "English category" "text" should exist in the ".breadcrumb" "css_element" + And I should see "English Y1" + And I should see "English Y2" + And I should not see "Biology" + And I should not see "Humanities" + And I should not see "Other category" + And I follow "English Y2" + And I should see "You can not enrol yourself in this course." + And I log out + + Scenario: Browse courses as a user who has access to several but not all categories + When I log in as "userb" + And I am on site homepage + Then I should see "Available courses" + And I should see "English Y1" + And I should see "English Y2" + And I should see "Biology" + And I should not see "Humanities" + And I click on "Courses" "link" in the "Navigation" "block" + And "category" "text" should not exist in the ".breadcrumb" "css_element" + And I should see "Science category" + And I should see "English category" + And I should not see "Other category" + And I follow "Science category" + And I should see "Biology Y2" + And I should not see "English Y1" + And the "Course categories" select box should contain "Science category" + And the "Course categories" select box should contain "English category" + And the "Course categories" select box should not contain "Other category" + And I follow "Biology Y1" + And I should see "You can not enrol yourself in this course." + And I log out diff --git a/course/tests/category_test.php b/course/tests/category_test.php index 1dcd1efc44afc..54899398c730e 100644 --- a/course/tests/category_test.php +++ b/course/tests/category_test.php @@ -418,14 +418,37 @@ public function test_count_all() { // Dont assume there is just one. An add-on might create a category as part of the install. $numcategories = $DB->count_records('course_categories'); $this->assertEquals($numcategories, core_course_category::count_all()); + $this->assertDebuggingCalled('Method core_course_category::count_all() is deprecated. Please use ' . + 'core_course_category::is_simple_site()', DEBUG_DEVELOPER); $category1 = core_course_category::create(array('name' => 'Cat1')); $category2 = core_course_category::create(array('name' => 'Cat2', 'parent' => $category1->id)); $category3 = core_course_category::create(array('name' => 'Cat3', 'parent' => $category2->id, 'visible' => 0)); // Now we've got three more. $this->assertEquals($numcategories + 3, core_course_category::count_all()); + $this->assertDebuggingCalled('Method core_course_category::count_all() is deprecated. Please use ' . + 'core_course_category::is_simple_site()', DEBUG_DEVELOPER); cache_helper::purge_by_event('changesincoursecat'); // We should still have 4. $this->assertEquals($numcategories + 3, core_course_category::count_all()); + $this->assertDebuggingCalled('Method core_course_category::count_all() is deprecated. Please use ' . + 'core_course_category::is_simple_site()', DEBUG_DEVELOPER); + } + + /** + * Test the is_simple_site function + */ + public function test_is_simple_site() { + // By default site has one category and is considered simple. + $this->assertEquals(true, core_course_category::is_simple_site()); + $default = core_course_category::get_default(); + // When there is only one category but it is hidden, it is not a simple site. + $default->update(['visible' => 0]); + $this->assertEquals(false, core_course_category::is_simple_site()); + $default->update(['visible' => 1]); + $this->assertEquals(true, core_course_category::is_simple_site()); + // As soon as there is more than one category, site is not simple any more. + core_course_category::create(array('name' => 'Cat1')); + $this->assertEquals(false, core_course_category::is_simple_site()); } /** @@ -1007,7 +1030,7 @@ public function test_current_user_coursecat_get() { // Expecting to get an exception as this new user does not have the moodle/category:viewhiddencategories capability. $this->expectException('moodle_exception'); - $this->expectExceptionMessage('unknowncategory'); + $this->expectExceptionMessage(get_string('cannotviewcategory', 'error')); core_course_category::get($category2->id); } @@ -1033,7 +1056,7 @@ public function test_another_user_coursecat_get() { $this->assertEquals($category1->id, core_course_category::get($category1->id, MUST_EXIST, false, $user2)->id); $this->expectException('moodle_exception'); - $this->expectExceptionMessage('unknowncategory'); + $this->expectExceptionMessage(get_string('cannotviewcategory', 'error')); core_course_category::get($category2->id, MUST_EXIST, false, $user2); } diff --git a/enrol/externallib.php b/enrol/externallib.php index 9ba48d2131376..71608dfdb3682 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -876,8 +876,7 @@ public static function get_course_enrolment_methods($courseid) { self::validate_context(context_system::instance()); $course = $DB->get_record('course', array('id' => $params['courseid']), '*', MUST_EXIST); - $context = context_course::instance($course->id); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) { + if (!core_course_category::can_view_course_info($course) && !can_access_course($course)) { throw new moodle_exception('coursehidden'); } diff --git a/enrol/guest/classes/external.php b/enrol/guest/classes/external.php index 656c29580a7df..65850b934888e 100644 --- a/enrol/guest/classes/external.php +++ b/enrol/guest/classes/external.php @@ -75,8 +75,7 @@ public static function get_instance_info($instanceid) { $enrolinstance = $DB->get_record('enrol', array('id' => $params['instanceid']), '*', MUST_EXIST); $course = $DB->get_record('course', array('id' => $enrolinstance->courseid), '*', MUST_EXIST); - $context = context_course::instance($course->id); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) { + if (!core_course_category::can_view_course_info($course) && !can_access_course($course)) { throw new moodle_exception('coursehidden'); } diff --git a/enrol/index.php b/enrol/index.php index dcc4d9639b9df..5a97c12ff51b3 100644 --- a/enrol/index.php +++ b/enrol/index.php @@ -61,6 +61,11 @@ print_error('loginasnoenrol', '', $CFG->wwwroot.'/course/view.php?id='.$USER->loginascontext->instanceid); } +// Check if user has access to the category where the course is located. +if (!core_course_category::can_view_course_info($course) && !is_enrolled($context, $USER, '', true)) { + print_error('coursehidden', '', $CFG->wwwroot . '/'); +} + // get all enrol forms available in this course $enrols = enrol_get_plugins(true); $enrolinstances = enrol_get_instances($course->id, true); diff --git a/enrol/self/externallib.php b/enrol/self/externallib.php index 9006e62f136b9..5a2270eb6107c 100644 --- a/enrol/self/externallib.php +++ b/enrol/self/externallib.php @@ -71,8 +71,7 @@ public static function get_instance_info($instanceid) { $enrolinstance = $DB->get_record('enrol', array('id' => $params['instanceid']), '*', MUST_EXIST); $course = $DB->get_record('course', array('id' => $enrolinstance->courseid), '*', MUST_EXIST); - $context = context_course::instance($course->id); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) { + if (!core_course_category::can_view_course_info($course) && !can_access_course($course)) { throw new moodle_exception('coursehidden'); } @@ -147,7 +146,7 @@ public static function enrol_user($courseid, $password = '', $instanceid = 0) { $context = context_course::instance($course->id); self::validate_context(context_system::instance()); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $context)) { + if (!core_course_category::can_view_course_info($course)) { throw new moodle_exception('coursehidden'); } diff --git a/lang/en/error.php b/lang/en/error.php index d6c2df6bc1802..ef172e246ae4f 100644 --- a/lang/en/error.php +++ b/lang/en/error.php @@ -169,6 +169,7 @@ $string['cannotuseadminadminorteacher'] = 'You need to be a teacher or admin user to use this page'; $string['cannotusepage'] = 'Only teachers and administrators can use this page'; $string['cannotusepage2'] = 'Sorry, you may not use this page'; +$string['cannotviewcategory'] = 'You don\'t have permission to view courses here'; $string['cannotviewprofile'] = 'You cannot view the profile of this user'; $string['cannotviewreport'] = 'You cannot view this report'; $string['cannotwritefile'] = 'Cannot write to file ({$a})'; diff --git a/lang/en/moodle.php b/lang/en/moodle.php index f531b32d335b6..b72f29cbf3bca 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -232,6 +232,7 @@ $string['categorycurrentcontents'] = 'Contents of {$a}'; $string['categorydeleted'] = 'The category \'{$a}\' was deleted'; $string['categoryduplicate'] = 'A category named \'{$a}\' already exists!'; +$string['categoryhidden'] = '(hidden)'; $string['categorymodifiedcancel'] = 'Category was modified! Please cancel and try again.'; $string['categoryname'] = 'Category name'; $string['categorysubcategoryof'] = '{$a->category} - subcategory of {$a->parentcategory}'; diff --git a/lang/en/role.php b/lang/en/role.php index ef271e8b7ce2e..13b4c1bb48e12 100644 --- a/lang/en/role.php +++ b/lang/en/role.php @@ -154,6 +154,7 @@ $string['confirmunassignno'] = 'Cancel'; $string['context'] = 'Context'; $string['course:activityvisibility'] = 'Hide/show activities'; +$string['course:browse'] = 'View list of courses where user is not enrolled'; $string['course:bulkmessaging'] = 'Send a message to many people'; $string['course:create'] = 'Create courses'; $string['course:creategroupconversations'] = 'Create group conversations'; diff --git a/lib/accesslib.php b/lib/accesslib.php index 238feaf144922..e80ba19c445f0 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -1948,6 +1948,11 @@ function can_access_course(stdClass $course, $user = null, $withcapability = '', return true; } + if (!core_course_category::can_view_course_info($course)) { + // No guest access if user does not have capability to browse courses. + return false; + } + // if not enrolled try to gain temporary guest access $instances = $DB->get_records('enrol', array('courseid'=>$course->id, 'status'=>ENROL_INSTANCE_ENABLED), 'sortorder, id ASC'); $enrols = enrol_get_plugins(true); diff --git a/lib/datalib.php b/lib/datalib.php index ad64468ad48cd..5a5309811b323 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -597,6 +597,9 @@ function get_course($courseid, $clone = true) { * we are using distinct. You almost _NEVER_ need all the fields * in such a large SELECT * + * Consider using core_course_category::get_courses() + * or core_course_category::search_courses() instead since they use caching. + * * @global object * @global object * @global object @@ -643,12 +646,7 @@ function get_courses($categoryid="all", $sort="c.sortorder ASC", $fields="c.*") // loop throught them foreach ($courses as $course) { context_helper::preload_from_record($course); - if (isset($course->visible) && $course->visible <= 0) { - // for hidden courses, require visibility check - if (has_capability('moodle/course:viewhiddencourses', context_course::instance($course->id))) { - $visiblecourses [$course->id] = $course; - } - } else { + if (core_course_category::can_view_course_info($course)) { $visiblecourses [$course->id] = $course; } } @@ -712,15 +710,7 @@ function get_courses_page($categoryid="all", $sort="c.sortorder ASC", $fields="c // iteration will have to be done inside loop to keep track of the limitfrom and limitnum foreach($rs as $course) { context_helper::preload_from_record($course); - if ($course->visible <= 0) { - // for hidden courses, require visibility check - if (has_capability('moodle/course:viewhiddencourses', context_course::instance($course->id))) { - $totalcount++; - if ($totalcount > $limitfrom && (!$limitnum or count($visiblecourses) < $limitnum)) { - $visiblecourses [$course->id] = $course; - } - } - } else { + if (core_course_category::can_view_course_info($course)) { $totalcount++; if ($totalcount > $limitfrom && (!$limitnum or count($visiblecourses) < $limitnum)) { $visiblecourses [$course->id] = $course; @@ -742,7 +732,7 @@ function get_courses_page($categoryid="all", $sort="c.sortorder ASC", $fields="c * @param int $recordsperpage The number of records per page * @param int $totalcount Passed in by reference. * @param array $requiredcapabilities Extra list of capabilities used to filter courses - * @return object {@link $COURSE} records + * @return stdClass[] {@link $COURSE} records */ function get_courses_search($searchterms, $sort, $page, $recordsperpage, &$totalcount, $requiredcapabilities = array()) { @@ -822,12 +812,13 @@ function get_courses_search($searchterms, $sort, $page, $recordsperpage, &$total WHERE $searchcond AND c.id <> ".SITEID." ORDER BY $sort"; + $mycourses = enrol_get_my_courses(); $rs = $DB->get_recordset_sql($sql, $params); foreach($rs as $course) { // Preload contexts only for hidden courses or courses we need to return. context_helper::preload_from_record($course); $coursecontext = context_course::instance($course->id); - if (!$course->visible && !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + if (!array_key_exists($course->id, $mycourses) && !core_course_category::can_view_course_info($course)) { continue; } if (!empty($requiredcapabilities)) { diff --git a/lib/db/access.php b/lib/db/access.php index c7e7d7326ca40..bfb9dd591282b 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -733,6 +733,16 @@ 'clonepermissionsfrom' => 'moodle/category:update' ), + 'moodle/course:browse' => array( + + 'captype' => 'read', + 'contextlevel' => CONTEXT_COURSE, + 'archetypes' => array( + 'guest' => CAP_ALLOW, + 'user' => CAP_ALLOW, + ) + ), + 'moodle/category:viewhiddencategories' => array( 'captype' => 'read', diff --git a/lib/filebrowser/file_info_context_coursecat.php b/lib/filebrowser/file_info_context_coursecat.php index d0b4746f41f2f..f20b4af0d70b0 100644 --- a/lib/filebrowser/file_info_context_coursecat.php +++ b/lib/filebrowser/file_info_context_coursecat.php @@ -61,7 +61,7 @@ public function __construct($browser, $context, $category) { public function get_file_info($component, $filearea, $itemid, $filepath, $filename) { global $DB; - if (!$this->category->visible and !has_capability('moodle/category:viewhiddencategories', $this->context)) { + if (!core_course_category::can_view_category($this->category)) { if (empty($component)) { // we can not list the category contents, so try parent, or top system if ($this->category->parent and $pc = $DB->get_record('course_categories', array('id'=>$this->category->parent))) { @@ -101,7 +101,7 @@ protected function get_area_coursecat_description($itemid, $filepath, $filename) // No coursecat description area for "system". return null; } - if (!$this->category->visible and !has_capability('moodle/category:viewhiddencategories', $this->context)) { + if (!core_course_category::can_view_category($this->category)) { return null; } if (!has_capability('moodle/category:manage', $this->context)) { @@ -228,8 +228,7 @@ protected function get_categories() { foreach ($coursecats as $id => &$category) { context_helper::preload_from_record($category); - $context = context_coursecat::instance($category->id); - if (!$category->visible && !has_capability('moodle/category:viewhiddencategories', $context)) { + if (!core_course_category::can_view_category($category)) { $hiddencats[$id] = $coursecats[$id]; unset($coursecats[$id]); } diff --git a/lib/filelib.php b/lib/filelib.php index 8e17525f5f9b3..1a3141b013571 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -4626,11 +4626,8 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null, $offlin } // Check if user can view this category. - if (!has_capability('moodle/category:viewhiddencategories', $context)) { - $coursecatvisible = $DB->get_field('course_categories', 'visible', array('id' => $context->instanceid)); - if (!$coursecatvisible) { - send_file_not_found(); - } + if (!core_course_category::get($context->instanceid, IGNORE_MISSING)) { + send_file_not_found(); } $filename = array_pop($args); diff --git a/lib/form/course.php b/lib/form/course.php index d3f4a5c090dde..6b44a0d7aeb22 100644 --- a/lib/form/course.php +++ b/lib/form/course.php @@ -146,12 +146,13 @@ public function setValue($value) { WHERE c.id ". $whereclause." ORDER BY c.sortorder"; $list = $DB->get_records_sql($sql, array('contextcourse' => CONTEXT_COURSE) + $params); + $mycourses = enrol_get_my_courses(null, null, 0, array_keys($list)); $coursestoselect = array(); foreach ($list as $course) { context_helper::preload_from_record($course); $context = context_course::instance($course->id); // Make sure we can see the course. - if (!$course->visible && !has_capability('moodle/course:viewhiddencourses', $context)) { + if (!array_key_exists($course->id, $mycourses) && !core_course_category::can_view_course_info($course)) { continue; } $label = format_string(get_course_display_name_for_list($course), true, ['context' => $context]); diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 88cdff04d2135..1cc6caa2b37f9 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -2906,7 +2906,7 @@ function require_login($courseorid = null, $autologinguest = true, $cm = null, $ $USER->enrol['enrolled'][$course->id] = $until; $access = true; - } else { + } else if (core_course_category::can_view_course_info($course)) { $params = array('courseid' => $course->id, 'status' => ENROL_INSTANCE_ENABLED); $instances = $DB->get_records('enrol', $params, 'sortorder, id ASC'); $enrols = enrol_get_plugins(true); @@ -2941,6 +2941,16 @@ function require_login($courseorid = null, $autologinguest = true, $cm = null, $ } } } + } else { + // User is not enrolled and is not allowed to browse courses here. + if ($preventredirect) { + throw new require_login_exception('Course is not available'); + } + $PAGE->set_context(null); + // We need to override the navigation URL as the course won't have been added to the navigation and thus + // the navigation will mess up when trying to find it. + navigation_node::override_active_url(new moodle_url('/')); + notice(get_string('coursehidden'), $CFG->wwwroot .'/'); } } } diff --git a/lib/navigationlib.php b/lib/navigationlib.php index a32d3ea32228b..bae521fa62810 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -1298,6 +1298,9 @@ public function initialise() { $this->rootnodes['currentcourse'] = $this->add(get_string('currentcourse'), null, self::TYPE_ROOTNODE, null, 'currentcourse'); $this->rootnodes['mycourses'] = $this->add(get_string('mycourses'), null, self::TYPE_ROOTNODE, null, 'mycourses', new pix_icon('i/course', '')); $this->rootnodes['courses'] = $this->add(get_string('courses'), new moodle_url('/course/index.php'), self::TYPE_ROOTNODE, null, 'courses'); + if (!core_course_category::user_top()) { + $this->rootnodes['courses']->hide(); + } $this->rootnodes['users'] = $this->add(get_string('users'), null, self::TYPE_ROOTNODE, null, 'users'); // We always load the frontpage course to ensure it is available without @@ -1568,7 +1571,7 @@ protected function show_categories($ismycourse = false) { protected function show_my_categories() { global $CFG; if ($this->showmycategories === null) { - $this->showmycategories = !empty($CFG->navshowmycoursecategories) && core_course_category::count_all() > 1; + $this->showmycategories = !empty($CFG->navshowmycoursecategories) && !core_course_category::is_simple_site(); } return $this->showmycategories; } @@ -1951,16 +1954,18 @@ protected function add_category(stdClass $category, navigation_node $parent, $no if (array_key_exists($category->id, $this->addedcategories)) { return; } - $url = new moodle_url('/course/index.php', array('categoryid' => $category->id)); + $canview = core_course_category::can_view_category($category); + $url = $canview ? new moodle_url('/course/index.php', array('categoryid' => $category->id)) : null; $context = context_coursecat::instance($category->id); - $categoryname = format_string($category->name, true, array('context' => $context)); + $categoryname = $canview ? format_string($category->name, true, array('context' => $context)) : + get_string('categoryhidden'); $categorynode = $parent->add($categoryname, $url, $nodetype, $categoryname, $category->id); - if (empty($category->visible)) { - if (has_capability('moodle/category:viewhiddencategories', context_system::instance())) { - $categorynode->hidden = true; - } else { - $categorynode->display = false; - } + if (!$canview) { + // User does not have required capabilities to view category. + $categorynode->display = false; + } else if (!$category->visible) { + // Category is hidden but user has capability to view hidden categories. + $categorynode->hidden = true; } $this->addedcategories[$category->id] = $categorynode; } @@ -2606,10 +2611,10 @@ public function add_course(stdClass $course, $forcegeneric = false, $coursetype $coursecontext = context_course::instance($course->id); - if ($course->id != $SITE->id && !$course->visible) { + if ($coursetype != self::COURSE_MY && $coursetype != self::COURSE_CURRENT && $course->id != $SITE->id) { if (is_role_switched($course->id)) { // user has to be able to access course in order to switch, let's skip the visibility test here - } else if (!has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + } else if (!core_course_category::can_view_course_info($course)) { return false; } } @@ -3057,6 +3062,10 @@ protected function load_courses_enrolled() { if (isset($this->addedcategories[$coursecat->id])) { continue; } + // Skip categories that are not visible. + if (!$coursecat->is_uservisible()) { + continue; + } // Get this course category's parent node. $parent = null; @@ -3209,7 +3218,7 @@ public function initialise() { } require_course_login($course, true, null, false, true); $this->page->set_context(context_course::instance($course->id)); - $coursenode = $this->add_course($course); + $coursenode = $this->add_course($course, false, self::COURSE_CURRENT); $this->add_course_essentials($coursenode, $course); $this->load_course_sections($course, $coursenode); break; @@ -3221,7 +3230,7 @@ public function initialise() { $course = $DB->get_record_sql($sql, array($this->instanceid), MUST_EXIST); require_course_login($course, true, null, false, true); $this->page->set_context(context_course::instance($course->id)); - $coursenode = $this->add_course($course); + $coursenode = $this->add_course($course, false, self::COURSE_CURRENT); $this->add_course_essentials($coursenode, $course); $this->load_course_sections($course, $coursenode, $course->sectionnumber); break; @@ -3236,7 +3245,7 @@ public function initialise() { $cm = $modinfo->get_cm($this->instanceid); require_course_login($course, true, $cm, false, true); $this->page->set_context(context_module::instance($cm->id)); - $coursenode = $this->load_course($course); + $coursenode = $this->add_course($course, false, self::COURSE_CURRENT); $this->load_course_sections($course, $coursenode, null, $cm); $activitynode = $coursenode->find($cm->id, self::TYPE_ACTIVITY); if ($activitynode) { @@ -3597,15 +3606,16 @@ private function get_course_categories() { $categories = array(); $cap = 'moodle/category:viewhiddencategories'; - $showcategories = core_course_category::count_all() > 1; + $showcategories = !core_course_category::is_simple_site(); if ($showcategories) { foreach ($this->page->categories as $category) { - if (!$category->visible && !has_capability($cap, get_category_or_system_context($category->parent))) { + $context = context_coursecat::instance($category->id); + if (!core_course_category::can_view_category($category)) { continue; } $url = new moodle_url('/course/index.php', array('categoryid' => $category->id)); - $name = format_string($category->name, true, array('context' => context_coursecat::instance($category->id))); + $name = format_string($category->name, true, array('context' => $context)); $categorynode = breadcrumb_navigation_node::create($name, $url, self::TYPE_CATEGORY, null, $category->id); if (!$category->visible) { $categorynode->hidden = true; diff --git a/lib/upgrade.txt b/lib/upgrade.txt index ab0d4e82b94f1..f1bbeceee21a8 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -26,6 +26,10 @@ attribute on forms to avoid collisions in forms loaded in AJAX requests. When the parameter is set to that constant, the function won't process file merging, keeping the original state of the file area. * Introduced new callback for plugin developers '_pre_processor_message_send($procname, $proceventdata)': This will allow any plugin to manipulate messages or notifications before they are sent by a processor (email, mobile...) +* New capability 'moodle/course:browse' in category context that controls whether user is able to browse list of courses + in this category. To work with list of courses use API methods in core_course_category and also 'course' form element. +* It is possible to pass additional conditions to get_courses_search(); + core_course_category::search_courses() now allows to search only among courses with completion enabled. === 3.6 === diff --git a/search/classes/manager.php b/search/classes/manager.php index 2abe63de5d4f8..68530844c5dae 100644 --- a/search/classes/manager.php +++ b/search/classes/manager.php @@ -720,7 +720,7 @@ protected function get_areas_user_accesses($limitcourseids = false, $limitcontex (!$limitcontextids || in_array($coursecontext->id, $limitcontextids))) { // Add the course contexts the user can view. foreach ($areasbylevel[CONTEXT_COURSE] as $areaid => $searchclass) { - if ($course->visible || has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + if (!empty($mycourses[$course->id]) || \core_course_category::can_view_course_info($course)) { $areascontexts[$areaid][$coursecontext->id] = $coursecontext->id; } } diff --git a/version.php b/version.php index 5368791a889ea..1368f74b77e7a 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019041000.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019041000.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.