From 8885cd573a9ca907afefbcc1682acc23debbfa18 Mon Sep 17 00:00:00 2001 From: Peter Date: Fri, 22 Mar 2019 10:40:57 +0800 Subject: [PATCH] MDL-65033 mod_forum: Feedback updates --- mod/forum/amd/build/discussion_list.min.js | 2 +- mod/forum/amd/build/favourite_toggle.min.js | 2 +- mod/forum/amd/build/pin_toggle.min.js | 2 +- mod/forum/amd/build/repository.min.js | 2 +- mod/forum/amd/src/discussion_list.js | 7 +-- mod/forum/amd/src/favourite_toggle.js | 2 +- mod/forum/amd/src/pin_toggle.js | 7 +-- mod/forum/amd/src/repository.js | 24 ++++++---- .../classes/local/entities/discussion.php | 4 +- .../classes/local/exporters/discussion.php | 17 ++++++- .../classes/local/renderers/discussion.php | 30 ------------ .../local/renderers/discussion_list.php | 6 ++- mod/forum/classes/local/vaults/author.php | 3 +- .../classes/local/vaults/db_table_vault.php | 3 +- mod/forum/classes/local/vaults/discussion.php | 3 +- .../classes/local/vaults/discussion_list.php | 33 ++++++------- mod/forum/classes/local/vaults/forum.php | 3 +- mod/forum/classes/local/vaults/post.php | 3 +- .../vaults/post_read_receipt_collection.php | 3 +- mod/forum/db/access.php | 2 +- mod/forum/externallib.php | 30 +++++------- mod/forum/lib.php | 3 +- .../discussion_favourite_toggle.mustache | 4 +- .../templates/discussion_pin_toggle.mustache | 2 + mod/forum/templates/forum_discussion.mustache | 12 ++--- ...forum_discussion_favourite_toggle.mustache | 4 +- mod/forum/tests/entities_discussion_test.php | 48 ------------------- mod/forum/tests/exporters_discussion_test.php | 48 +++++++++++++++++++ 28 files changed, 151 insertions(+), 158 deletions(-) diff --git a/mod/forum/amd/build/discussion_list.min.js b/mod/forum/amd/build/discussion_list.min.js index 9b3d50bd211..c2084fc6bcc 100644 --- a/mod/forum/amd/build/discussion_list.min.js +++ b/mod/forum/amd/build/discussion_list.min.js @@ -1 +1 @@ -define(["jquery","mod_forum/subscription_toggle","mod_forum/selectors","mod_forum/repository"],function(a,b,c,d){var e=function(b){b.on("click",c.favourite.toggle,function(){var b=a(this),c=b.data("forumid"),e=b.data("discussionid"),f=b.data("targetstate");d.toggleFavouriteDiscussionState(c,e,f).then(function(){location.reload()})["catch"](Notification.exception)}),b.on("click",c.pin.toggle,function(){var b=a(this),c=b.data("forumid"),e=b.data("discussionid"),f=b.data("targetstate");d.togglePin(c,e,f).then(function(){location.reload()})["catch"](Notification.exception)})};return{init:function(a){b.init(a),e(a)}}}); \ No newline at end of file +define(["jquery","mod_forum/subscription_toggle","mod_forum/selectors","mod_forum/repository"],function(a,b,c,d){var e=function(b){b.on("click",c.favourite.toggle,function(){var b=a(this),c=b.data("forumid"),e=b.data("discussionid"),f=b.data("targetstate");d.setFavouriteDiscussionState(c,e,f).then(function(){location.reload()})["catch"](Notification.exception)}),b.on("click",c.pin.toggle,function(b){b.preventDefault();var c=a(this),e=c.data("forumid"),f=c.data("discussionid"),g=c.data("targetstate");d.setPinDiscussionState(e,f,g).then(function(){location.reload()})["catch"](Notification.exception)})};return{init:function(a){b.init(a),e(a)}}}); \ No newline at end of file diff --git a/mod/forum/amd/build/favourite_toggle.min.js b/mod/forum/amd/build/favourite_toggle.min.js index ffde663e8ab..f9729cb8706 100644 --- a/mod/forum/amd/build/favourite_toggle.min.js +++ b/mod/forum/amd/build/favourite_toggle.min.js @@ -1 +1 @@ -define(["jquery","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e){var f=function(f){f.on("click",e.favourite.toggle,function(e){var f=a(this),g=f.data("forumid"),h=f.data("discussionid"),i=f.data("targetstate");d.toggleFavouriteDiscussionState(g,h,i).then(function(a){return b.render("mod_forum/discussion_favourite_toggle",a)}).then(function(a,c){return b.replaceNode(f,a,c)})["catch"](c.exception),e.preventDefault()})};return{init:function(a){f(a)}}}); \ No newline at end of file +define(["jquery","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e){var f=function(f){f.on("click",e.favourite.toggle,function(e){var f=a(this),g=f.data("forumid"),h=f.data("discussionid"),i=f.data("targetstate");d.setFavouriteDiscussionState(g,h,i).then(function(a){return b.render("mod_forum/discussion_favourite_toggle",a)}).then(function(a,c){return b.replaceNode(f,a,c)})["catch"](c.exception),e.preventDefault()})};return{init:function(a){f(a)}}}); \ No newline at end of file diff --git a/mod/forum/amd/build/pin_toggle.min.js b/mod/forum/amd/build/pin_toggle.min.js index a7742528046..d230fde9e9d 100644 --- a/mod/forum/amd/build/pin_toggle.min.js +++ b/mod/forum/amd/build/pin_toggle.min.js @@ -1 +1 @@ -define(["jquery","core/ajax","core/str","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e,f,g){var h=function(b){b.on("click",g.pin.toggle,function(b){var c=a(this),g=c.data("forumid"),h=c.data("discussionid"),i=c.data("targetstate"),j=c.data("includetext");f.togglePin(g,h,i,j).then(function(a){return d.render("mod_forum/discussion_pin_toggle",a)}).then(function(a,b){return d.replaceNode(c,a,b)}).then(function(){return e.fetchNotifications()}).fail(e.exception),b.preventDefault()})};return{init:function(a){h(a)}}}); \ No newline at end of file +define(["jquery","core/ajax","core/str","core/templates","core/notification","mod_forum/repository","mod_forum/selectors"],function(a,b,c,d,e,f,g){var h=function(b){b.on("click",g.pin.toggle,function(b){var c=a(this),g=c.data("forumid"),h=c.data("discussionid"),i=c.data("targetstate");f.setPinDiscussionState(g,h,i).then(function(a){return d.render("mod_forum/discussion_pin_toggle",a)}).then(function(a,b){return d.replaceNode(c,a,b)}).fail(e.exception),b.preventDefault()})};return{init:function(a){h(a)}}}); \ No newline at end of file diff --git a/mod/forum/amd/build/repository.min.js b/mod/forum/amd/build/repository.min.js index 208bd20e449..b20b5325704 100644 --- a/mod/forum/amd/build/repository.min.js +++ b/mod/forum/amd/build/repository.min.js @@ -1 +1 @@ -define(["core/ajax"],function(a){var b=function(b,c,d){var e={methodname:"mod_forum_set_subscription_state",args:{forumid:b,discussionid:c,targetstate:d}};return a.call([e])[0]},c=function(b,c,d){var e={methodname:"mod_forum_add_discussion_post",args:{postid:b,message:d,subject:c}};return a.call([e])[0]},d=function(b,c,d){var e={methodname:"mod_forum_toggle_favourite_state",args:{forumid:b,discussionid:c,targetstate:d}};return a.call([e])[0]},e=function(b,c,d){var e={methodname:"mod_forum_set_lock_state",args:{forumid:b,discussionid:c,targetstate:d}};return a.call([e])[0]},f=function(b,c,d,e){var f={methodname:"mod_forum_set_pin_state",args:{forumid:b,discussionid:c,targetstate:d,includetext:e}};return a.call([f])[0]};return{setDiscussionSubscriptionState:b,addDiscussionPost:c,setDiscussionLockState:e,toggleFavouriteDiscussionState:d,togglePin:f}}); \ No newline at end of file +define(["core/ajax"],function(a){var b=function(b,c,d){var e={methodname:"mod_forum_set_subscription_state",args:{forumid:b,discussionid:c,targetstate:d}};return a.call([e])[0]},c=function(b,c,d){var e={methodname:"mod_forum_add_discussion_post",args:{postid:b,message:d,subject:c}};return a.call([e])[0]},d=function(b,c,d){var e={methodname:"mod_forum_toggle_favourite_state",args:{discussionid:c,targetstate:d}};return a.call([e])[0]},e=function(b,c,d){var e={methodname:"mod_forum_set_lock_state",args:{forumid:b,discussionid:c,targetstate:d}};return a.call([e])[0]},f=function(b,c,d){var e={methodname:"mod_forum_set_pin_state",args:{discussionid:c,targetstate:d}};return a.call([e])[0]};return{setDiscussionSubscriptionState:b,addDiscussionPost:c,setDiscussionLockState:e,setFavouriteDiscussionState:d,setPinDiscussionState:f}}); \ No newline at end of file diff --git a/mod/forum/amd/src/discussion_list.js b/mod/forum/amd/src/discussion_list.js index 12783b88301..35007ad381a 100644 --- a/mod/forum/amd/src/discussion_list.js +++ b/mod/forum/amd/src/discussion_list.js @@ -38,19 +38,20 @@ define([ var forumId = toggleElement.data('forumid'); var discussionId = toggleElement.data('discussionid'); var subscriptionState = toggleElement.data('targetstate'); - Repository.toggleFavouriteDiscussionState(forumId, discussionId, subscriptionState) + Repository.setFavouriteDiscussionState(forumId, discussionId, subscriptionState) .then(function() { location.reload(); }) .catch(Notification.exception); }); - root.on('click', Selectors.pin.toggle, function() { + root.on('click', Selectors.pin.toggle, function(e) { + e.preventDefault(); var toggleElement = $(this); var forumId = toggleElement.data('forumid'); var discussionId = toggleElement.data('discussionid'); var state = toggleElement.data('targetstate'); - Repository.togglePin(forumId, discussionId, state) + Repository.setPinDiscussionState(forumId, discussionId, state) .then(function() { location.reload(); }) diff --git a/mod/forum/amd/src/favourite_toggle.js b/mod/forum/amd/src/favourite_toggle.js index 4fe7bab8a57..e613e07d37e 100644 --- a/mod/forum/amd/src/favourite_toggle.js +++ b/mod/forum/amd/src/favourite_toggle.js @@ -48,7 +48,7 @@ define([ var discussionId = toggleElement.data('discussionid'); var subscriptionState = toggleElement.data('targetstate'); - Repository.toggleFavouriteDiscussionState(forumId, discussionId, subscriptionState) + Repository.setFavouriteDiscussionState(forumId, discussionId, subscriptionState) .then(function(context) { return Templates.render('mod_forum/discussion_favourite_toggle', context); }) diff --git a/mod/forum/amd/src/pin_toggle.js b/mod/forum/amd/src/pin_toggle.js index 86dff2ce026..7fffde2e984 100644 --- a/mod/forum/amd/src/pin_toggle.js +++ b/mod/forum/amd/src/pin_toggle.js @@ -54,18 +54,13 @@ define([ var forumid = toggleElement.data('forumid'); var discussionid = toggleElement.data('discussionid'); var pinstate = toggleElement.data('targetstate'); - var includetext = toggleElement.data('includetext'); - - Repository.togglePin(forumid, discussionid, pinstate, includetext) + Repository.setPinDiscussionState(forumid, discussionid, pinstate) .then(function(context) { return Templates.render('mod_forum/discussion_pin_toggle', context); }) .then(function(html, js) { return Templates.replaceNode(toggleElement, html, js); }) - .then(function() { - return Notification.fetchNotifications(); - }) .fail(Notification.exception); e.preventDefault(); diff --git a/mod/forum/amd/src/repository.js b/mod/forum/amd/src/repository.js index e329ea2baa4..fb5aef13725 100644 --- a/mod/forum/amd/src/repository.js +++ b/mod/forum/amd/src/repository.js @@ -60,14 +60,13 @@ define(['core/ajax'], function(Ajax) { * * @param {number} forumId ID of the forum the discussion belongs to * @param {number} discussionId ID of the discussion with the subscription state - * @param {boolean} targetState Set the favourite state. True == favourited; false == unfavourited. + * @param {null|date} targetState Set the favourite state. True == favourited; false == unfavourited. * @return {object} jQuery promise */ - var toggleFavouriteDiscussionState = function(forumId, discussionId, targetState) { + var setFavouriteDiscussionState = function(forumId, discussionId, targetState) { var request = { methodname: 'mod_forum_toggle_favourite_state', args: { - forumid: forumId, discussionid: discussionId, targetstate: targetState } @@ -86,14 +85,21 @@ define(['core/ajax'], function(Ajax) { return Ajax.call([request])[0]; }; - var togglePin = function(forumid, discussionid, targetstate, includetext) { + /** + * Set the pinned state for the discussion provided. + * + * @param {number} forumid + * @param {number} discussionid + * @param {boolean} targetstate + * @param includetext + * @return {*|Promise} + */ + var setPinDiscussionState = function(forumid, discussionid, targetstate) { var request = { methodname: 'mod_forum_set_pin_state', args: { - forumid: forumid, discussionid: discussionid, - targetstate: targetstate, - includetext: includetext, + targetstate: targetstate } }; return Ajax.call([request])[0]; @@ -103,7 +109,7 @@ define(['core/ajax'], function(Ajax) { setDiscussionSubscriptionState: setDiscussionSubscriptionState, addDiscussionPost: addDiscussionPost, setDiscussionLockState: setDiscussionLockState, - toggleFavouriteDiscussionState: toggleFavouriteDiscussionState, - togglePin: togglePin + setFavouriteDiscussionState: setFavouriteDiscussionState, + setPinDiscussionState: setPinDiscussionState }; }); diff --git a/mod/forum/classes/local/entities/discussion.php b/mod/forum/classes/local/entities/discussion.php index e50bffa975b..affc7c0bbba 100644 --- a/mod/forum/classes/local/entities/discussion.php +++ b/mod/forum/classes/local/entities/discussion.php @@ -306,12 +306,10 @@ class discussion { * @param int $targetstate The state to change the pin to * @return bool */ - public function set_pinned(int $targetstate): bool { + public function set_pinned(int $targetstate): void { if ($targetstate != $this->pinned) { $this->pinned = $targetstate; } - - return true; } /** diff --git a/mod/forum/classes/local/exporters/discussion.php b/mod/forum/classes/local/exporters/discussion.php index 5602361b09c..de0ed1ab2c7 100644 --- a/mod/forum/classes/local/exporters/discussion.php +++ b/mod/forum/classes/local/exporters/discussion.php @@ -202,7 +202,7 @@ class discussion extends exporter { ], 'userstate' => [ 'subscribed' => \mod_forum\subscriptions::is_subscribed($user->id, $forumrecord, $discussion->get_id()), - 'favourited' => discussion_entity::is_favourited($discussion, $forum->get_context(), $user), + 'favourited' => $this->is_favourited($discussion, $forum->get_context(), $user), ], 'capabilities' => [ 'subscribe' => $capabilitymanager->can_subscribe_to_discussion($user, $discussion), @@ -243,6 +243,21 @@ 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. * diff --git a/mod/forum/classes/local/renderers/discussion.php b/mod/forum/classes/local/renderers/discussion.php index 3eb42e5f76b..81d6bd80e2b 100644 --- a/mod/forum/classes/local/renderers/discussion.php +++ b/mod/forum/classes/local/renderers/discussion.php @@ -197,10 +197,6 @@ class discussion { $exporteddiscussion['html']['movediscussion'] = $this->get_move_discussion_html(); } - if ($capabilities['pin']) { - $exporteddiscussion['html']['pindiscussion'] = $this->get_pin_discussion_html(); - } - return $this->renderer->render_from_template('mod_forum/forum_discussion', $exporteddiscussion); } @@ -325,32 +321,6 @@ class discussion { return null; } - /** - * Get the HTML to render the pin discussion button. - * - * @return string - */ - private function get_pin_discussion_html() : string { - $discussion = $this->discussion; - - if ($discussion->is_pinned()) { - $pinlink = FORUM_DISCUSSION_UNPINNED; - $pintext = get_string('unpindiscussion', 'forum'); - } else { - $pinlink = FORUM_DISCUSSION_PINNED; - $pintext = get_string('pindiscussion', 'forum'); - } - - $params = [ - 'pin' => $pinlink, - 'd' => $discussion->get_id(), - 'sesskey' => sesskey(), - ]; - $url = new moodle_url('discuss.php', $params); - $link = new \action_link($url, $pintext, null, ['class' => 'btn btn-link']); - return $this->renderer->render($link); - } - /** * Get the HTML to render the export discussion button. * diff --git a/mod/forum/classes/local/renderers/discussion_list.php b/mod/forum/classes/local/renderers/discussion_list.php index c4db64d7469..8c39aa7ccfe 100644 --- a/mod/forum/classes/local/renderers/discussion_list.php +++ b/mod/forum/classes/local/renderers/discussion_list.php @@ -314,7 +314,8 @@ class discussion_list { $user->id, $sortorder, $this->get_page_size($pagesize), - $this->get_page_number($pageno) * $this->get_page_size($pagesize)); + $this->get_page_number($pageno) * $this->get_page_size($pagesize), + $user); } else { return $discussions = $discussionvault->get_from_forum_id_and_group_id( $forum->get_id(), @@ -323,7 +324,8 @@ class discussion_list { $user->id, $sortorder, $this->get_page_size($pagesize), - $this->get_page_number($pageno) * $this->get_page_size($pagesize)); + $this->get_page_number($pageno) * $this->get_page_size($pagesize), + $user); } } diff --git a/mod/forum/classes/local/vaults/author.php b/mod/forum/classes/local/vaults/author.php index 148bbacdf0b..bd0f78b90a0 100644 --- a/mod/forum/classes/local/vaults/author.php +++ b/mod/forum/classes/local/vaults/author.php @@ -55,9 +55,10 @@ class author extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param \stdClass|null $user The user object * @return string */ - protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = null) : string { $selectsql = 'SELECT * FROM {' . self::TABLE . '} ' . $this->get_table_alias(); $selectsql .= $wheresql ? ' WHERE ' . $wheresql : ''; $selectsql .= $sortsql ? ' ORDER BY ' . $sortsql : ''; diff --git a/mod/forum/classes/local/vaults/db_table_vault.php b/mod/forum/classes/local/vaults/db_table_vault.php index b78daf8cb66..e86fcff9a03 100644 --- a/mod/forum/classes/local/vaults/db_table_vault.php +++ b/mod/forum/classes/local/vaults/db_table_vault.php @@ -72,9 +72,10 @@ abstract class db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param object|null $user The user object * @return string */ - abstract protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string; + abstract protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = null) : string; /** * Convert the DB records into entities. The list of records will have been diff --git a/mod/forum/classes/local/vaults/discussion.php b/mod/forum/classes/local/vaults/discussion.php index 00d4e97fd7d..d6aa04945ab 100644 --- a/mod/forum/classes/local/vaults/discussion.php +++ b/mod/forum/classes/local/vaults/discussion.php @@ -58,9 +58,10 @@ class discussion extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param \stdClass|null $user The user object * @return string */ - protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = null) : string { $selectsql = 'SELECT * FROM {' . self::TABLE . '} ' . $this->get_table_alias(); $selectsql .= $wheresql ? ' WHERE ' . $wheresql : ''; $selectsql .= $sortsql ? ' ORDER BY ' . $sortsql : ''; diff --git a/mod/forum/classes/local/vaults/discussion_list.php b/mod/forum/classes/local/vaults/discussion_list.php index e1b059c138d..9e2919e5309 100644 --- a/mod/forum/classes/local/vaults/discussion_list.php +++ b/mod/forum/classes/local/vaults/discussion_list.php @@ -88,14 +88,20 @@ class discussion_list extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL - * @param string|null $joinsql Additional join conditions for the sql + * @param string|null $joinsql Additional join conditions for the sql + * @param stdClass|null $user User we are performing this query for * * @return string */ - protected function generate_get_records_sql(string $wheresql = null, ?string $sortsql = null, ?string $joinsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, ?string $sortsql = null, stdClass $user = null) : string { $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); + } + // Fetch: // - Discussion // - First post @@ -117,7 +123,7 @@ class discussion_list extends db_table_vault { $tables .= ' JOIN {user} fa ON fa.id = ' . $alias . '.userid'; $tables .= ' JOIN {user} la ON la.id = ' . $alias . '.usermodified'; $tables .= ' JOIN ' . $posttable->get_from_sql() . ' ON fp.id = ' . $alias . '.firstpost'; - $tables .= $joinsql ? $joinsql : ''; + $tables .= $favsql ? $favsql : ''; $selectsql = 'SELECT ' . $fields . ' FROM ' . $tables; $selectsql .= $wheresql ? ' WHERE ' . $wheresql : ''; @@ -260,7 +266,8 @@ class discussion_list extends db_table_vault { ?int $includepostsforuser, ?int $sortorder, int $limit, - int $offset + int $offset, + stdClass $user ) { $alias = $this->get_table_alias(); $wheresql = "{$alias}.forum = :forumid"; @@ -274,10 +281,7 @@ class discussion_list extends db_table_vault { 'forumid' => $forumid, ]); - list($favsql, $favparams) = $this->get_favourite_sql(); - $params += $favparams; - - $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $favsql); + $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $user); $records = $this->get_db()->get_records_sql($sql, $params, $offset, $limit); return $this->transform_db_records_to_entities($records); @@ -303,7 +307,8 @@ class discussion_list extends db_table_vault { ?int $includepostsforuser, ?int $sortorder, int $limit, - int $offset + int $offset, + stdClass $user ) { $alias = $this->get_table_alias(); @@ -327,10 +332,7 @@ class discussion_list extends db_table_vault { 'allgroupsid' => -1, ]); - list($favsql, $favparams) = $this->get_favourite_sql(); - $params += $favparams; - - $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $favsql); + $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $user); $records = $this->get_db()->get_records_sql($sql, $params, $offset, $limit); return $this->transform_db_records_to_entities($records); @@ -411,9 +413,8 @@ class discussion_list extends db_table_vault { /** * Get the standard favouriting sql. */ - private function get_favourite_sql(): array { - global $USER; - $usercontext = \context_user::instance($USER->id); + private function get_favourite_sql($user): array { + $usercontext = \context_user::instance($user->id); $alias = $this->get_table_alias(); $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext); list($favsql, $favparams) = $ufservice->get_join_sql_by_type('mod_forum', 'discussions', diff --git a/mod/forum/classes/local/vaults/forum.php b/mod/forum/classes/local/vaults/forum.php index 853e87dfcc6..1fb21f715b0 100644 --- a/mod/forum/classes/local/vaults/forum.php +++ b/mod/forum/classes/local/vaults/forum.php @@ -61,9 +61,10 @@ class forum extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param \stdClass|null $user The user object * @return string */ - protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = null) : string { $db = $this->get_db(); $alias = $this->get_table_alias(); diff --git a/mod/forum/classes/local/vaults/post.php b/mod/forum/classes/local/vaults/post.php index d92544f40ee..a278001f696 100644 --- a/mod/forum/classes/local/vaults/post.php +++ b/mod/forum/classes/local/vaults/post.php @@ -63,9 +63,10 @@ class post extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param \stdClass|null $user The user object * @return string */ - protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, stdClass $user = null) : string { $table = self::TABLE; $alias = $this->get_table_alias(); $fields = $alias . '.*'; diff --git a/mod/forum/classes/local/vaults/post_read_receipt_collection.php b/mod/forum/classes/local/vaults/post_read_receipt_collection.php index f9e550bdda2..28b1db82151 100644 --- a/mod/forum/classes/local/vaults/post_read_receipt_collection.php +++ b/mod/forum/classes/local/vaults/post_read_receipt_collection.php @@ -57,9 +57,10 @@ class post_read_receipt_collection extends db_table_vault { * * @param string|null $wheresql Where conditions for the SQL * @param string|null $sortsql Order by conditions for the SQL + * @param \stdClass|null $user The user object * @return string */ - protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null) : string { + protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = null) : string { $selectsql = 'SELECT * FROM {' . self::TABLE . '} ' . $this->get_table_alias(); $selectsql .= $wheresql ? ' WHERE ' . $wheresql : ''; $selectsql .= $sortsql ? ' ORDER BY ' . $sortsql : ''; diff --git a/mod/forum/db/access.php b/mod/forum/db/access.php index 592f67b2412..3a5733c33df 100644 --- a/mod/forum/db/access.php +++ b/mod/forum/db/access.php @@ -407,7 +407,7 @@ $capabilities = array( ), 'mod/forum:cantogglefavourite' => array( 'captype' => 'write', - 'contextlevel' => CONTEXT_SYSTEM, + 'contextlevel' => CONTEXT_MODULE, 'archetypes' => array( 'user' => CAP_ALLOW ) diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php index 76925e2af27..bda06a490c0 100644 --- a/mod/forum/externallib.php +++ b/mod/forum/externallib.php @@ -1100,23 +1100,25 @@ class mod_forum_external extends external_api { /** * Toggle the favouriting value for the discussion provided * - * @param int $forumid The forum id * @param int $discussionid The discussion we need to favourite * @param bool $targetstate The state of the favourite value * @return array The exported discussion */ - public static function toggle_favourite_state($forumid, $discussionid, $targetstate) { + public static function toggle_favourite_state($discussionid, $targetstate) { global $DB, $PAGE, $USER; $params = self::validate_parameters(self::toggle_favourite_state_parameters(), [ - 'forumid' => $forumid, 'discussionid' => $discussionid, 'targetstate' => $targetstate ]); $vaultfactory = mod_forum\local\container::get_vault_factory(); + // Get the discussion vault and the corresponding discussion entity. + $discussionvault = $vaultfactory->get_discussion_vault(); + $discussion = $discussionvault->get_from_id($params['discussionid']); + $forumvault = $vaultfactory->get_forum_vault(); - $forum = $forumvault->get_from_id($params['forumid']); + $forum = $forumvault->get_from_id($discussion->get_forum_id()); $forumcontext = $forum->get_context(); $usercontext = context_user::instance($USER->id); @@ -1125,9 +1127,7 @@ class mod_forum_external extends external_api { $managerfactory = mod_forum\local\container::get_manager_factory(); $capabilitymanager = $managerfactory->get_capability_manager($forum); - // Get the discussion vault and the corresponding discussion entity. - $discussionvault = $vaultfactory->get_discussion_vault(); - $discussion = $discussionvault->get_from_id($params['discussionid']); + // Does the user have the ability to favourite the discussion? if (!$capabilitymanager->can_favourite_discussion($USER, $discussion)) { @@ -1165,7 +1165,6 @@ class mod_forum_external extends external_api { public static function toggle_favourite_state_parameters() { return new external_function_parameters( [ - 'forumid' => new external_value(PARAM_INT, 'Forum that the discussion is in'), 'discussionid' => new external_value(PARAM_INT, 'The discussion to subscribe or unsubscribe'), 'targetstate' => new external_value(PARAM_BOOL, 'The target state') ] @@ -1588,7 +1587,7 @@ class mod_forum_external extends external_api { * @return external_description */ public static function set_subscription_state_returns() { - return \mod_forum\local\exporters\discussion::get_read_structure(); + return discussion_exporter::get_read_structure(); } /** @@ -1638,15 +1637,13 @@ class mod_forum_external extends external_api { /** * Set the pin state. * - * @param int $forumid * @param int $discussionid * @param bool $targetstate * @return \stdClass */ - public static function set_pin_state($forumid, $discussionid, $targetstate) { + public static function set_pin_state($discussionid, $targetstate) { global $PAGE, $USER; $params = self::validate_parameters(self::set_pin_state_parameters(), [ - 'forumid' => $forumid, 'discussionid' => $discussionid, 'targetstate' => $targetstate, ]); @@ -1654,19 +1651,18 @@ class mod_forum_external extends external_api { $managerfactory = mod_forum\local\container::get_manager_factory(); $forumvault = $vaultfactory->get_forum_vault(); $discussionvault = $vaultfactory->get_discussion_vault(); - $forum = $forumvault->get_from_id($params['forumid']); + $discussion = $discussionvault->get_from_id($params['discussionid']); + $forum = $forumvault->get_from_id($discussion->get_forum_id()); $capabilitymanager = $managerfactory->get_capability_manager($forum); self::validate_context($forum->get_context()); $legacydatamapperfactory = mod_forum\local\container::get_legacy_data_mapper_factory(); - $forumrecord = $legacydatamapperfactory->get_forum_data_mapper()->to_legacy_object($forum); if (!$capabilitymanager->can_pin_discussions($USER)) { // Nothing to do. We won't actually output any content here though. throw new \moodle_exception('cannotpindiscussions', 'mod_forum'); } - $discussion = $discussionvault->get_from_id($params['discussionid']); $discussion->set_pinned($targetstate); $discussionrecord = $legacydatamapperfactory->get_discussion_data_mapper()->to_legacy_object($discussion); $discussionvault->update_discussion($discussionrecord); @@ -1699,8 +1695,6 @@ class mod_forum_external extends external_api { public static function set_pin_state_parameters() { return new external_function_parameters( [ - 'forumid' => new external_value(PARAM_INT, 'Forum that the discussion is in', VALUE_REQUIRED, - null, NULL_NOT_ALLOWED), 'discussionid' => new external_value(PARAM_INT, 'The discussion to pin or unpin', VALUE_REQUIRED, null, NULL_NOT_ALLOWED), 'targetstate' => new external_value(PARAM_INT, 'The target state', VALUE_REQUIRED, @@ -1730,6 +1724,6 @@ class mod_forum_external extends external_api { * @return external_single_structure */ public static function set_pin_state_returns() { - return \mod_forum\local\exporters\discussion::get_read_structure(); + return discussion_exporter::get_read_structure(); } } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 869529f2864..b726fae5df3 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -6437,7 +6437,8 @@ 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/unstar' => 'fa-star-o', + 'mod_forum:t/emptystar' => 'fa-star-o', + 'mod_forum:t/star' => 'fa-star', ]; } diff --git a/mod/forum/templates/discussion_favourite_toggle.mustache b/mod/forum/templates/discussion_favourite_toggle.mustache index 74836616ae4..43d21dd8f82 100644 --- a/mod/forum/templates/discussion_favourite_toggle.mustache +++ b/mod/forum/templates/discussion_favourite_toggle.mustache @@ -39,12 +39,14 @@ data-action="toggle" data-discussionid="{{id}}" data-forumid="{{forumid}}" - href="#" + tabindex="-1" {{#userstate.favourited}} data-targetstate="0" + title="{{#str}}removefromfavourites, mod_forum{{/str}}" {{/userstate.favourited}} {{^userstate.favourited}} data-targetstate="1" + title="{{#str}}addtofavourites, mod_forum{{/str}}" {{/userstate.favourited}} > {{$favouritecontent}} diff --git a/mod/forum/templates/discussion_pin_toggle.mustache b/mod/forum/templates/discussion_pin_toggle.mustache index fc6e44af1ae..f4e9182b3c8 100644 --- a/mod/forum/templates/discussion_pin_toggle.mustache +++ b/mod/forum/templates/discussion_pin_toggle.mustache @@ -42,9 +42,11 @@ href="{{urls.pin}}" {{#pinned}} data-targetstate="0" + title="{{#str}}unpindiscussion, mod_forum{{/str}}" {{/pinned}} {{^pinned}} data-targetstate="1" + title="{{#str}}pindiscussion, mod_forum{{/str}}" {{/pinned}} > {{#pinned}} diff --git a/mod/forum/templates/forum_discussion.mustache b/mod/forum/templates/forum_discussion.mustache index 783933f8926..df83616fd77 100644 --- a/mod/forum/templates/forum_discussion.mustache +++ b/mod/forum/templates/forum_discussion.mustache @@ -32,13 +32,13 @@
{{#html}} -
+
{{#capabilities.manage}} - {{^istimelocked}} -
- {{> forum/discussion_lock_toggle }} -
- {{/istimelocked}} + {{^istimelocked}} +
+ {{> forum/discussion_lock_toggle }} +
+ {{/istimelocked}} {{/capabilities.manage}}
diff --git a/mod/forum/templates/forum_discussion_favourite_toggle.mustache b/mod/forum/templates/forum_discussion_favourite_toggle.mustache index a757f7590eb..8b460b2dc1c 100644 --- a/mod/forum/templates/forum_discussion_favourite_toggle.mustache +++ b/mod/forum/templates/forum_discussion_favourite_toggle.mustache @@ -32,10 +32,10 @@ {{< mod_forum/discussion_favourite_toggle }} {{$favouritecontent}} {{#userstate.favourited}} - {{#pix}}t/star, mod_forum{{/pix}} + {{#pix}}t/star, mod_forum, {{#str}}removefromfavourites, mod_forum{{/str}}{{/pix}} {{/userstate.favourited}} {{^userstate.favourited}} - {{#pix}}t/unstar, mod_forum{{/pix}} + {{#pix}}t/emptystar, mod_forum, {{#str}}addtofavourites, mod_forum{{/str}}{{/pix}} {{/userstate.favourited}} {{/favouritecontent}} {{/ mod_forum/discussion_favourite_toggle }} diff --git a/mod/forum/tests/entities_discussion_test.php b/mod/forum/tests/entities_discussion_test.php index 9e6d46b56e8..c3bb6f8f674 100644 --- a/mod/forum/tests/entities_discussion_test.php +++ b/mod/forum/tests/entities_discussion_test.php @@ -158,54 +158,6 @@ class mod_forum_entities_discussion_testcase extends advanced_testcase { $this->assertEquals($isvisible, $discussion->is_timed_discussion_visible(), $testdescription); } - 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)); - } - /** * Data provider for test_display_period_settings(). * diff --git a/mod/forum/tests/exporters_discussion_test.php b/mod/forum/tests/exporters_discussion_test.php index 7290395fc85..c6b9b0ca38d 100644 --- a/mod/forum/tests/exporters_discussion_test.php +++ b/mod/forum/tests/exporters_discussion_test.php @@ -113,4 +113,52 @@ 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)); + } }