datalib:course_parent_visible() rework, remove category_parent_visible()

Reworked course_parent_visible() to always return in a constant
number of db queries (2 worst case) regardless of nesting depth.

The rewritten version has a small cache, but if you are going to
walk many courses, it's still 1~2 DB queries per category seen,
so the right thing to do is to check it in the caller, as seen
in get_my_courses().

diff --git a/lib/datalib.php b/lib/datalib.php
index 72b45d442a7611c9b103a018c7fac76ab17bcd9e..9f2873848fde95173f5583d596b6faa8dd8d06dd 100644 (file)
--- a/lib/datalib.php
+++ b/lib/datalib.php
@@ -1771,46 +1771,71 @@ function print_object($object) {
     echo '<pre class="notifytiny">' . htmlspecialchars(print_r($object,true)) . '</pre>';
 }
 
+/*
+ * Check whether a course is visible through its parents
+ * path. 
+ *
+ * Notes:
+ *
+ * - All we need from the course is ->category. _However_
+ *   if the course object has a categorypath property,
+ *   we'll save a dbquery
+ *
+ * - If we return false, you'll still need to check if
+ *   the user can has the 'moodle/category:visibility'
+ *   capability...
+ *
+ * - Will generate 2 DB calls. 
+ *
+ * - It does have a small local cache, however...
+ *
+ * - Do NOT call this over many courses as it'll generate
+ *   DB traffic. Instead, see what get_my_courses() does.
+ *
+ * @param mixed $object A course object
+ * @return bool
+ */
 function course_parent_visible($course = null) {
     global $CFG;
+    //return true;
+    static $mycache;
 
-    if (empty($course)) {
+    if (!is_object($course)) {
         return true;
     }
     if (!empty($CFG->allowvisiblecoursesinhiddencategories)) {
         return true;
     }
-    return category_parent_visible($course->category);
-}
 
-function category_parent_visible($parent = 0) {
-
-    static $visible;
-
-    if (!$parent) {
-        return true;
+    if (!isset($mycache)) {
+        $mycache = array();
+    } else {
+        // cast to force assoc array
+        $k = (string)$course->category; 
+        if (isset($mycache[$k])) {
+            return $mycache[$k];
+        }
     }
 
-    if (empty($visible)) {
-        $visible = array(); // initialize
+    if (isset($course->categorypath)) {
+        $path = $course->categorypath;
+    } else {
+        $path = get_field('course_categories', 'path', 
+                          'id', $course->category);
     }
+    $catids = substr($path,1); // strip leading slash
+    $catids = str_replace('/',',',$catids);
 
-    if (array_key_exists($parent,$visible)) {
-        return $visible[$parent];
-    }
+    $sql = "SELECT MIN(visible)
+            FROM {$CFG->prefix}course_categories
+            WHERE id IN ($catids)";
+    $vis = get_field_sql($sql);
 
-    $category = get_record('course_categories', 'id', $parent);
-    $list = explode('/', preg_replace('/^\/(.*)$/', '$1', $category->path));
-    $list[] = $parent;
-    $parents = get_records_list('course_categories', 'id', implode(',', $list), 'depth DESC');
-    $v = true;
-    foreach ($parents as $p) {
-        if (!$p->visible) {
-            $v = false;
-        }
-    }
-    $visible[$parent] = $v; // now cache it
-    return $v;
+    // cast to force assoc array
+    $k = (string)$course->category;
+    $mycache[$k] = $vis;
+
+    return $vis;
 }
 
 /**