WR422914 Bug fixes and minor performance improvements

catalyst · Nov 6, 2024 · f11afdc · f11afdc
1 parent be02015
commit f11afdc
Show file tree

Hide file tree

Showing 23 changed files with 177 additions and 146 deletions.
diff --git a/classes/external.php b/classes/external.php
@@ -56,7 +56,7 @@ public static function get_courses_parameters() : external_function_parameters {
      * @return string JSON response.
      */
     public static function get_courses(string $query) : string {
-        global $DB;
+        global $DB, $SITE, $COURSE;
         manager::write_close(); // Close session early this is a read op.
 
         // Parameter validation.
@@ -66,18 +66,29 @@ public static function get_courses(string $query) : string {
         );
 
         // Execute API call.
-        $sql = 'SELECT id, fullname FROM {course} WHERE ' . $DB->sql_like('fullname', ':fullname', false) . ' AND id <> 1';
+        $sql = 'SELECT id, fullname, category FROM {course} WHERE ' . $DB->sql_like('fullname', ':fullname', false) . ' AND id <> 1';
         $params = ['fullname' => '%' . $DB->sql_like_escape($query) . '%'];
-        $courses = $DB->get_records_sql($sql, $params, 0, 11);
+        $courses = $DB->get_records_sql($sql, $params, 0, 30);
 
         $data = [];
+        if (has_capability('local/assessfreq:view', context_system::instance())) {
+            $data[SITEID] = [
+                "id" => $SITE->id,
+                "fullname" => external_format_string($SITE->fullname, true, ["escape" => false])
+            ];
+        }
+        $categories = \core_course_category::make_categories_list();
         foreach ($courses as $course) {
             $data[$course->id] = [
                 "id" => $course->id,
-                "fullname" => external_format_string($course->fullname, true, ["escape" => false])
+                "fullname" => $categories[$course->category] . ' / ' . external_format_string($course->fullname, true, ["escape" => false])
             ];
         }
 
+        if (isset($data[$COURSE->id])) {
+            unset($data[$COURSE->id]);
+        }
+
         return json_encode(array_values($data));
     }
 

diff --git a/classes/frequency.php b/classes/frequency.php
@@ -27,6 +27,7 @@
 use cache;
 use context;
 use core\dml\sql_join;
+use core\oauth2\service\microsoft;
 use Exception;
 use moodle_recordset;
 use stdClass;
@@ -63,6 +64,11 @@ class frequency {
      */
     private int $batchsize = 100;
 
+    /**
+     * Cache of event users.
+     */
+    private array $eventuserscache = [];
+
     /**
      * Given a modle shortname get capabilities that users must have
      * before that activity event applies to them.
@@ -335,15 +341,33 @@ private function get_enrolled_users(context $context, array $capabilities): arra
      * this can take a long time. Consider using the get_event_users method
      * if you don't need the most up to date data.
      *
-     * @param int $contextid The context ID in a course for the event to check.
+     * @param int $contextid The module context ID for the event to check.
      * @param string $module The type of module the event is for.
      * @return array $users An array of user IDs.
      */
     public function get_event_users_raw(int $contextid, string $module): array {
+
         $context = context::instance_by_id($contextid);
+        $coursecontext = $context->get_parent_context();
+
+        $cachekey = "{$coursecontext->id}-{$module}";
+        if (isset($this->eventuserscache[$cachekey])) {
+            return $this->eventuserscache[$cachekey];
+        }
+
         $capabilities = $this->get_module_capabilities($module);
 
-        return $this->get_enrolled_users($context, $capabilities);
+        $roles = [];
+        foreach ($capabilities as $capability) {
+            $roles = $roles + get_roles_with_capability($capability, CAP_ALLOW, $context);
+        }
+        $users = [];
+        foreach ($roles as $role) {
+            $users = $users + get_users_from_role_on_context($role, $coursecontext);
+        }
+
+        $this->eventuserscache[$cachekey] = array_column($users, 'userid');
+        return $this->eventuserscache[$cachekey];
     }
 
     /**
@@ -421,7 +445,7 @@ private function prepare_user_event_records(array $users, int $eventid): array {
         $userrecords = [];
         foreach ($users as $user) {
             $record = new stdClass();
-            $record->userid = $user->id;
+            $record->userid = $user->userid;
             $record->eventid = $eventid;
 
             $userrecords[] = $record;

diff --git a/classes/output/renderer.php b/classes/output/renderer.php
@@ -25,6 +25,8 @@
 
 namespace local_assessfreq\output;
 
+use local_assessfreq\form\course_search;
+use local_assessfreq\report_base;
 use plugin_renderer_base;
 
 class renderer extends plugin_renderer_base {
@@ -34,9 +36,11 @@ class renderer extends plugin_renderer_base {
      * @return void
      */
     public function render_reports() : void {
+        global $PAGE;
         $reports = get_reports();
         $reportoutputs = [];
         foreach ($reports as $report) {
+            /* @var $report report_base */
             $reportoutputs[] = [
                 'tablink' => $report->get_tablink(), // Plugin name.
                 'tabname' => $report->get_name(), // Display name.
@@ -47,11 +51,31 @@ public function render_reports() : void {
         usort($reportoutputs, function($a, $b) {
             return $a['weight'] <=> $b['weight'];
         });
+        $courseselectoptions = [];
+        $courseselect = '';
+        $categories = \core_course_category::make_categories_list();
+
+        foreach ($categories as $categoryid => $category) {
+            $courses = get_courses($categoryid);
+            foreach ($courses as $course) {
+                $courseselectoptions[$course->id] = $category . ' - ' . $course->fullname;
+            }
+        }
+        if (!empty($courseselectoptions)) {
+            $courseselect = $this->single_select(
+                '#',
+                'courseid',
+                $courseselectoptions,
+                $PAGE->course->id != SITEID ? $PAGE->course->id : '',
+                ['' => get_string('courseselect', 'local_assessfreq')],
+            );
+        }
         $output = $this->output->header();
         $output .= $this->render_from_template(
             'local_assessfreq/index',
             [
-                'reports' => $reportoutputs
+                'reports' => $reportoutputs,
+                'courseselect' => $courseselect,
             ]
         );
         $output .= $this->output->footer();

diff --git a/classes/source_base.php b/classes/source_base.php
@@ -157,7 +157,7 @@ protected function get_tracking(int $assessid, bool $limited = false) : array {
      * Given an assess ID and module get its most recent tracking information.
      *
      * @param int $assessid The ID of the assessment.
-     * @return mixed $tracking Tracking reocrds for the quiz.
+     * @return mixed $tracking Tracking record.
      */
     protected function get_recent_tracking(int $assessid) {
         global $DB;
@@ -173,5 +173,4 @@ protected function get_recent_tracking(int $assessid) {
             [$assessid, $this->get_module()]
         );
     }
-
 }
diff --git a/classes/task/data_process.php b/classes/task/data_process.php
@@ -97,5 +97,15 @@ public function execute() {
         ]);
         $event->trigger();
         mtrace('local_assessfreq: Processing user events finished in: ' . $actionduration . ' seconds');
+
+        //mtrace('local_assessfreq: Clearing legacy tracking data');
+        //$actionstart = time();
+        //$actionduration = time() - $actionstart;
+        //$event = event_processed::create([
+        //    'context' => $context,
+        //    'other' => ['action' => 'user', 'duration' => $actionduration],
+        //]);
+        //$event->trigger();
+        //mtrace('local_assessfreq: Processing user events finished in: ' . $actionduration . ' seconds');
     }
 }
diff --git a/classes/task/quiz_tracking.php b/classes/task/quiz_tracking.php
diff --git a/db/install.xml b/db/install.xml
@@ -82,7 +82,7 @@
       </KEYS>
       <INDEXES>
         <INDEX NAME="assessid" UNIQUE="false" FIELDS="assessid"/>
-        <INDEX NAME="module" UNIQUE="false" FIELDS="module"/>
+        <INDEX NAME="assessidmodule" UNIQUE="false" FIELDS="assessid,module"/>
       </INDEXES>
     </TABLE>
   </TABLES>

diff --git a/db/tasks.php b/db/tasks.php
@@ -36,14 +36,5 @@
         'day' => '*',
         'dayofweek' => '*',
         'month' => '*',
-    ],
-    [
-        'classname' => 'local_assessfreq\task\quiz_tracking',
-        'blocking' => 0,
-        'minute' => '*',
-        'hour' => '*',
-        'day' => '*',
-        'dayofweek' => '*',
-        'month' => '*',
-    ],
+    ]
 ];
diff --git a/db/upgrade.php b/db/upgrade.php
@@ -33,11 +33,14 @@ function xmldb_local_assessfreq_upgrade($oldversion) {
 
     $dbman = $DB->get_manager();
 
-    if ($oldversion < 2024021413) {
+    if ($oldversion < 2024040302) {
 
         $table = new xmldb_table('local_assessfreq_trend');
-        $field = new xmldb_field('module', XMLDB_TYPE_CHAR, '20', true, true);
-        $index = new xmldb_index('module', XMLDB_INDEX_NOTUNIQUE, ['module']);
+        /*
+         * Previously we only used this table for quiz, so all existing modules will be quiz modules, hence the default.
+         */
+        $field = new xmldb_field('module', XMLDB_TYPE_CHAR, '20', true, true, null, 'quiz');
+        $index = new xmldb_index('module', XMLDB_INDEX_NOTUNIQUE, ['assessid', 'module']);
 
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
@@ -47,12 +50,7 @@ function xmldb_local_assessfreq_upgrade($oldversion) {
             $dbman->add_index($table, $index);
         }
 
-        /*
-         * Previously we only used this table for quiz, so all existing modules will be quiz modules.
-         */
-        $DB->execute("UPDATE {local_assessfreq_trend} SET module = 'quiz'");
-
-        upgrade_plugin_savepoint(true, 2024021413, 'local', 'assessfreq');
+        upgrade_plugin_savepoint(true, 2024040302, 'local', 'assessfreq');
     }
 
     return true;

diff --git a/index.php b/index.php
@@ -35,6 +35,10 @@
 
 // If we have a course selected, update the PAGE object accordinging.
 if ($courseid = optional_param('courseid', 0, PARAM_INT)) {
+    // If we've been given the side id redirect without the param.
+    if ($courseid == SITEID) {
+        redirect('/local/assessfreq/');
+    }
     $context = context_course::instance($courseid);
     $PAGE->set_pagelayout('incourse');
     $course = get_course($courseid);

diff --git a/lang/en/local_assessfreq.php b/lang/en/local_assessfreq.php
@@ -24,6 +24,8 @@
  */
 
 $string['pluginname'] = 'Assessment Frequency Report';
+$string['subplugintype_assessfreqreport_plural'] = 'Assessment Frequency Reports';
+$string['subplugintype_assessfreqsource_plural'] = 'Assessment Frequency Sources';
 
 $string['privacy:metadata'] = 'The assessment frequency reports only display data';
 

diff --git a/lib.php b/lib.php
@@ -33,17 +33,13 @@
  * @param context $context The context of the course
  */
 function local_assessfreq_extend_navigation_course(navigation_node $navigation, stdClass $course, context $context) {
-    global $CFG, $OUTPUT;
     if (has_capability('local/assessfreq:view', $context)) {
         $url = new moodle_url('/local/assessfreq/', ['courseid' => $course->id]);
-        $navigation->add(
-            get_string('pluginname', 'local_assessfreq'),
-            $url,
-            navigation_node::TYPE_SETTING,
-            null,
-            null,
-            new pix_icon('i/report', '')
-        );
+        $settingsnode = navigation_node::create(get_string('pluginname', 'local_assessfreq'), $url);
+        $reportnode = $navigation->get('coursereports');
+        if (isset($settingsnode) && !empty($reportnode)) {
+            $reportnode->add_node($settingsnode);
+        }
     }
 }