From 13cd05ac14fae641fea573e7c5d602a2770e3654 Mon Sep 17 00:00:00 2001 From: Peter Date: Mon, 25 Mar 2019 08:16:14 +0800 Subject: [PATCH] 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 | 10 +- .../output/icon_system_fontawesome.php | 1 + lib/tests/exporter_test.php | 25 ++- mod/forum/amd/src/repository.js | 1 - .../local/builders/exported_discussion.php | 151 ++++++++++++++++++ .../exported_discussion_summaries.php | 23 ++- .../classes/local/entities/discussion.php | 15 -- .../classes/local/exporters/discussion.php | 21 +-- .../local/exporters/discussion_summaries.php | 1 + .../local/exporters/discussion_summary.php | 1 + mod/forum/classes/local/factories/builder.php | 16 ++ .../classes/local/factories/exporter.php | 10 +- .../classes/local/factories/renderer.php | 10 +- .../classes/local/renderers/discussion.php | 13 +- .../classes/local/vaults/discussion_list.php | 13 +- mod/forum/externallib.php | 6 +- mod/forum/lib.php | 1 - ...forum_discussion_favourite_toggle.mustache | 2 +- mod/forum/tests/exporters_discussion_test.php | 48 ------ mod/forum/tests/externallib_test.php | 12 +- .../pix/t/unstar.png => pix/t/emptystar.png | Bin .../pix/t/unstar.svg => pix/t/emptystar.svg | 0 22 files changed, 268 insertions(+), 112 deletions(-) create mode 100644 mod/forum/classes/local/builders/exported_discussion.php rename mod/forum/pix/t/unstar.png => pix/t/emptystar.png (100%) rename mod/forum/pix/t/unstar.svg => pix/t/emptystar.svg (100%) diff --git a/lib/classes/external/exporter.php b/lib/classes/external/exporter.php index 2eee814846d..6195288734f 100644 --- a/lib/classes/external/exporter.php +++ b/lib/classes/external/exporter.php @@ -79,14 +79,16 @@ abstract class exporter { } $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 @@ abstract class exporter { } } 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))) { diff --git a/lib/classes/output/icon_system_fontawesome.php b/lib/classes/output/icon_system_fontawesome.php index 249961ee9ac..aa138d6d061 100644 --- a/lib/classes/output/icon_system_fontawesome.php +++ b/lib/classes/output/icon_system_fontawesome.php @@ -356,6 +356,7 @@ class icon_system_fontawesome extends icon_system_font { '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', diff --git a/lib/tests/exporter_test.php b/lib/tests/exporter_test.php index 57929ca6d8a..b56cd130655 100644 --- a/lib/tests/exporter_test.php +++ b/lib/tests/exporter_test.php @@ -47,7 +47,8 @@ class core_exporter_testcase extends advanced_testcase { 'context' => null, 'aint' => 5, 'astring' => 'valid string', - 'abool' => false + 'abool' => false, + 'ints' => [] ); $this->invalidrelated = array( 'simplestdClass' => 'a string', @@ -55,7 +56,8 @@ class core_exporter_testcase extends advanced_testcase { '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 @@ class core_exporter_testcase extends advanced_testcase { $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) { diff --git a/mod/forum/amd/src/repository.js b/mod/forum/amd/src/repository.js index fb5aef13725..e7d32afb27e 100644 --- a/mod/forum/amd/src/repository.js +++ b/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) { diff --git a/mod/forum/classes/local/builders/exported_discussion.php b/mod/forum/classes/local/builders/exported_discussion.php new file mode 100644 index 00000000000..0ebdc09c9b2 --- /dev/null +++ b/mod/forum/classes/local/builders/exported_discussion.php @@ -0,0 +1,151 @@ +. + +/** + * Exported discussion builder class. + * + * @package mod_forum + * @copyright 2019 Peter Dias + * @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 + * @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); + } + + +} diff --git a/mod/forum/classes/local/builders/exported_discussion_summaries.php b/mod/forum/classes/local/builders/exported_discussion_summaries.php index ba1e0aa862d..5df95be9af3 100644 --- a/mod/forum/classes/local/builders/exported_discussion_summaries.php +++ b/mod/forum/classes/local/builders/exported_discussion_summaries.php @@ -127,7 +127,7 @@ class exported_discussion_summaries { $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 @@ class exported_discussion_summaries { $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 diff --git a/mod/forum/classes/local/entities/discussion.php b/mod/forum/classes/local/entities/discussion.php index affc7c0bbba..13daac36d8b 100644 --- a/mod/forum/classes/local/entities/discussion.php +++ b/mod/forum/classes/local/entities/discussion.php @@ -332,19 +332,4 @@ class discussion { 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); - } } diff --git a/mod/forum/classes/local/exporters/discussion.php b/mod/forum/classes/local/exporters/discussion.php index de0ed1ab2c7..5f799d0601d 100644 --- a/mod/forum/classes/local/exporters/discussion.php +++ b/mod/forum/classes/local/exporters/discussion.php @@ -157,6 +157,7 @@ class discussion extends exporter { $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 @@ class discussion extends exporter { ], '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 @@ class discussion extends exporter { 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 @@ class discussion extends exporter { 'urlfactory' => 'mod_forum\local\factories\url', 'user' => 'stdClass', 'groupsbyid' => 'stdClass[]', - 'latestpostid' => 'int?' + 'latestpostid' => 'int?', + 'favouriteids' => 'int[]?' ]; } } diff --git a/mod/forum/classes/local/exporters/discussion_summaries.php b/mod/forum/classes/local/exporters/discussion_summaries.php index ded471d3f01..13eac7e8527 100644 --- a/mod/forum/classes/local/exporters/discussion_summaries.php +++ b/mod/forum/classes/local/exporters/discussion_summaries.php @@ -152,6 +152,7 @@ class discussion_summaries extends exporter { 'capabilitymanager' => 'mod_forum\local\managers\capability', 'urlfactory' => 'mod_forum\local\factories\url', 'user' => 'stdClass', + 'favouriteids' => 'int[]?' ]; } } diff --git a/mod/forum/classes/local/exporters/discussion_summary.php b/mod/forum/classes/local/exporters/discussion_summary.php index 1a7ac8e07db..6ba2bcb531c 100644 --- a/mod/forum/classes/local/exporters/discussion_summary.php +++ b/mod/forum/classes/local/exporters/discussion_summary.php @@ -175,6 +175,7 @@ class discussion_summary extends exporter { 'capabilitymanager' => 'mod_forum\local\managers\capability', 'urlfactory' => 'mod_forum\local\factories\url', 'user' => 'stdClass', + 'favouriteids' => 'int[]?' ]; } } diff --git a/mod/forum/classes/local/factories/builder.php b/mod/forum/classes/local/factories/builder.php index 2a71d5fb2c7..a648d1efdc0 100644 --- a/mod/forum/classes/local/factories/builder.php +++ b/mod/forum/classes/local/factories/builder.php @@ -28,6 +28,7 @@ defined('MOODLE_INTERNAL') || die(); use mod_forum\local\builders\exported_posts as exported_posts_builder; use mod_forum\local\builders\exported_discussion_summaries as exported_discussion_summaries_builder; +use mod_forum\local\builders\exported_discussion as exported_discussion_builder; use mod_forum\local\factories\vault as vault_factory; use mod_forum\local\factories\legacy_data_mapper as legacy_data_mapper_factory; use mod_forum\local\factories\exporter as exporter_factory; @@ -108,4 +109,19 @@ class builder { $this->managerfactory ); } + + /** + * Get an instance of the exported discussion builder. + * + * @return exported_discussion_summaries_builder + */ + public function get_exported_discussion_builder() : exported_discussion_builder { + return new exported_discussion_builder( + $this->rendererbase, + $this->legacydatamapperfactory, + $this->exporterfactory, + $this->vaultfactory, + $this->managerfactory->get_rating_manager() + ); + } } diff --git a/mod/forum/classes/local/factories/exporter.php b/mod/forum/classes/local/factories/exporter.php index 3d071bba6e2..8aaea94e265 100644 --- a/mod/forum/classes/local/factories/exporter.php +++ b/mod/forum/classes/local/factories/exporter.php @@ -122,7 +122,8 @@ class exporter { stdClass $user, forum_entity $forum, discussion_entity $discussion, - array $groupsbyid = [] + array $groupsbyid = [], + array $favouriteids = [] ) : discussion_exporter { return new discussion_exporter($discussion, [ 'context' => $forum->get_context(), @@ -132,7 +133,8 @@ class exporter { 'user' => $user, 'legacydatamapperfactory' => $this->legacydatamapperfactory, 'latestpostid' => null, - 'groupsbyid' => $groupsbyid + 'groupsbyid' => $groupsbyid, + 'favouriteids' => $favouriteids ]); } @@ -166,7 +168,8 @@ class exporter { array $groupsbyauthorid = [], array $discussionreplycount = [], array $discussionunreadcount = [], - array $latestpostid = [] + array $latestpostid = [], + array $favourites = [] ) : discussion_summaries_exporter { return new discussion_summaries_exporter( $discussions, @@ -182,6 +185,7 @@ class exporter { 'capabilitymanager' => $this->managerfactory->get_capability_manager($forum), 'urlfactory' => $this->urlfactory, 'user' => $user, + 'favouriteids' => $favourites ] ); } diff --git a/mod/forum/classes/local/factories/renderer.php b/mod/forum/classes/local/factories/renderer.php index 75c39a3153e..a8a7b0d3fa4 100644 --- a/mod/forum/classes/local/factories/renderer.php +++ b/mod/forum/classes/local/factories/renderer.php @@ -138,7 +138,15 @@ class renderer { $ratingmanager, $this->entityfactory->get_exported_posts_sorter(), $baseurl, - $notifications + $notifications, + function($discussion, $user, $forum) { + $exportbuilder = $this->builderfactory->get_exported_discussion_builder(); + return $exportedposts = $exportbuilder->build( + $user, + $forum, + $discussion + ); + } ); } diff --git a/mod/forum/classes/local/renderers/discussion.php b/mod/forum/classes/local/renderers/discussion.php index 81d6bd80e2b..8b7e6249186 100644 --- a/mod/forum/classes/local/renderers/discussion.php +++ b/mod/forum/classes/local/renderers/discussion.php @@ -90,6 +90,8 @@ class discussion { private $notifications; /** @var sorter_entity $exportedpostsorter Sorter for the exported posts */ private $exportedpostsorter; + /** @var callable $postprocessfortemplate Function to process exported posts before template rendering */ + private $postprocessfortemplate; /** * Constructor. @@ -123,7 +125,8 @@ class discussion { rating_manager $ratingmanager, sorter_entity $exportedpostsorter, moodle_url $baseurl, - array $notifications = [] + array $notifications = [], + callable $postprocessfortemplate = null ) { $this->forum = $forum; $this->discussion = $discussion; @@ -140,6 +143,7 @@ class discussion { $this->notifications = $notifications; $this->exportedpostsorter = $exportedpostsorter; + $this->postprocessfortemplate = $postprocessfortemplate; $forumdatamapper = $this->legacydatamapperfactory->get_forum_data_mapper(); $this->forumrecord = $forumdatamapper->to_legacy_object($forum); @@ -173,7 +177,12 @@ class discussion { $posts = array_merge([$firstpost], array_values($replies)); - $exporteddiscussion = $this->get_exported_discussion($user); + if ($this->postprocessfortemplate !== null) { + $exporteddiscussion = ($this->postprocessfortemplate) ($this->discussion, $user, $this->forum); + } else { + $exporteddiscussion = $this->get_exported_discussion($user); + } + $exporteddiscussion = array_merge($exporteddiscussion, [ 'notifications' => $this->get_notifications($user), 'html' => [ diff --git a/mod/forum/classes/local/vaults/discussion_list.php b/mod/forum/classes/local/vaults/discussion_list.php index 9e2919e5309..ee7342a3736 100644 --- a/mod/forum/classes/local/vaults/discussion_list.php +++ b/mod/forum/classes/local/vaults/discussion_list.php @@ -97,9 +97,11 @@ class discussion_list extends db_table_vault { $alias = $this->get_table_alias(); $db = $this->get_db(); - list($favsql, $favparams) = $this->get_favourite_sql($user); - foreach ($favparams as $key => $param) { - $favsql = str_replace(":$key", "'$param'", $favsql); + if ($user) { + list($favsql, $favparams) = $this->get_favourite_sql($user); + foreach ($favparams as $key => $param) { + $favsql = str_replace(":$key", "'$param'", $favsql); + } } // Fetch: @@ -412,8 +414,11 @@ class discussion_list extends db_table_vault { /** * Get the standard favouriting sql. + * + * @param stdClass $user The user we are getting the sql for + * @return [$sql, $params] An array comprising of the sql and any associated params */ - private function get_favourite_sql($user): array { + private function get_favourite_sql(stdClass $user): array { $usercontext = \context_user::instance($user->id); $alias = $this->get_table_alias(); $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext); diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index bda06a490c0..4b88a315ba3 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -1127,8 +1127,6 @@ class mod_forum_external extends external_api { $managerfactory = mod_forum\local\container::get_manager_factory(); $capabilitymanager = $managerfactory->get_capability_manager($forum); - - // Does the user have the ability to favourite the discussion? if (!$capabilitymanager->can_favourite_discussion($USER, $discussion)) { throw new moodle_exception('cannotfavourite', 'forum'); @@ -1143,7 +1141,9 @@ class mod_forum_external extends external_api { } $exporterfactory = mod_forum\local\container::get_exporter_factory(); - $exporter = $exporterfactory->get_discussion_exporter($USER, $forum, $discussion); + $builder = mod_forum\local\container::get_builder_factory()->get_exported_discussion_builder(); + $favourited = ($builder->is_favourited($discussion, $forumcontext, $USER) ? [$discussion->get_id()] : []); + $exporter = $exporterfactory->get_discussion_exporter($USER, $forum, $discussion, [], $favourited); return $exporter->export($PAGE->get_renderer('mod_forum')); } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index b726fae5df3..c2cd20d9a09 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -6437,7 +6437,6 @@ function mod_forum_get_fontawesome_icon_map() { 'mod_forum:t/selected' => 'fa-check', 'mod_forum:t/subscribed' => 'fa-envelope-o', 'mod_forum:t/unsubscribed' => 'fa-envelope-open-o', - 'mod_forum:t/emptystar' => 'fa-star-o', 'mod_forum:t/star' => 'fa-star', ]; } diff --git a/mod/forum/templates/forum_discussion_favourite_toggle.mustache b/mod/forum/templates/forum_discussion_favourite_toggle.mustache index 8b460b2dc1c..c1c93ad4b6a 100644 --- a/mod/forum/templates/forum_discussion_favourite_toggle.mustache +++ b/mod/forum/templates/forum_discussion_favourite_toggle.mustache @@ -35,7 +35,7 @@ {{#pix}}t/star, mod_forum, {{#str}}removefromfavourites, mod_forum{{/str}}{{/pix}} {{/userstate.favourited}} {{^userstate.favourited}} - {{#pix}}t/emptystar, mod_forum, {{#str}}addtofavourites, mod_forum{{/str}}{{/pix}} + {{#pix}}t/emptystar, core, {{#str}}addtofavourites, mod_forum{{/str}}{{/pix}} {{/userstate.favourited}} {{/favouritecontent}} {{/ mod_forum/discussion_favourite_toggle }} diff --git a/mod/forum/tests/exporters_discussion_test.php b/mod/forum/tests/exporters_discussion_test.php index c6b9b0ca38d..7290395fc85 100644 --- a/mod/forum/tests/exporters_discussion_test.php +++ b/mod/forum/tests/exporters_discussion_test.php @@ -113,52 +113,4 @@ class mod_forum_exporters_discussion_testcase extends advanced_testcase { $this->assertEquals(0, $exporteddiscussion->times['end']); $this->assertEquals($group->name, $exporteddiscussion->group['name']); } - - public function test_is_favourited() { - $this->resetAfterTest(true); - - $time = time() + 10; - // Create a user. - $user = self::getDataGenerator()->create_user(array('trackforums' => 1)); - - // Set to the user. - self::setUser($user); - - // Create courses to add the modules. - $course1 = self::getDataGenerator()->create_course(); - $this->getDataGenerator()->enrol_user($user->id, $course1->id); - - $record = new stdClass(); - $record->introformat = FORMAT_HTML; - $record->course = $course1->id; - $record->trackingtype = FORUM_TRACKING_OFF; - $forum1 = self::getDataGenerator()->create_module('forum', $record); - - $discussion = new discussion_entity( - 1, - $course1->id, - $forum1->id, - 'test discussion', - 4, - 5, - 6, - false, - $time, - $time, - 0, - 0, - false - ); - $coursemodule = get_coursemodule_from_instance('forum', $forum1->id); - $contextmodule = context_module::instance($coursemodule->id); - - $this->assertFalse(\mod_forum\local\entities\discussion::is_favourited($discussion, $contextmodule, $user)); - - // Toggle the favourite for discussion. - $usercontext = \context_user::instance($user->id); - $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext); - $ufservice->create_favourite('mod_forum', 'discussions', $discussion->get_id(), $contextmodule); - - $this->assertTrue(\mod_forum\local\entities\discussion::is_favourited($discussion, $contextmodule, $user)); - } } diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index 9549f30e7d2..44ea7d89c23 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -212,11 +212,11 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $record->forum = $forum1->id; $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - $response = mod_forum_external::toggle_favourite_state($forum1->id, $discussion1->id, 1); + $response = mod_forum_external::toggle_favourite_state($discussion1->id, 1); $response = external_api::clean_returnvalue(mod_forum_external::toggle_favourite_state_returns(), $response); $this->assertTrue($response['userstate']['favourited']); - $response = mod_forum_external::toggle_favourite_state($forum1->id, $discussion1->id, 0); + $response = mod_forum_external::toggle_favourite_state($discussion1->id, 0); $response = external_api::clean_returnvalue(mod_forum_external::toggle_favourite_state_returns(), $response); $this->assertFalse($response['userstate']['favourited']); } @@ -252,17 +252,17 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); try { - $response = mod_forum_external::set_pin_state($forum1->id, $discussion1->id, 1); + $response = mod_forum_external::set_pin_state($discussion1->id, 1); } catch (Exception $e) { $this->assertEquals('cannotpindiscussions', $e->errorcode); } self::setAdminUser(); - $response = mod_forum_external::set_pin_state($forum1->id, $discussion1->id, 1); + $response = mod_forum_external::set_pin_state($discussion1->id, 1); $response = external_api::clean_returnvalue(mod_forum_external::set_pin_state_returns(), $response); $this->assertTrue($response['pinned']); - $response = mod_forum_external::set_pin_state($forum1->id, $discussion1->id, 0); + $response = mod_forum_external::set_pin_state($discussion1->id, 0); $response = external_api::clean_returnvalue(mod_forum_external::set_pin_state_returns(), $response); $this->assertFalse($response['pinned']); } @@ -1754,7 +1754,7 @@ class mod_forum_external_testcase extends externallib_advanced_testcase { // Check default values for capabilities. $enabledcaps = array('canviewdiscussion', 'canstartdiscussion', 'canreplypost', 'canviewrating', 'cancreateattachment', - 'canexportownpost', 'candeleteownpost', 'canallowforcesubscribe'); + 'canexportownpost', 'cancantogglefavourite', 'candeleteownpost', 'canallowforcesubscribe'); unset($result['warnings']); foreach ($result as $capname => $capvalue) { diff --git a/mod/forum/pix/t/unstar.png b/pix/t/emptystar.png similarity index 100% rename from mod/forum/pix/t/unstar.png rename to pix/t/emptystar.png diff --git a/mod/forum/pix/t/unstar.svg b/pix/t/emptystar.svg similarity index 100% rename from mod/forum/pix/t/unstar.svg rename to pix/t/emptystar.svg