From acf5018d832b6365f0053385671cede65adae2a3 Mon Sep 17 00:00:00 2001
From: Julian Wendling <xdeavenx@gmail.com>
Date: Mon, 4 Oct 2021 13:11:28 +0900
Subject: [PATCH] Reduce load time in case of ppl with many courses

This commit includeD the following changes:

* REMOVE DB requests and enrol_get_my_courses method usage
* Inrease version
* ADD Arrays for semester containing their courses, optimize sorting of semesters and courses, internal changes to work with new changes
* FIX Error related to missing navigation when user is not yet logged in
* REMOVE Mutliple calls to get_course_metadata
* Prevent array access with empty key
* Change parameter type from int to constant variable
---
 lib.php     | 208 +++++++++++++++++++---------------------------------
 version.php |   2 +-
 2 files changed, 76 insertions(+), 134 deletions(-)

diff --git a/lib.php b/lib.php
index f34f279..19915cc 100644
--- a/lib.php
+++ b/lib.php
@@ -22,10 +22,11 @@
 
 function local_sembasednav_extend_navigation(global_navigation $navigation)
 {
-    global $PAGE, $CFG;
+    global $PAGE, $CFG, $DB;
+
+    $semFieldName = $DB->get_record('customfield_field', array('type' => 'semester'), '*', MUST_EXIST);
 
     $myCoursesNode = $navigation->find('mycourses', global_navigation::TYPE_ROOTNODE);
-    $myCoursesChildNodes = $myCoursesNode->get_children_key_list();
 
     $maxSemesterNodes = get_config('sembasednav', 'setting_nodescount');
     $openFirstSemester = get_config('sembasednav', 'setting_openfirstnode');
@@ -35,17 +36,8 @@ function local_sembasednav_extend_navigation(global_navigation $navigation)
     // Register JS to close all child nodes from root node (mycourse)
     $PAGE->requires->js_call_amd('local_sembasednav/sembasednav', 'closeAllChildNodes', [$myCoursesNode->key]);
 
-    try {
-        $courses = enrol_get_my_courses();
-    } catch (Exception $e) {
-        echo $e->getMessage();
-        $courses = [];
-    }
-
-    // Removes courses that are shown by default
-    foreach ($myCoursesChildNodes as $cn) {
-        $myCoursesNode->find($cn, null)->showinflatnavigation = false;
-    }
+    // type() needed to get an array for usort - navigation_node_collection by itself is not sortable
+    $courses = $myCoursesNode->children->type(navigation_node::TYPE_COURSE);
 
     // Nodes that are excluded from max semester count - will always be shown
     $noSemesterAssignedName = get_config('sembasednav', 'setting_nosemestername');
@@ -53,90 +45,66 @@ function local_sembasednav_extend_navigation(global_navigation $navigation)
     $specialNodesList = [];
     $mySemesters = [];
 
-    // Sort semester depending on setting
-    if ($semesterDescending == 0) {
-        // Descending
-        usort($courses, function ($a, $b) {
-            $aId = get_semester_id($a->id);
-            $bId = get_semester_id($b->id);
-
-            return strcasecmp($bId, $aId);
-        });
-    } else {
-        // Ascending
-        usort($courses, function ($a, $b) {
-            $aId = get_semester_id($a->id);
-            $bId = get_semester_id($b->id);
-
-            return strcasecmp($aId, $bId);
-        });
-    }
+    // Sorts all courses basend on the shorttext
+    usort($courses, function ($a, $b) {
+        $aId = $a->shorttext;
+        $bId = $b->shorttext;
+        return strcasecmp($aId, $bId);
+    });
 
-    // Assign all modules and semesters
+    // Creates a new array with semesters that contain their courses
     foreach ($courses as $c) {
-        $semesterName = get_semester_name($c->id);
+        $semesterCustomField = get_semester_customFields((int) $c->key, $semFieldName);
+        $semesterId = preg_replace('/[^0-9]/', '', $semesterCustomField);
+        $semesterName = \customfield_semester\data_controller::get_name_for_semester((int)$semesterId);
+
+        $c->semesterId = $semesterId;
+        $c->semesterName = $semesterName;
 
-        $courseName = empty($CFG->navshowfullcoursenames) ? $c->shortname : $c->fullname;
+        $courseName = empty($CFG->navshowfullcoursenames) ? $c->shorttext : $c->title;
 
-        if (in_array($semesterName, $specialNodes, true)) {
-            $specialNodesList[$semesterName][] = $courseName;
+        if ($c->semesterId == 1) {
+            // Term-independent
+            $specialNodesList[$c->semesterId]["name"] = $c->semesterName;
+            $specialNodesList[$c->semesterId]["courses"][] = $c;
+            $c->showinflatnavigation = false; 
         } else {
-            if (count($mySemesters) >= $maxSemesterNodes) {
-                continue;
-            }
-            $mySemesters[$semesterName][] = $courseName;
+            // Normal semesters
+            $mySemesters[$c->semesterId]["name"] = $c->semesterName;
+            $mySemesters[$c->semesterId]["courses"][] = $c;
+            $c->showinflatnavigation = false;
         }
     }
 
+    // Sort semesters depending on setting
+    if ($semesterDescending == 1) {
+        // Descending
+        ksort($mySemesters);
+    } else {
+        // Ascending
+        krsort($mySemesters);
+    }
+
     if ($specialNodesFirst == 0)
         add_semester_nodes($specialNodesList, $myCoursesNode);
 
     // Add all semester nodes
-    add_semester_nodes($mySemesters, $myCoursesNode);
-
-    if ($openFirstSemester)
-       open_semester_node(array_keys($mySemesters)[0] , $myCoursesNode);
+    add_semester_nodes(array_slice($mySemesters, 0, $maxSemesterNodes), $myCoursesNode);
 
     // Add all special nodes
     if ($specialNodesFirst == 1)
         add_semester_nodes($specialNodesList, $myCoursesNode);
 
-    // Sorts course node alphabetically
-    usort($courses, function ($a, $b) {
-        $aId = $a->shortname;
-        $bId = $b->shortname;
-
-        return strcasecmp($aId, $bId);
-    });
-
-    // Creates all course nodes and assigns them to their semester node
-    foreach ($courses as $c) {
-        $semesterName = get_semester_name($c->id);
-        $semesterKey = create_node_key($semesterName);
-        $semesterNode = $navigation->find($semesterKey, null);
-
-        // Can't find semester of current course
-        if (!$semesterNode) {
-            continue;
-        }
-
-        $courseName = empty($CFG->navshowfullcoursenames) ? $c->shortname : $c->fullname;
-        $courseKey = create_node_key($courseName);
-
-        $courseNode = navigation_node::create($courseName, new moodle_url('/course/view.php', array('id' => $c->id)), global_navigation::TYPE_COURSE, $courseName, $courseKey, new pix_icon('i/course', 'grades'));
-
-        $courseNode->showinflatnavigation = true;
-        $courseNode->add_class('p-l-3');
-
-        if (!$semesterNode->forceopen)
-            $courseNode->add_class('localboostnavigationcollapsedchild');
-
-        $semesterNode->add_node($courseNode);
+    if (count($mySemesters)) {
+        $firstSemesterName = $mySemesters[array_key_first($mySemesters)]["name"];
+    } else {
+        $firstSemesterName = null;
+    }
 
-        // Open semester node of active course
-        if ($courseNode->isactive) {
-            open_semester_node($semesterNode->text , $myCoursesNode);
-        }
+    if ($openFirstSemester && !is_null($firstSemesterName)) {
+        $key = create_node_key($firstSemesterName);
+        $semesterNode = $myCoursesNode->find($key, global_navigation::NODETYPE_BRANCH);
+        open_semester_node($semesterNode, $myCoursesNode);
     }
 }
 
@@ -155,13 +123,31 @@ function add_semester_nodes(array $semesterList, $parentNode)
     $collapsenodesforjs = [];
 
     foreach ($semesterList as $key => $value) {
-        $semesterNode = create_semester_node($key);
+        $semesterNode = create_semester_node($value["name"]);
 
         if (!in_array($semesterNode->key, $collapsenodesforjs)) {
             $collapsenodesforjs[] = $semesterNode->key;
         }
 
         $parentNode->add_node($semesterNode);
+
+        // Assign courses to their semester in the navigation
+        foreach ($value["courses"] as $c) {
+            $parentNode->children->remove($c->key); 
+            $c->add_class('p-l-3');
+            $c->set_parent($semesterNode);
+            $c->showinflatnavigation = true;
+
+            if (!$semesterNode->forceopen)
+                $c->add_class('localboostnavigationcollapsedchild');
+
+            $semesterNode->add_node($c);
+
+            // Open semester node of active course
+            if ($c->isactive) {
+                open_semester_node($semesterNode, $parentNode);
+            }
+        }
     }
 
     // Apply collapse navigation js to every semester
@@ -176,22 +162,19 @@ function add_semester_nodes(array $semesterList, $parentNode)
 }
 
 /**
- * @param $semesterKey
+ * @param $semesterNode
  * @param $myCourseNode
  */
-function open_semester_node($semesterKey, $myCourseNode)
+function open_semester_node($semesterNode, $myCourseNode)
 {
-    if (empty($semesterKey))
+    if (empty($semesterNode))
         return;
 
-    $key = create_node_key($semesterKey);
-    $semesterNode = $myCourseNode->find($key, global_navigation::NODETYPE_BRANCH);
-
     $semesterNode->forceopen = true;
     $semesterNode->remove_class('localboostnavigationcollapsedparent');
 
-    foreach ($semesterNode->get_children_key_list() as $c) {
-        $node = $myCourseNode->find($c, null);
+    foreach ($semesterNode->get_children_key_list() as $key) {
+        $node = $myCourseNode->find($key, null);
         $node->remove_class('localboostnavigationcollapsedchild');
     }
 }
@@ -235,60 +218,19 @@ function create_node_key(string $semesterName)
 }
 
 /**
- * Returns semester name of given course id.
- * If no semester is found, return no semester assigned name.
+ * Gets the custom fields of a given course id 
  * @param int $id
  * @return string
  */
-function get_semester_name(int $id)
-{
-    global $DB;
-
-    $noSemesterAssigned = get_config('sembasednav', 'setting_nosemestername');
-    $semesterName = '';
-
-    try {
-        $semFieldName = $DB->get_record('customfield_field', array('type' => 'semester'), '*', MUST_EXIST);
-    } catch (Exception $e) {
-        return $noSemesterAssigned;
-    }
-
+function get_semester_customFields(int $id, $semFieldName) {
     try {
         $allCustomFields = get_course_metadata($id);
         $semesterField = $allCustomFields[$semFieldName->shortname];
-        $semesterValue = preg_replace('/[^0-9]/', '', $semesterField); // extract semester int from string
-        $semesterName = \customfield_semester\data_controller::get_name_for_semester((int)$semesterValue);
-
-    } catch (Exception $e) {
-    }
-
-    return empty($semesterName) || $semesterName === '' ? $noSemesterAssigned : $semesterName;
-}
-
-/**
- * @param int $id
- * @return string
- */
-function get_semester_id(int $id)
-{
-    global $DB;
-
-    $semesterValue = '';
 
-    try {
-        $semFieldName = $DB->get_record('customfield_field', array('type' => 'semester'), '*', MUST_EXIST);
-    } catch (Exception $e) {
-        return 0;
-    }
-
-    try {
-        $allCustomFields = get_course_metadata($id);
-        $semesterField = $allCustomFields[$semFieldName->shortname];
-        $semesterValue = preg_replace('/[^0-9]/', '', $semesterField); // extract semester int from string
+        return $semesterField;
     } catch (Exception $e) {
+        echo $e->getMessage();
     }
-
-    return empty($semesterValue) || $semesterValue === '' ? 0 : $semesterValue;
 }
 
 // https://docs.moodle.org/dev/Custom_fields_API#Example_code_for_course_custom_fields
diff --git a/version.php b/version.php
index ef096af..bc7e8e1 100644
--- a/version.php
+++ b/version.php
@@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'local_sembasednav';
 $plugin->release = '0.1.0';
-$plugin->version = 2020060300;
+$plugin->version = 2020060302;
 $plugin->requires = 2019052000;
 $plugin->maturity = MATURITY_ALPHA;
 $plugin->dependencies = [
-- 
GitLab