MDL-65033 mod_forum: Testing updates

Testing and code updates after rebase

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.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)}}});
+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(){return 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(){return location.reload()})["catch"](Notification.exception)})};return{init:function(a){b.init(a),e(a)}}});

mod/forum/amd/src/discussion_list.js

@@ -40,7 +40,7 @@ define([
             var subscriptionState = toggleElement.data('targetstate');
             Repository.setFavouriteDiscussionState(forumId, discussionId, subscriptionState)
                 .then(function() {
-                    location.reload();
+                    return location.reload();
                 })
                 .catch(Notification.exception);
         });
@@ -53,7 +53,7 @@ define([
             var state = toggleElement.data('targetstate');
             Repository.setPinDiscussionState(forumId, discussionId, state)
                 .then(function() {
-                    location.reload();
+                    return location.reload();
                 })
                 .catch(Notification.exception);
         });

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

@@ -142,6 +142,10 @@ class exported_discussion {
      * @return bool Whether or not the user has favourited the discussion
      */
     public function is_favourited(discussion_entity $discussion, \context_module $forumcontext, \stdClass $user) {
+        if (!isloggedin()) {
+            return false;
+        }
+
         $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

@@ -157,12 +157,15 @@ class exported_discussion_summaries {
      * @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;
+
+        if (isloggedin()) {
+            $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');
+            foreach ($favourites as $favourite) {
+                $ids[] = $favourite->itemid;
+            }
         }
 
         return $ids;

mod/forum/classes/local/vaults/db_table_vault.php

@@ -75,7 +75,8 @@ abstract class db_table_vault {
      * @param object|null $user The user object
      * @return string
      */
-    abstract protected function generate_get_records_sql(string $wheresql = null, string $sortsql = null, \stdClass $user = 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

mod/forum/classes/local/vaults/discussion_list.php

@@ -97,11 +97,9 @@ class discussion_list extends db_table_vault {
         $alias = $this->get_table_alias();
         $db = $this->get_db();
 
-        if ($user) {
-            list($favsql, $favparams) = $this->get_favourite_sql($user);
-            foreach ($favparams as $key => $param) {
-                $favsql = str_replace(":$key", "'$param'", $favsql);
-            }
+        list($favsql, $favparams) = $this->get_favourite_sql($user);
+        foreach ($favparams as $key => $param) {
+            $favsql = str_replace(":$key", "'$param'", $favsql);
         }
 
         // Fetch:
@@ -198,7 +196,7 @@ class discussion_list extends db_table_vault {
      *
      * @param int|null $sortmethod
      */
-    public function get_sort_order(?int $sortmethod) : string {
+    public function get_sort_order(?int $sortmethod, $includefavourites = true) : string {
         global $CFG;
 
         $alias = $this->get_table_alias();
@@ -219,7 +217,9 @@ class discussion_list extends db_table_vault {
             }
         }
 
-        return "{$alias}.pinned DESC, {$this->get_favourite_alias()}.id DESC, {$keyfield} {$direction}";
+        $favouritesort = ($includefavourites ? ", {$this->get_favourite_alias()}.id DESC" : "");
+
+        return "{$alias}.pinned DESC $favouritesort , {$keyfield} {$direction}";
     }
 
     /**
@@ -283,7 +283,7 @@ class discussion_list extends db_table_vault {
             'forumid' => $forumid,
         ]);
 
-        $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $user);
+        $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder, isloggedin()), $user);
         $records = $this->get_db()->get_records_sql($sql, $params, $offset, $limit);
 
         return $this->transform_db_records_to_entities($records);
@@ -334,7 +334,7 @@ class discussion_list extends db_table_vault {
             'allgroupsid' => -1,
         ]);
 
-        $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder), $user);
+        $sql = $this->generate_get_records_sql($wheresql, $this->get_sort_order($sortorder, isloggedin()), $user);
         $records = $this->get_db()->get_records_sql($sql, $params, $offset, $limit);
 
         return $this->transform_db_records_to_entities($records);
@@ -418,12 +418,17 @@ class discussion_list extends db_table_vault {
      * @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(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);
-        list($favsql, $favparams) = $ufservice->get_join_sql_by_type('mod_forum', 'discussions',
-            $this->get_favourite_alias(), "$alias.id");
+    private function get_favourite_sql(?stdClass $user): array {
+        $favsql = "";
+        $favparams = [];
+
+        if ($user && isloggedin()) {
+            $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',
+                $this->get_favourite_alias(), "$alias.id");
+        }
 
         return [$favsql, $favparams];
     }

mod/forum/externallib.php

@@ -592,8 +592,10 @@ class mod_forum_external extends external_api {
             $canlock = has_capability('moodle/course:manageactivities', $modcontext, $USER);
             $replies = forum_count_discussion_replies($forumid, $sort, -1, $page, $perpage, $canseeprivatereplies);
 
-            $usercontext = context_user::instance($USER->id);
-            $ufservice = core_favourites\service_factory::get_service_for_user_context($usercontext);
+            if (isloggedin()) {
+                $usercontext = context_user::instance($USER->id);
+                $ufservice = core_favourites\service_factory::get_service_for_user_context($usercontext);
+            }
             $canfavourite = has_capability('mod/forum:cantogglefavourite', $modcontext, $USER);
             foreach ($alldiscussions as $discussion) {
 
@@ -643,8 +645,8 @@ class mod_forum_external extends external_api {
 
                 $discussion->locked = forum_discussion_is_locked($forum, $discussion);
                 $discussion->canlock = $canlock;
-                $discussion->starred = $ufservice->favourite_exists('mod_forum', 'discussions',
-                    $discussionrec->id, $modcontext);
+                $discussion->starred = !empty($ufservice) ? $ufservice->favourite_exists('mod_forum', 'discussions',
+                    $discussionrec->id, $modcontext) : false;
                 $discussion->canreply = forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext);
                 $discussion->canfavourite = $canfavourite;
 
@@ -1128,8 +1130,6 @@ class mod_forum_external extends external_api {
         $forumvault = $vaultfactory->get_forum_vault();
         $forum = $forumvault->get_from_id($discussion->get_forum_id());
         $forumcontext = $forum->get_context();
-        $usercontext = context_user::instance($USER->id);
-
         self::validate_context($forumcontext);
 
         $managerfactory = mod_forum\local\container::get_manager_factory();
@@ -1139,7 +1139,7 @@ class mod_forum_external extends external_api {
         if (!$capabilitymanager->can_favourite_discussion($USER, $discussion)) {
             throw new moodle_exception('cannotfavourite', 'forum');
         }
-
+        $usercontext = context_user::instance($USER->id);
         $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
         $isfavourited = $ufservice->favourite_exists('mod_forum', 'discussions', $discussion->get_id(), $forumcontext);
 

mod/forum/lang/en/forum.php

@@ -251,6 +251,7 @@ $string['forum:addquestion'] = 'Add question';
 $string['forum:allowforcesubscribe'] = 'Allow force subscribe';
 $string['forum:canoverridecutoff'] = 'Post to forums after their cut-off date';
 $string['forum:canoverridediscussionlock'] = 'Reply to locked discussions';
+$string['forum:cantogglefavourite'] = 'Can toggle favourites.';
 $string['forumauthorhidden'] = 'Author (hidden)';
 $string['forumblockingalmosttoomanyposts'] = 'You are approaching the posting threshold. You have posted {$a->numposts} times in the last {$a->blockperiod} and the limit is {$a->blockafter} posts.';
 $string['forumbodyhidden'] = 'This post cannot be viewed by you, probably because you have not posted in the discussion, the maximum editing time hasn\'t passed yet, the discussion has not started or the discussion has expired.';

mod/forum/templates/discussion_favourite_toggle.mustache

@@ -39,7 +39,6 @@
         data-action="toggle"
         data-discussionid="{{id}}"
         data-forumid="{{forumid}}"
-        role="menuitem",
         tabindex="-1"
         {{#userstate.favourited}}
             data-targetstate="0"

mod/forum/templates/discussion_pin_toggle.mustache

@@ -41,7 +41,6 @@
         data-forumid="{{forumid}}"
         href="{{urls.pin}}"
         tabindex="-1"
-        role="menuitem"
         {{#pinned}}
             data-targetstate="0"
             title="{{#str}}unpindiscussion, mod_forum{{/str}}"

mod/forum/templates/forum_action_menu.mustache

@@ -49,11 +49,15 @@
          data-rel="menu-content"
          role="menu"
          id="forum-action-menu-{{id}}-menu">
-        <div class="dropdown-item">
+        {{#capabilities.favourite}}
+        <div class="dropdown-item" role="menuitem">
             {{> mod_forum/discussion_favourite_toggle}}
         </div>
-        <div class="dropdown-item">
+        {{/capabilities.favourite}}
+        {{#capabilities.pin}}
+        <div class="dropdown-item" role="menuitem">
             {{> mod_forum/discussion_pin_toggle}}
         </div>
+        {{/capabilities.pin}}
     </div>
 </div>

mod/forum/templates/forum_discussion.mustache

@@ -70,7 +70,7 @@ require(['jquery', 'mod_forum/discussion', 'mod_forum/posts_list', 'mod_forum/lo
         var root = $("[data-content='forum-discussion']");
         Discussion.init(root);
         PostsList.init(root);
-        var root = $('[data-container="discussion-tools"]');
+        root = $('[data-container="discussion-tools"]');
         LockToggle.init(root);
         FavouriteToggle.init(root);
         Pin.init(root);

mod/forum/tests/behat/behat_mod_forum.php

@@ -136,7 +136,8 @@ class behat_mod_forum extends behat_base {
     public function i_click_on_action_menu($discussion) {
         $this->execute('behat_general::i_click_on_in_the', [
             "[data-container='discussion-tools'] [data-toggle='dropdown']", "css_element",
-            "//tr[@class='discussion' and contains(.,'$discussion')]", "xpath_element"
+            "//tr[contains(concat(' ', normalize-space(@class), ' '), ' discussion ') and contains(.,'$discussion')]",
+            "xpath_element"
         ]);
     }
 

mod/forum/tests/behat/favourite_discussion.feature

@@ -32,10 +32,10 @@ Feature: A student can favourite a discussion via the forum settings menu
       | Message | Discussion contents 1, third message |
     And I wait until the page is ready
     And I open the action menu in "[data-container='discussion-tools']" "css_element"
-    And I click on "Star this discussion" "link"
+    And I click on "[title='Star this discussion']" "css_element"
     And I wait "3" seconds
     And I open the action menu in "[data-container='discussion-tools']" "css_element"
-    Then I should see "Unstar this discussion"
+    And I click on "[title='Unstar this discussion']" "css_element"
 
   Scenario: Student can favourite a discussion from the discussion list
     Given I reply "Discussion 1" post from "Test forum name" forum with:
@@ -43,6 +43,6 @@ Feature: A student can favourite a discussion via the forum settings menu
       | Message | Discussion contents 1, third message |
     And I follow "Test forum name"
     And I click on "Discussion 1" action menu
-    And I click on "Star this discussion" "link"
+    And I click on "[title='Star this discussion']" "css_element"
     And I click on "Discussion 1" action menu
-    Then I should see "Unstar this discussion"
+    And I click on "[title='Unstar this discussion']" "css_element"

mod/forum/tests/externallib_test.php

@@ -219,6 +219,13 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
         $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']);
+
+        $this->setUser(0);
+        try {
+            $response = mod_forum_external::toggle_favourite_state($discussion1->id, 0);
+        } catch (moodle_exception $e) {
+            $this->assertEquals('requireloginerror', $e->errorcode);
+        }
     }
 
     /**
@@ -1096,7 +1103,7 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
 
         $this->assertEquals($expectedreturn, $discussions);
 
-        // Test the starring functionality return
+        // Test the starring functionality return.
         $t = mod_forum_external::toggle_favourite_state($discussion1->id, 1);
         $expectedreturn['discussions'][0]['starred'] = true;
         $discussions = mod_forum_external::get_forum_discussions_paginated($forum1->id);