MDL-66301 forumreport_summary: Refactors and fixes to course report

mod/forum/report/summary/classes/event/report_downloaded.php

@@ -75,8 +75,13 @@ class report_downloaded extends \core\event\base {
      * @return \moodle_url
      */
     public function get_url() {
-        return new \moodle_url('/mod/forum/report/summary/index.php',
+        $params = ['courseid' => $this->courseid];
-                ['courseid' => $this->courseid, 'forumid' => $this->other['forumid']]);
+
+        if (!empty($this->other['forumid'])) {
+            $params['forumid'] = $this->other['forumid'];
+        }
+
+        return new \moodle_url('/mod/forum/report/summary/index.php', $params);
     }
 
     /**

mod/forum/report/summary/classes/event/report_viewed.php

@@ -76,8 +76,13 @@ class report_viewed extends \core\event\base {
      * @return \moodle_url
      */
     public function get_url() {
-        return new \moodle_url('/mod/forum/report/summary/index.php',
+        $params = ['courseid' => $this->courseid];
-                ['courseid' => $this->courseid, 'forumid' => $this->other['forumid']]);
+
+        if (!empty($this->other['forumid'])) {
+            $params['forumid'] = $this->other['forumid'];
+        }
+
+        return new \moodle_url('/mod/forum/report/summary/index.php', $params);
     }
 
     /**

mod/forum/report/summary/classes/output/filters.php

@@ -142,6 +142,7 @@ class filters implements renderable, templatable {
         $coursegroups = groups_get_all_groups($this->courseid);
         $forumids = [];
         $allgroups = false;
+        $hasgroups = false;
 
         // Check if any forum gives the user access to all groups and no groups.
         foreach ($this->cms as $cm) {
@@ -156,6 +157,8 @@ class filters implements renderable, templatable {
                     continue;
                 }
 
+                $hasgroups = true;
+
                 // Fetch for the current cm's forum.
                 $context = \context_module::instance($cm->id);
                 $aag = has_capability('moodle/site:accessallgroups', $context);
@@ -166,6 +169,11 @@ class filters implements renderable, templatable {
             }
         }
 
+        // If no groups mode enabled, nothing to prepare.
+        if (!$hasgroups) {
+            return;
+        }
+
         // Any groups, and no groups.
         if ($allgroups) {
             $nogroups = new stdClass();

mod/forum/report/summary/classes/summary_table.php

@@ -65,7 +65,10 @@ class summary_table extends table_sql {
     /** @var int The course ID containing the forum(s) being reported on. */
     protected $courseid;
 
-    /** @var bool True if reporting on all forums in course, false if reporting on specific forum(s) */
+    /** @var bool True if reporting on all forums in course user has access to, false if reporting on a single forum */
+    protected $iscoursereport = false;
+
+    /** @var bool True if user has access to all forums in the course (and is running course report), otherwise false. */
     protected $accessallforums = false;
 
     /** @var \stdClass The course module object(s) of the forum(s) being reported on. */
@@ -119,16 +122,18 @@ class summary_table extends table_sql {
      * @param bool $canseeprivatereplies Whether the user can see all private replies or not.
      * @param int $perpage The number of rows to display per page.
      * @param bool $canexport Is the user allowed to export records?
+     * @param bool $iscoursereport Whether the user is running a course level report
      * @param bool $accessallforums If user is running a course level report, do they have access to all forums in the course?
      */
     public function __construct(int $courseid, array $filters, bool $allowbulkoperations,
-            bool $canseeprivatereplies, int $perpage, bool $canexport, bool $accessallforums) {
+            bool $canseeprivatereplies, int $perpage, bool $canexport, bool $iscoursereport, bool $accessallforums) {
-        global $USER, $OUTPUT;
+        global $OUTPUT;
 
-        $uniqueid = $courseid . (empty($filters['forums']) ? '' : '_' . $filters['forums'][0]);
+        $uniqueid = $courseid . ($iscoursereport ? '' : '_' . $filters['forums'][0]);
         parent::__construct("summaryreport_{$uniqueid}");
 
         $this->courseid = $courseid;
+        $this->iscoursereport = $iscoursereport;
         $this->accessallforums = $accessallforums;
         $this->allowbulkoperations = $allowbulkoperations;
         $this->canseeprivatereplies = $canseeprivatereplies;
@@ -197,8 +202,8 @@ class summary_table extends table_sql {
     protected function set_forum_properties(array $forumids): void {
         global $USER;
 
-        // If no forum IDs filtered, reporting on all forums in the course the user has access to.
+        // Course context if reporting on all forums in the course the user has access to.
-        if ($this->accessallforums) {
+        if ($this->iscoursereport) {
             $this->userfieldscontext = \context_course::instance($this->courseid);
         }
 
@@ -269,7 +274,7 @@ class summary_table extends table_sql {
         }
 
         global $OUTPUT;
-        return $OUTPUT->user_picture($data, array('size' => 35, 'courseid' => $this->courseid, 'includefullname' => true));
+        return $OUTPUT->user_picture($data, array('courseid' => $this->courseid, 'includefullname' => true));
     }
 
     /**
@@ -336,8 +341,7 @@ class summary_table extends table_sql {
         global $OUTPUT;
 
         // If no posts, nothing to export.
-        // If reporting on more than one forum (eg a course), unable to export (export only handles a single forum).
+        if (empty($data->earliestpost)) {
-        if (empty($data->earliestpost) || count($this->cms) > 1) {
             return '';
         }
 
@@ -682,7 +686,7 @@ class summary_table extends table_sql {
      * @return void.
      */
     protected function apply_filters(array $filters): void {
-        // Apply the forums filter if not reporting on whole course.
+        // Apply the forums filter if not reporting on every forum in a course.
         if (!$this->accessallforums) {
             $this->add_filter(self::FILTER_FORUM, $filters['forums']);
         }

mod/forum/report/summary/index.php

@@ -49,65 +49,66 @@ $courseforums = $modinfo->instances['forum'];
 $cms = [];
 
 // Determine which forums the user has access to in the course.
+$accessallforums = false;
 $allforumidsincourse = array_keys($courseforums);
 $forumsvisibletouser = [];
+$forumselectoptions = [0 => get_string('forumselectcourseoption', 'forumreport_summary')];
 
 foreach ($courseforums as $courseforumid => $courseforum) {
     if ($courseforum->uservisible) {
-        $forumsvisibletouser[$courseforumid] = $courseforum->name;
+        $forumsvisibletouser[$courseforumid] = $courseforum;
+        $forumselectoptions[$courseforumid] = $courseforum->name;
     }
 }
 
 if ($forumid) {
-    $filters['forums'] = [$forumid];
+    if (!isset($forumsvisibletouser[$forumid])) {
-
+        throw new \moodle_exception('A valid forum ID is required to generate a summary report.');
-    if (!isset($courseforums[$forumid])) {
-        throw new \moodle_exception("A valid forum ID is required to generate a summary report.");
     }
 
-    $foruminfo = $courseforums[$forumid];
+    $filters['forums'] = [$forumid];
-    $title = $foruminfo->name;
+    $title = $forumsvisibletouser[$forumid]->name;
-    $forumcm = $foruminfo->get_course_module_record();
+    $forumcm = $forumsvisibletouser[$forumid]->get_course_module_record();
     $cms[] = $forumcm;
 
     require_login($courseid, false, $forumcm);
     $context = \context_module::instance($forumcm->id);
     $canexport = !$download && has_capability('mod/forum:exportforum', $context);
-    $redirecturl = new moodle_url("/mod/forum/view.php");
+    $redirecturl = new moodle_url('/mod/forum/view.php', ['id' => $forumid]);
-    $redirecturl->param('id', $forumid);
+    $numforums = 1;
     $pageurlparams['forumid'] = $forumid;
-    $accessallforums = false;
+    $iscoursereport = false;
 } else {
-    // Course level report
+    // Course level report.
     require_login($courseid, false);
 
     $filters['forums'] = array_keys($forumsvisibletouser);
 
-    // Fetch the forum cms for the course.
+    // Fetch the forum CMs for the course.
-    foreach ($courseforums as $courseforum) {
+    foreach ($forumsvisibletouser as $visibleforum) {
-        $cms[] = $courseforum->get_course_module_record();
+        $cms[] = $visibleforum->get_course_module_record();
     }
 
     $context = \context_course::instance($courseid);
     $title = $course->fullname;
     // Export currently only supports single forum exports.
     $canexport = false;
-    $redirecturl = new moodle_url("/course/view.php");
+    $redirecturl = new moodle_url('/course/view.php', ['id' => $courseid]);
-    $redirecturl->param('id', $courseid);
+    $numforums = count($forumsvisibletouser);
+    $iscoursereport = true;
 
-    // Determine whether user has access to all forums in the course.
+    // Specify whether user has access to all forums in the course.
     $accessallforums = empty(array_diff($allforumidsincourse, $filters['forums']));
 }
 
-$pageurl = new moodle_url("/mod/forum/report/summary/index.php", $pageurlparams);
+$pageurl = new moodle_url('/mod/forum/report/summary/index.php', $pageurlparams);
 
 $PAGE->set_url($pageurl);
 $PAGE->set_pagelayout('report');
 $PAGE->set_title($title);
 $PAGE->set_heading($course->fullname);
-$PAGE->navbar->add(get_string('nodetitle', "forumreport_summary"));
+$PAGE->navbar->add(get_string('nodetitle', 'forumreport_summary'));
 
-$numforums = count($filters['forums']);
 $allowbulkoperations = !$download && !empty($CFG->messaging) && has_capability('moodle/course:bulkmessaging', $context);
 $canseeprivatereplies = false;
 $hasviewall = false;
@@ -119,7 +120,7 @@ foreach ($cms as $cm) {
     $forumcontext = \context_module::instance($cm->id);
 
     // This capability is required in at least one of the given contexts to view any version of the report.
-    if (has_capability("forumreport/summary:view", $forumcontext)) {
+    if (has_capability('forumreport/summary:view', $forumcontext)) {
         $canview = true;
     }
 
@@ -148,7 +149,7 @@ if ($numforums === $viewallcount) {
 
 // Prepare and display the report.
 $table = new \forumreport_summary\summary_table($courseid, $filters, $allowbulkoperations,
-        $canseeprivatereplies, $perpage, $canexport, $accessallforums);
+        $canseeprivatereplies, $perpage, $canexport, $iscoursereport, $accessallforums);
 $table->baseurl = $pageurl;
 
 $eventparams = [
@@ -173,9 +174,7 @@ if ($download) {
     }
 
     // Allow switching to course report (or other forum user has access to).
-    $forumselectoptions = [0 => get_string('forumselectcourseoption', 'forumreport_summary')] + $forumsvisibletouser;
+    $reporturl = new moodle_url('/mod/forum/report/summary/index.php', ['courseid' => $courseid]);
-    $reporturl = new moodle_url("/mod/forum/report/summary/index.php");
-    $reporturl->param('courseid', $courseid);
     $forumselect = new single_select($reporturl, 'forumid', $forumselectoptions, $forumid);
     $forumselect->set_label(get_string('forumselectlabel', 'forumreport_summary'));
     echo $OUTPUT->render($forumselect);