Skip to content

fix: trim directory user search term before building regex#41546

Open
Pr241singh wants to merge 2 commits into
RocketChat:developfrom
Pr241singh:fix/41545-directory-users-search-trim-whitespace
Open

fix: trim directory user search term before building regex#41546
Pr241singh wants to merge 2 commits into
RocketChat:developfrom
Pr241singh:fix/41545-directory-users-search-trim-whitespace

Conversation

@Pr241singh

@Pr241singh Pr241singh commented Jul 24, 2026

Copy link
Copy Markdown

Proposed changes (including videos or screenshots)

This PR fixes an issue where Directory → Users search did not ignore leading or trailing whitespace in the search term.

Previously, leading or trailing whitespace became part of the generated regular expression, causing valid users not to appear in the search results. This change normalizes the search term before building the regex and before determining whether a search filter should be applied, ensuring consistent search behavior.

After fix

Screen.Recording.2026-07-24.145053.mp4

Issue(s)

Closes #41545

Steps to test or reproduce

  1. Open DirectoryUsers.
  2. Search using the following inputs:
    • preeti
    • preeti
    • preeti
    • preeti
    • preeti
    • pre eti
  3. Verify the results:
    • preeti → ✅ Found
    • preeti → ✅ Found
    • preeti → ✅ Found
    • preeti → ✅ Found
    • preeti → ✅ Found
    • pre eti → ❌ Not Found

Further comments

The fix normalizes the search term in the shared user search helper before constructing the regular expression. This prevents leading and trailing whitespace from affecting user search while preserving the existing behavior for searches containing internal whitespace.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved user search by ignoring leading and trailing spaces in search terms.
    • Prevented empty or whitespace-only searches from producing unnecessary search conditions.
    • Applied consistent whitespace handling across user status and active-user searches.

@Pr241singh
Pr241singh requested review from a team as code owners July 24, 2026 09:27
@dionisio-bot

dionisio-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 219ea63

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

User-search methods now trim leading and trailing whitespace before constructing regex queries. Whitespace-only terms no longer add search clauses across the status-filtered API and active-user model queries.

Changes

User search whitespace normalization

Layer / File(s) Summary
Active-user search normalization
packages/models/src/models/Users.ts
Active-user queries use trimmed search terms for regex construction and omit $or clauses for whitespace-only input.
Status-filtered user search
apps/meteor/server/api/lib/users.ts
Paginated status-filtered searches build email, username, and name regex clauses from the trimmed term.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

  • RocketChat/Rocket.Chat#41531 — Updates the same status-filtered user-search function to normalize search terms before building regex clauses.

Suggested labels: type: bug

Suggested reviewers: sampaiodiego, kevl ehman

🚥 Pre-merge checks | ✅ 5
✅ 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 is concise and accurately describes the main change: trimming the directory user search term before regex construction.
Linked Issues check ✅ Passed The change trims leading/trailing whitespace before regex matching, preserving internal spaces, which matches #41545's expected behavior.
Out of Scope Changes check ✅ Passed All changes are confined to search-term trimming in user search code and appear directly tied to the whitespace bug fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/models/src/models/Users.ts`:
- Around line 353-355: Update the termRegex construction in
findByActiveUsersExcept to use normalizedSearchTerm instead of the raw
searchTerm when escaping and building the pattern. Preserve the existing
startsWith and endsWith anchors and case-insensitive matching so trimmed and
untrimmed inputs behave equivalently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4918ddbd-5ca4-49b4-bf55-b984d04684df

📥 Commits

Reviewing files that changed from the base of the PR and between a27dd2f and 219ea63.

📒 Files selected for processing (2)
  • apps/meteor/server/api/lib/users.ts
  • packages/models/src/models/Users.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • packages/models/src/models/Users.ts
  • apps/meteor/server/api/lib/users.ts
🧠 Learnings (4)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • packages/models/src/models/Users.ts
  • apps/meteor/server/api/lib/users.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • packages/models/src/models/Users.ts
  • apps/meteor/server/api/lib/users.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.

Applied to files:

  • packages/models/src/models/Users.ts
  • apps/meteor/server/api/lib/users.ts
📚 Learning: 2026-07-15T01:31:50.632Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 41382
File: packages/model-typings/src/models/IPushTokenModel.ts:10-10
Timestamp: 2026-07-15T01:31:50.632Z
Learning: In Rocket.Chat’s push-token model, APNs/GCM standard tokens and PushKit VoIP tokens use distinct, globally-unique token string values. As a result, when deduplicating/looking up tokens, key identity solely on `{ tokenValue, appName }` (e.g., for `findOneByTokenAndAppName`, `removeDuplicateTokens`) and do **not** include `tokenType` in the uniqueness criteria to avoid collisions between standard and VoIP tokens.

Applied to files:

  • packages/models/src/models/Users.ts
🪛 ast-grep (0.44.1)
packages/models/src/models/Users.ts

[warning] 354-354: Do not use variable for regular expressions
Context: new RegExp((startsWith ? '^' : '') + escapeRegExp(searchTerm) + (endsWith ? '$' : ''), 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.

(regexp-non-literal-typescript)


[warning] 402-402: Do not use variable for regular expressions
Context: new RegExp((startsWith ? '^' : '') + escapeRegExp(normalizedSearchTerm) + (endsWith ? '$' : ''), 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.

(regexp-non-literal-typescript)

🔇 Additional comments (2)
packages/models/src/models/Users.ts (1)

401-403: LGTM!

Also applies to: 421-422

apps/meteor/server/api/lib/users.ts (1)

188-215: LGTM!

Comment thread packages/models/src/models/Users.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/models/src/models/Users.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directory Users search does not ignore leading or trailing whitespace in search term

1 participant