Skip to content

Conversation

@dougfabris
Copy link
Member

@dougfabris dougfabris commented Oct 10, 2025

Proposed changes (including videos or screenshots)

Due to a wrong usage of the size of the results instead of the total from the API response, the pagination isn't working properly in this table. In order to be able to test this table, we'll need to make a refactor in and isolate the data from the UI. It will be covered here

before

image

after

image

Issue(s)

Steps to test or reproduce

  • Go to admin and then permissions
  • Open a role with more than 25 users
  • Click in users in role inside the context bar
  • The table and pagination should working fine

Further comments

SUP-856

Summary by CodeRabbit

  • Bug Fixes

    • Fixed pagination in the “Users in Role” table to correctly reflect the total number of users from the server, ensuring accurate page counts and navigation across multiple pages and filters.
  • Chores

    • Bumped patch version and updated the changelog to document the fix.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 10, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.12.0, but it targets 7.11.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 10, 2025

🦋 Changeset detected

Latest commit: 8a82b5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 42 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/federation-service Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dougfabris dougfabris added this to the 7.12.0 milestone Oct 10, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

Pagination in the Users in Role table now uses the backend-provided total count (data.total) instead of the local users.length. A changeset bumps the @rocket.chat/meteor package patch version and documents the bug fix.

Changes

Cohort / File(s) Summary
Changeset update
\.changeset/slow-tomatoes-try.md
Adds a patch release entry for @rocket.chat/meteor noting the pagination bug fix.
Admin UI: Users in Role pagination
apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx
Pagination count now sourced from API response data.total with fallback to 0, replacing reliance on local users.length.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Admin as Admin UI (UsersInRoleTable)
  participant API as Backend API

  Admin->>API: fetch users in role (page, pageSize)
  API-->>Admin: { users: [...], total: N }

  rect rgba(200, 240, 255, 0.3)
    note right of Admin: Updated behavior
    Admin->>Admin: Set pagination count = data.total (fallback 0)
  end

  Admin->>Admin: Render table rows and pagination
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • MartinSchoeler

Poem

I thump my paws—pages hop in line,
From backend burrow, totals now align.
No more guessing in the meadow’s scroll,
Each carrot counted, every role.
Click, hop, next—so smooth and dandy,
Patch packed neat, like treats in candy. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the core fix by identifying the pagination issue and specifying the affected component, making it clear and directly tied to the changes in the UsersInRoleTable.
Linked Issues Check ✅ Passed The change replaces the local user count with the API-provided total for pagination, directly addressing SUP-856’s requirement to fix pagination in the UsersInRoleTable.
Out of Scope Changes Check ✅ Passed All modifications—updating the changeset entry and adjusting the pagination source in UsersInRoleTable—are directly related to fixing the pagination bug and do not introduce unrelated functionality.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/usersInRolePagination

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6b9b1d1 and a59805c.

📒 Files selected for processing (2)
  • .changeset/slow-tomatoes-try.md (1 hunks)
  • apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: 🔨 Test Unit / Unit Tests
  • GitHub Check: 🔎 Code Check / Code Lint
  • GitHub Check: 🔎 Code Check / TypeScript
  • GitHub Check: 📦 Meteor Build - coverage
🔇 Additional comments (2)
apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx (1)

118-118: LGTM! Correct fix for pagination.

Using data.total from the API response is the correct approach for pagination, as it represents the actual total count of records rather than just the current page size. The fallback to 0 appropriately handles cases where total might be undefined.

.changeset/slow-tomatoes-try.md (1)

1-5: LGTM! Changeset properly documents the fix.

The changeset follows the standard format and correctly documents the pagination bug fix with an appropriate patch version bump for the @rocket.chat/meteor package.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.38%. Comparing base (7ac146b) to head (8a82b5a).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #37203   +/-   ##
========================================
  Coverage    67.38%   67.38%           
========================================
  Files         3288     3288           
  Lines       111815   111815           
  Branches     20422    20432   +10     
========================================
+ Hits         75341    75343    +2     
+ Misses       33787    33786    -1     
+ Partials      2687     2686    -1     
Flag Coverage Δ
e2e 57.32% <0.00%> (+<0.01%) ⬆️
unit 71.38% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dougfabris dougfabris marked this pull request as ready for review October 13, 2025 23:21
@dougfabris dougfabris requested a review from a team as a code owner October 13, 2025 23:21
@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Oct 14, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Oct 14, 2025
@kodiakhq kodiakhq bot merged commit ebbf98d into develop Oct 14, 2025
86 of 88 checks passed
@kodiakhq kodiakhq bot deleted the fix/usersInRolePagination branch October 14, 2025 14:46
antm-rp pushed a commit to antm-rp/Rocket.Chat that referenced this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants