MDL-65033 mod_forum: Testing updates

Testing and code updates after rebase

diff --git a/mod/forum/amd/build/discussion_list.min.js b/mod/forum/amd/build/discussion_list.min.js
index c2084fc..ebee587 100644 (file)
Binary files a/mod/forum/amd/build/discussion_list.min.js and b/mod/forum/amd/build/discussion_list.min.js differ

diff --git a/mod/forum/amd/src/discussion_list.js b/mod/forum/amd/src/discussion_list.js
index 35007ad..02f1473 100644 (file)
--- a/mod/forum/amd/src/discussion_list.js
+++ b/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);
         });

diff --git a/mod/forum/classes/local/builders/exported_discussion.php b/mod/forum/classes/local/builders/exported_discussion.php
index 0ebdc09..d0e98f5 100644 (file)
--- a/mod/forum/classes/local/builders/exported_discussion.php
+++ b/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);

diff --git a/mod/forum/classes/local/builders/exported_discussion_summaries.php b/mod/forum/classes/local/builders/exported_discussion_summaries.php
index 5df95be..7859c47 100644 (file)
--- a/mod/forum/classes/local/builders/exported_discussion_summaries.php
+++ b/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;

diff --git a/mod/forum/classes/local/vaults/db_table_vault.php b/mod/forum/classes/local/vaults/db_table_vault.php
index e86fcff..862ad36 100644 (file)
--- a/mod/forum/classes/local/vaults/db_table_vault.php
+++ b/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

diff --git a/mod/forum/classes/local/vaults/discussion_list.php b/mod/forum/classes/local/vaults/discussion_list.php
index 528d34b..49e9c94 100644 (file)
--- a/mod/forum/classes/local/vaults/discussion_list.php
+++ b/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];
     }

diff --git a/mod/forum/externallib.php b/mod/forum/externallib.php
index 4c1303c..8a20dc3 100644 (file)
--- a/mod/forum/externallib.php
+++ b/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);
 

diff --git a/mod/forum/lang/en/forum.php b/mod/forum/lang/en/forum.php
index 8c73bfe..f3bdd8d 100644 (file)
--- a/mod/forum/lang/en/forum.php
+++ b/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.';

diff --git a/mod/forum/templates/discussion_favourite_toggle.mustache b/mod/forum/templates/discussion_favourite_toggle.mustache
index 675d4c0..b99270e 100644 (file)
--- a/mod/forum/templates/discussion_favourite_toggle.mustache
+++ b/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"

diff --git a/mod/forum/templates/discussion_pin_toggle.mustache b/mod/forum/templates/discussion_pin_toggle.mustache
index deacd1e..587fd1d 100644 (file)
--- a/mod/forum/templates/discussion_pin_toggle.mustache
+++ b/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}}"

diff --git a/mod/forum/templates/forum_action_menu.mustache b/mod/forum/templates/forum_action_menu.mustache
index ae0fee3..dfa3654 100644 (file)
--- a/mod/forum/templates/forum_action_menu.mustache
+++ b/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>
\ No newline at end of file

diff --git a/mod/forum/templates/forum_discussion.mustache b/mod/forum/templates/forum_discussion.mustache
index df83616..3799616 100644 (file)
--- a/mod/forum/templates/forum_discussion.mustache
+++ b/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);

diff --git a/mod/forum/tests/behat/behat_mod_forum.php b/mod/forum/tests/behat/behat_mod_forum.php
index b5cb276..b721ec4 100644 (file)
--- a/mod/forum/tests/behat/behat_mod_forum.php
+++ b/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"
         ]);
     }
 

diff --git a/mod/forum/tests/behat/favourite_discussion.feature b/mod/forum/tests/behat/favourite_discussion.feature
index 58a2d82..4b85a74 100644 (file)
--- a/mod/forum/tests/behat/favourite_discussion.feature
+++ b/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"
\ No newline at end of file
+    And I click on "[title='Unstar this discussion']" "css_element"
\ No newline at end of file

diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php
index 3982cd6..5277bb0 100644 (file)
--- a/mod/forum/tests/externallib_test.php
+++ b/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);