From efacf364230d088b6d086163e28a7f29777566a3 Mon Sep 17 00:00:00 2001 From: meirzamoodle Date: Wed, 12 Mar 2025 13:37:19 +0700 Subject: [PATCH] MDL-83943 core_tag: Fix the broken user tag pagination --- tag/tests/taglib_test.php | 86 +++++++++++++++++++++++++++++++++++++++ user/lib.php | 70 ++++++++++++++++++++++--------- 2 files changed, 137 insertions(+), 19 deletions(-) diff --git a/tag/tests/taglib_test.php b/tag/tests/taglib_test.php index 0e27d721754..08238dc055f 100644 --- a/tag/tests/taglib_test.php +++ b/tag/tests/taglib_test.php @@ -28,6 +28,7 @@ use core_tag; * @category test * @copyright 2014 Mark Nelson * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core_tag_tag */ final class taglib_test extends \advanced_testcase { @@ -1904,6 +1905,91 @@ final class taglib_test extends \advanced_testcase { core_tag_tag::change_instances_context([$fooinstance1->id], $context2); } + /** + * Tests user pagination works correctly for filtered users. + */ + public function test_user_get_tagged_users(): void { + global $DB; + + // Create some users. + $users = []; + for ($i = 0; $i < 11; $i++) { + $users[] = $this->getDataGenerator()->create_user(); + } + + // Create the tag. + $tagcollid = core_tag_collection::get_default(); + $tag = $this->getDataGenerator()->create_tag(['tagcollid' => $tagcollid, 'rawname' => 'bike']); + + // Add the tag to the users. + for ($i = 0; $i < count($users); $i++) { + core_tag_tag::add_item_tag('core', 'user', $users[$i]->id, + \context_user::instance($users[$i]->id), 'bike'); + } + + // The logged-in user. + $this->setUser($users[0]); + + // Get the tagged users. + $tag = core_tag_tag::get($tag->id, '*'); + $taggedusers = user_get_tagged_users($tag); + + // Ensure it has content. + $this->assertEquals(1, $taggedusers->hascontent); + + // Ensure it should have 1 user and the "more" link is hidden (null). + $this->assertEquals(1, $this->count_html_elements($taggedusers->content, 'li')); + $this->assertNull($taggedusers->nextpageurl); + + // Test which users are visible to the logged-in user based on the course. + // Create a course to tag. + $course = $this->getDataGenerator()->create_course(); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + for ($i = 0; $i < count($users); $i++) { + // Enrol only some users (0, 2, 4, 6, 8, 10). + if ($i % 2 === 0) { + $this->getDataGenerator()->enrol_user($users[$i]->id, $course->id, $studentrole->id); + } + } + + // First page should have 5 users and the "more" link is visible (not null). + $taggedusers = user_get_tagged_users( + tag: $tag, + page: 0, + ); + $this->assertNotNull($taggedusers->nextpageurl); + $this->assertEquals(5, $this->count_html_elements($taggedusers->content, 'li')); + + // Second page should have 1 user and the "more" link is hidden (null). + $taggedusers = user_get_tagged_users( + tag: $tag, + page: 1, + ); + $this->assertNull($taggedusers->nextpageurl); + $this->assertEquals(1, $this->count_html_elements($taggedusers->content, 'li')); + } + + /** + * Counts the number of specified HTML elements in a given HTML string. + * + * @param string $html The HTML string to be parsed. + * @param string $tagname The name of the HTML tag to count (e.g., 'li', 'div'). + * @return int The number of elements with the specified tag name found in the HTML. + */ + private function count_html_elements(string $html, string $tagname): int { + // Load the HTML into DOMDocument. + $dom = new \DOMDocument(); + libxml_use_internal_errors(true); // Suppress warnings for invalid HTML. + $dom->loadHTML($html); + libxml_clear_errors(); + + // Find all elements with the specified tag name. + $elements = $dom->getElementsByTagName($tagname); + + // Count the number of elements. + return $elements->length; + } + /** * Help method to return sorted array of names of correlated tags to use for assertions * @param core_tag $tag diff --git a/user/lib.php b/user/lib.php index de8fb8bf4d4..844befe4038 100644 --- a/user/lib.php +++ b/user/lib.php @@ -1291,32 +1291,64 @@ function user_can_view_profile($user, $course = null, $usercontext = null) { function user_get_tagged_users($tag, $exclusivemode = false, $fromctx = 0, $ctx = 0, $rec = 1, $page = 0) { global $PAGE; - if ($ctx && $ctx != context_system::instance()->id) { - $usercount = 0; - } else { - // Users can only be displayed in system context. - $usercount = $tag->count_tagged_items('core', 'user', - 'it.deleted=:notdeleted', array('notdeleted' => 0)); - } $perpage = $exclusivemode ? 24 : 5; - $content = ''; - $excludedusers = 0; + $filteredusers = []; // Initialize an array to hold users that pass filtering. - if ($usercount) { - $userlist = $tag->get_tagged_items('core', 'user', $page * $perpage, $perpage, - 'it.deleted=:notdeleted', array('notdeleted' => 0)); - foreach ($userlist as $user) { - if (!user_can_view_profile($user)) { - unset($userlist[$user->id]); - $excludedusers++; + $totalusers = $tag->count_tagged_items('core', 'user', 'it.deleted=:notdeleted', ['notdeleted' => 0]); + $withinuserlimit = ($page * $perpage < $totalusers); + // Check if the requested page is within the user limit and if the context is valid or matches the system context. + if ($withinuserlimit && (!$ctx || $ctx == context_system::instance()->id)) { + // The output from get_tagged_items() will be filtered to check if users are visible to the current user. + // It’s possible that the count of users meeting the filtering criteria may fall short of the per-page limit, + // necessitating additional data beyond this limit. + // Implementing a batch approach addressed this issue by minimizing database queries. + $batch = 0; + // Increase the per-page limit to create a batch size for chunked querying. + // If the first chunk $perpagebatch doesn't return enough users, fetch the next chunk without re-querying the database. + $perpagebatch = $perpage * 2; + + do { + $userlist = $tag->get_tagged_items( + component: 'core', + itemtype: 'user', + limitfrom: $perpagebatch * $batch, + limitnum: $perpagebatch, + subquery: 'it.deleted=:notdeleted', + params: ['notdeleted' => 0], + ); + + foreach ($userlist as $user) { + // Check if the user profile can be viewed. + if (user_can_view_profile($user)) { + $filteredusers[] = $user; + // If enough users have been collected for the requested page, exit both loops. + if (count($filteredusers) > $perpage * ($page + 1)) { + break 2; + } + } } - } + + $batch++; + + } while (count($userlist) > 0); // If all the data is still insufficient, run another batch. + + } + + $usercount = count($filteredusers); + + // Initialize the content to display tagged users. + $content = ''; + if ($usercount > 0) { + // Prepare the paginated list of users, limiting it to the number of users per page. + $paginatedusers = array_slice($filteredusers, $page * $perpage, $perpage); + + // Get the renderer for the user module to create the user list content. $renderer = $PAGE->get_renderer('core', 'user'); - $content .= $renderer->user_list($userlist, $exclusivemode); + $content = $renderer->user_list($paginatedusers, $exclusivemode); } // Calculate the total number of pages. - $totalpages = ceil(($usercount - $excludedusers) / $perpage); + $totalpages = ceil($usercount / $perpage); return new core_tag\output\tagindex($tag, 'core', 'user', $content, $exclusivemode, $fromctx, $ctx, $rec, $page, $totalpages);