MDL-65033 mod_forum: Pass the favourites into the exporter

Query and pass the favouriting information into the exporter instead of within the exporter itself

lib/classes/external/exporter.php

@@ -79,14 +79,16 @@ public function __construct($data, $related = array()) {
             }
 
             $missingdataerr = 'Exporter class is missing required related data: (' . get_called_class() . ') ';
+            $scalartypes = ['string', 'int', 'bool', 'float'];
+            $scalarcheck = 'is_' . $classname;
 
-            if ($nullallowed && array_key_exists($key, $related) && $related[$key] === null) {
-                $this->related[$key] = $related[$key];
+            if ($nullallowed && (!array_key_exists($key, $related) || $related[$key] === null)) {
+                $this->related[$key] = null;
 
             } else if ($isarray) {
                 if (array_key_exists($key, $related) && is_array($related[$key])) {
                     foreach ($related[$key] as $index => $value) {
-                        if (!$value instanceof $classname) {
+                        if (!$value instanceof $classname && !$scalarcheck($value)) {
                             throw new coding_exception($missingdataerr . $key . ' => ' . $classname . '[]');
                         }
                     }
@@ -96,8 +98,6 @@ public function __construct($data, $related = array()) {
                 }
 
             } else {
-                $scalartypes = ['string', 'int', 'bool', 'float'];
-                $scalarcheck = 'is_' . $classname;
                 if (array_key_exists($key, $related) &&
                         ((in_array($classname, $scalartypes) && $scalarcheck($related[$key])) ||
                         ($related[$key] instanceof $classname))) {

lib/classes/output/icon_system_fontawesome.php

@@ -356,6 +356,7 @@ public function get_core_icon_map() {
             'core:t/edit' => 'fa-cog',
             'core:t/emailno' => 'fa-ban',
             'core:t/email' => 'fa-envelope-o',
+            'core:t/emptystar' => 'fa-star-o',
             'core:t/enrolusers' => 'fa-user-plus',
             'core:t/expanded' => 'fa-caret-down',
             'core:t/go' => 'fa-play',

lib/tests/exporter_test.php

@@ -47,15 +47,17 @@ public function setUp() {
             'context' => null,
             'aint' => 5,
             'astring' => 'valid string',
-            'abool' => false
+            'abool' => false,
+            'ints' => []
         );
         $this->invalidrelated = array(
             'simplestdClass' => 'a string',
             'arrayofstdClass' => 5,
             'context' => null,
             'aint' => false,
             'astring' => 4,
-            'abool' => 'not a boolean'
+            'abool' => 'not a boolean',
+            'ints' => null
         );
 
         $this->validdata = array('stringA' => 'A string', 'stringAformat' => FORMAT_HTML, 'intB' => 4);
@@ -118,6 +120,23 @@ public function test_invalid_related() {
         $result = $exporter->export($output);
     }
 
+    public function test_invalid_related_all_cases() {
+        global $PAGE;
+
+        foreach ($this->invalidrelated as $key => $value) {
+            $data = $this->validrelated;
+            $data[$key] = $value;
+
+            try {
+                $exporter = new core_testable_exporter($this->validdata, $data);
+                $output = $PAGE->get_renderer('core');
+                $result = $exporter->export($output);
+            } catch (coding_exception $e) {
+                $this->assertNotFalse(strpos($e->getMessage(), $key));
+            }
+        }
+    }
+
     public function test_valid_data_and_related() {
         global $PAGE;
         $output = $PAGE->get_renderer('core');
@@ -196,7 +215,7 @@ class core_testable_exporter extends \core\external\exporter {
     protected static function define_related() {
         // We cache the context so it does not need to be retrieved from the course.
         return array('simplestdClass' => 'stdClass', 'arrayofstdClass' => 'stdClass[]', 'context' => 'context?',
-            'astring' => 'string', 'abool' => 'bool', 'aint' => 'int');
+            'astring' => 'string', 'abool' => 'bool', 'aint' => 'int', 'ints' => 'int[]');
     }
 
     protected function get_other_values(renderer_base $output) {

mod/forum/amd/src/repository.js

@@ -91,7 +91,6 @@ define(['core/ajax'], function(Ajax) {
      * @param {number} forumid
      * @param {number} discussionid
      * @param {boolean} targetstate
-     * @param includetext
      * @return {*|Promise}
      */
     var setPinDiscussionState = function(forumid, discussionid, targetstate) {

mod/forum/classes/local/builders/exported_discussion.php

@@ -0,0 +1,151 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Exported discussion builder class.
+ *
+ * @package    mod_forum
+ * @copyright  2019 Peter Dias<[email protected]>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace mod_forum\local\builders;
+
+defined('MOODLE_INTERNAL') || die();
+
+use mod_forum\local\entities\discussion as discussion_entity;
+use mod_forum\local\entities\forum as forum_entity;
+use mod_forum\local\factories\legacy_data_mapper as legacy_data_mapper_factory;
+use mod_forum\local\factories\exporter as exporter_factory;
+use mod_forum\local\factories\vault as vault_factory;
+use rating_manager;
+use renderer_base;
+use stdClass;
+
+/**
+ * Exported discussion builder class
+ *
+ * This class is an implementation of the builder pattern (loosely). It is responsible
+ * for taking a set of related forums, discussions, and posts and generate the exported
+ * version of the discussion.
+ *
+ * It encapsulates the complexity involved with exporting discussions. All of the relevant
+ * additional resources will be loaded by this class in order to ensure the exporting
+ * process can happen.
+ *
+ * See this doc for more information on the builder pattern:
+ * https://designpatternsphp.readthedocs.io/en/latest/Creational/Builder/README.html
+ *
+ * @copyright  2019 Peter Dias<[email protected]>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class exported_discussion {
+    /** @var renderer_base $renderer Core renderer */
+    private $renderer;
+
+    /** @var legacy_data_mapper_factory $legacydatamapperfactory Data mapper factory */
+    private $legacydatamapperfactory;
+
+    /** @var exporter_factory $exporterfactory Exporter factory */
+    private $exporterfactory;
+
+    /** @var vault_factory $vaultfactory Vault factory */
+    private $vaultfactory;
+
+    /** @var rating_manager $ratingmanager Rating manager */
+    private $ratingmanager;
+
+    /**
+     * Constructor.
+     *
+     * @param renderer_base $renderer Core renderer
+     * @param legacy_data_mapper_factory $legacydatamapperfactory Legacy data mapper factory
+     * @param exporter_factory $exporterfactory Exporter factory
+     * @param vault_factory $vaultfactory Vault factory
+     * @param rating_manager $ratingmanager Rating manager
+     */
+    public function __construct(
+        renderer_base $renderer,
+        legacy_data_mapper_factory $legacydatamapperfactory,
+        exporter_factory $exporterfactory,
+        vault_factory $vaultfactory,
+        rating_manager $ratingmanager
+    ) {
+        $this->renderer = $renderer;
+        $this->legacydatamapperfactory = $legacydatamapperfactory;
+        $this->exporterfactory = $exporterfactory;
+        $this->vaultfactory = $vaultfactory;
+        $this->ratingmanager = $ratingmanager;
+    }
+
+    /**
+     * Build any additional variables for the exported discussion for a given set of discussions.
+     *
+     * This will typically be used for a list of discussions in the same forum.
+     *
+     * @param stdClass $user The user to export the posts for.
+     * @param forum_entity $forum The forum that each of the $discussions belong to
+     * @param discussion_entity $discussion A list of all discussions that each of the $posts belong to
+     * @return stdClass[] List of exported posts in the same order as the $posts array.
+     */
+    public function build(
+        stdClass $user,
+        forum_entity $forum,
+        discussion_entity $discussion
+    ) : array {
+
+        $favouriteids = [];
+        if ($this->is_favourited($discussion, $forum->get_context(), $user)) {
+            $favouriteids[] = $discussion->get_id();
+        }
+
+        $groupsbyid = $this->get_groups_available_in_forum($forum);
+        $discussionexporter = $this->exporterfactory->get_discussion_exporter(
+            $user, $forum, $discussion, $groupsbyid, $favouriteids
+        );
+
+        return (array) $discussionexporter->export($this->renderer);
+    }
+
+    /**
+     * Get the groups details for all groups available to the forum.
+     * @param forum_entity $forum The forum entity
+     * @return stdClass[]
+     */
+    private function get_groups_available_in_forum($forum) : array {
+        $course = $forum->get_course_record();
+        $coursemodule = $forum->get_course_module_record();
+
+        return groups_get_all_groups($course->id, 0, $coursemodule->groupingid);
+    }
+
+    /**
+     * Check whether the provided discussion has been favourited by the user.
+     *
+     * @param discussion_entity $discussion The discussion record
+     * @param \context_module $forumcontext Forum context
+     * @param \stdClass $user The user to check the favourite against
+     *
+     * @return bool Whether or not the user has favourited the discussion
+     */
+    public function is_favourited(discussion_entity $discussion, \context_module $forumcontext, \stdClass $user) {
+        $usercontext = \context_user::instance($user->id);
+        $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
+        return $ufservice->favourite_exists('mod_forum', 'discussions', $discussion->get_id(), $forumcontext);
+    }
+
+
+}

mod/forum/classes/local/builders/exported_discussion_summaries.php

@@ -127,7 +127,7 @@ public function build(
         $latestposts = $postvault->get_latest_post_id_for_discussion_ids($user, $discussionids, $canseeanyprivatereply);
 
         $unreadcounts = [];
-
+        $favourites = $this->get_favourites($user);
         $forumdatamapper = $this->legacydatamapperfactory->get_forum_data_mapper();
         $forumrecord = $forumdatamapper->to_legacy_object($forum);
 
@@ -143,12 +143,31 @@ public function build(
             $groupsbyauthorid,
             $replycounts,
             $unreadcounts,
-            $latestposts
+            $latestposts,
+            $favourites
         );
 
         return (array) $summaryexporter->export($this->renderer);
     }
 
+    /**
+     * Get a list of all favourited discussions.
+     *
+     * @param stdClass $user The user we are getting favourites for
+     * @return int[] A list of favourited itemids
+     */
+    private function get_favourites(stdClass $user) : array {
+        $usercontext = \context_user::instance($user->id);
+        $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
+        $favourites = $ufservice->find_favourites_by_type('mod_forum', 'discussions');
+        $ids = [];
+        foreach ($favourites as $favourite) {
+            $ids[] = $favourite->itemid;
+        }
+
+        return $ids;
+    }
+
     /**
      * Get the groups details for all groups available to the forum.
      * @param forum_entity $forum The forum entity

mod/forum/classes/local/entities/discussion.php

@@ -332,19 +332,4 @@ public function is_timed_discussion() : bool {
     public function is_timed_discussion_visible() : bool {
         return !$this->is_timed_discussion() || ($this->has_started() && !$this->has_ended());
     }
-
-    /**
-     * Check whether the provided discussion has been favourited by the user.
-     *
-     * @param discussion $discussion The discussion record
-     * @param context $forumcontext Forum context
-     * @param \stdClass $user The user to check the favourite against
-     *
-     * @return bool Whether or not the user has favourited the discussion
-     */
-    public static function is_favourited(discussion $discussion, \context_module $forumcontext, \stdClass $user) {
-        $usercontext = \context_user::instance($user->id);
-        $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
-        return $ufservice->favourite_exists('mod_forum', 'discussions', $discussion->get_id(), $forumcontext);
-    }
 }

mod/forum/classes/local/exporters/discussion.php

@@ -157,6 +157,7 @@ protected function get_other_values(renderer_base $output) {
 
         $capabilitymanager = $this->related['capabilitymanager'];
         $urlfactory = $this->related['urlfactory'];
+        $favouriteids = isset($this->related['favouriteids']) ? $this->related['favouriteids'] : [];
 
         $forum = $this->related['forum'];
         $forumrecord = $this->get_forum_record();
@@ -202,7 +203,7 @@ protected function get_other_values(renderer_base $output) {
             ],
             'userstate' => [
                 'subscribed' => \mod_forum\subscriptions::is_subscribed($user->id, $forumrecord, $discussion->get_id()),
-                'favourited' => $this->is_favourited($discussion, $forum->get_context(), $user),
+                'favourited' => in_array($discussion->get_id(), $favouriteids) ? true : false,
             ],
             'capabilities' => [
                 'subscribe' => $capabilitymanager->can_subscribe_to_discussion($user, $discussion),
@@ -243,21 +244,6 @@ protected function get_other_values(renderer_base $output) {
         return $data;
     }
 
-    /**
-     * Check whether the provided discussion has been favourited by the user.
-     *
-     * @param discussion $discussion The discussion record
-     * @param context $forumcontext Forum context
-     * @param \stdClass $user The user to check the favourite against
-     *
-     * @return bool Whether or not the user has favourited the discussion
-     */
-    private function is_favourited(discussion_entity $discussion, \context_module $forumcontext, \stdClass $user) {
-        $usercontext = \context_user::instance($user->id);
-        $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
-        return $ufservice->favourite_exists('mod_forum', 'discussions', $discussion->get_id(), $forumcontext);
-    }
-
     /**
      * Get the legacy forum record from the forum entity.
      *
@@ -282,7 +268,8 @@ protected static function define_related() {
             'urlfactory' => 'mod_forum\local\factories\url',
             'user' => 'stdClass',
             'groupsbyid' => 'stdClass[]',
-            'latestpostid' => 'int?'
+            'latestpostid' => 'int?',
+            'favouriteids' => 'int[]?'
         ];
     }
 }

mod/forum/classes/local/exporters/discussion_summaries.php

@@ -152,6 +152,7 @@ protected static function define_related() {
             'capabilitymanager' => 'mod_forum\local\managers\capability',
             'urlfactory' => 'mod_forum\local\factories\url',
             'user' => 'stdClass',
+            'favouriteids' => 'int[]?'
         ];
     }
 }

mod/forum/classes/local/exporters/discussion_summary.php

@@ -175,6 +175,7 @@ protected static function define_related() {
             'capabilitymanager' => 'mod_forum\local\managers\capability',
             'urlfactory' => 'mod_forum\local\factories\url',
             'user' => 'stdClass',
+            'favouriteids' => 'int[]?'
         ];
     }
 }