feat(uikit): add room to some other interaction contexts#39858
Open
feat(uikit): add room to some other interaction contexts#39858
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 1e468f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
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 |
Contributor
WalkthroughRestores room ID (rid) propagation for UIKit BlockAction, ViewSubmit, and ViewClose interactions originating from ContextualBar surfaces by updating client hooks/components, server route handling, type definitions, test apps, and E2E test infra. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as ContextualBar UI
participant ActionMgr as actionManager
participant Server as AppUIKitInteractionApi
participant App as App Handler
User->>Client: click button / submit view (ContextualBar)
Client->>ActionMgr: emitInteraction({ type, payload, rid })
ActionMgr->>Server: POST /api/apps/ui.interaction (body includes rid)
Server->>App: trigger handler with { interaction, room } (room derived from rid)
App-->>Server: handler logs/response
Server-->>ActionMgr: acknowledge
ActionMgr-->>Client: update/response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
12 tasks
d-gubert
force-pushed
the
fix/uikit-interaction-room-data
branch
from
4c13525 to
18c29d2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #39858 +/- ##
========================================
Coverage 69.82% 69.82%
========================================
Files 3296 3299 +3
Lines 119173 119271 +98
Branches 21468 21535 +67
========================================
+ Hits 83211 83280 +69
- Misses 32654 32675 +21
- Partials 3308 3316 +8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d-gubert
force-pushed
the
fix/uikit-interaction-room-data
branch
from
18c29d2 to
6da7cc1
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
.changeset/tricky-kiwis-sit.md (1)
6-6: Nit: minor wording."Adds support to room information on..." reads awkwardly. Suggest: "Adds support for room information (
rid) on ViewSubmit and ViewClose events for the ContextualBar surface."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/tricky-kiwis-sit.md at line 6, Update the changelog entry wording in .changeset/tricky-kiwis-sit.md to read "Adds support for room information (`rid`) on ViewSubmit and ViewClose events for the ContextualBar surface." — specifically replace the phrase "Adds support to room information on..." with "Adds support for room information (`rid`) on..." so the sentence reads clearly and includes the `rid` identifier.apps/meteor/tests/e2e/utils/apps.ts (1)
11-21: Consider a hard-coded setting value or constant instead of'test'.
VideoConf_Default_Provideris being set to'test'with no assertion on the response. If the call silently fails (e.g., auth issue flagged above), subsequent tests depending on this setting will pass misleadingly. Addingawait expect(response).toBeOK()here — as you do in the other helpers — would catch regressions.Proposed fix
- await api.post(`${BASE_URL}/api/apps`, { data: { url: APP_URL }, headers }); - await api.post(`${BASE_API_URL}/settings/VideoConf_Default_Provider`, { data: { value: 'test' }, headers }); + const appResponse = await api.post(`${BASE_URL}/api/apps`, { data: { url: APP_URL }, headers }); + await expect(appResponse).toBeOK(); + const settingResponse = await api.post(`${BASE_API_URL}/settings/VideoConf_Default_Provider`, { data: { value: 'test' }, headers }); + await expect(settingResponse).toBeOK();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/utils/apps.ts` around lines 11 - 21, In insertDefaultTestApp, avoid the hard-coded string 'test' and add assertions: replace the literal 'test' with a named constant (e.g., DEFAULT_VIDEO_CONF_PROVIDER or DEFAULT_PROVIDER) and capture the responses from the two api.post calls (the apps POST and the settings POST for VideoConf_Default_Provider) into variables, then add await expect(response).toBeOK() (or equivalent) for each to ensure the requests succeeded; reference function insertDefaultTestApp and the setting key VideoConf_Default_Provider when making these changes.apps/meteor/tests/data/apps/app-packages/README.md (3)
457-477: Consolidate duplicated imports in the UIKit test app snippet.The snippet imports from
@rocket.chat/apps-engine/definition/metadatatwice (Line 468 forIAppInfo, Line 476 forAppMethod) and from@rocket.chat/apps-engine/definition/uikittwice (Lines 469-475 for the context/response types, Line 477 forUIKitSurfaceType). Since this code is pasted as-is from the packaged test app, consolidating reduces confusion for readers who copy it.♻️ Consolidated imports
import { App } from '@rocket.chat/apps-engine/definition/App'; -import { IAppInfo } from '@rocket.chat/apps-engine/definition/metadata'; +import { AppMethod, IAppInfo } from '@rocket.chat/apps-engine/definition/metadata'; import { IUIKitInteractionHandler, IUIKitResponse, UIKitBlockInteractionContext, UIKitViewCloseInteractionContext, UIKitViewSubmitInteractionContext, + UIKitSurfaceType, } from '@rocket.chat/apps-engine/definition/uikit'; -import { AppMethod } from '@rocket.chat/apps-engine/definition/metadata'; -import { UIKitSurfaceType } from '@rocket.chat/apps-engine/definition/uikit';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/data/apps/app-packages/README.md` around lines 457 - 477, The snippet duplicates imports from the same modules; consolidate the duplicated imports from '@rocket.chat/apps-engine/definition/metadata' (IAppInfo, AppMethod) into a single import and similarly merge the two imports from '@rocket.chat/apps-engine/definition/uikit' (UIKitViewSubmitInteractionContext, UIKitViewCloseInteractionContext, UIKitBlockInteractionContext, IUIKitInteractionHandler, IUIKitResponse, UIKitSurfaceType) so each module is imported once; update the import statements that reference symbols like IAppInfo, AppMethod, UIKitSurfaceType, UIKitViewSubmitInteractionContext, UIKitViewCloseInteractionContext, UIKitBlockInteractionContext, IUIKitInteractionHandler, and IUIKitResponse accordingly.
451-451: Minor: capitalize "view id".For consistency with "room ID" elsewhere in the paragraph, write "view ID" (or use the canonical camelCase
viewId, matchingtriggerId/actionIdused on the same line).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/data/apps/app-packages/README.md` at line 451, Update the README sentence to use the canonical casing for the view identifier: change "view id" to "view ID" (or use camelCase "viewId" to match `triggerId`/`actionId`) in the description of the interaction handlers (`executeBlockActionHandler`, `executeViewSubmitHandler`, `executeViewClosedHandler`) so the paragraph consistently uses "room ID" and "view ID"/"viewId".
602-619: Fix indentation in themessageaction sample.The
msgbuilder chain is indented with one tab on the chained methods, which is inconsistent with the surrounding modal/ctx blocks (2-tab chain indentation) and renders as visually broken in most Markdown viewers.✏️ Suggested indentation
- const msg = modify - .getCreator() - .startMessage() - .setRoom(context.getRoom()) - .setBlocks([ + const msg = modify + .getCreator() + .startMessage() + .setRoom(context.getRoom()) + .setBlocks([🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/data/apps/app-packages/README.md` around lines 602 - 619, The chained message builder in the 'message' action sample is misindented (the chained calls on modify.getCreator() -> startMessage() -> setRoom() -> setBlocks() are indented with one tab), making the block visually inconsistent; align the chaining indentation to match the surrounding examples by indenting the chained methods for the msg variable the same as other blocks so startMessage(), setRoom(), setBlocks() and their array/object contents use the two-tab style, and ensure the await modify.getCreator().finish(msg) call remains aligned with the surrounding control flow.apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts (2)
96-97: Standardize locator strategy for surfaces and extract to constants.The same contextual bar / modal surfaces are queried inconsistently —
getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' })on Line 96 vsgetByLabel('UIKit Room Test Contextual Bar')on Lines 188/238, and similarlygetByRole('dialog', { name: 'UIKit Room Test Modal' })on Line 130 vsgetByLabel('UIKit Room Test Modal')on Lines 160/214. Picking one strategy per surface (and moving the title strings into module-level constants) improves reliability and readability, and aligns with the guideline to store commonly used locators in variables/constants.♻️ Example constants + consistent locator
+const MODAL_TITLE = 'UIKit Room Test Modal'; +const CTX_BAR_TITLE = 'UIKit Room Test Contextual Bar'; + test.describe.serial('Apps > UIKit interactions data', () => { ... - const surface = page.getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' }); + const surface = page.getByRole('dialog', { name: CTX_BAR_TITLE }); ... - await page.getByLabel('UIKit Room Test Contextual Bar').getByRole('button', { name: 'Submit' }).click(); + await page.getByRole('dialog', { name: CTX_BAR_TITLE }).getByRole('button', { name: 'Submit' }).click();As per coding guidelines: "Store commonly used locators in variables/constants for reuse".
Also applies to: 130-131, 160-160, 188-188, 214-214, 238-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts` around lines 96 - 97, The test uses inconsistent locator strategies for the same surfaces (calling page.getByRole('dialog', { name: ... }) in one place and page.getByLabel(...) elsewhere), making maintenance brittle; extract the surface title strings into module-level constants (e.g., UIKIT_ROOM_TEST_CONTEXTUAL_BAR, UIKIT_ROOM_TEST_MODAL) and standardize all lookups to a single strategy (prefer page.getByRole('dialog', { name: ... }) for dialog surfaces) by replacing occurrences of getByLabel(...) and inline strings and reusing the new constants (refer to the existing surface variable and the button click lines that call getByRole/getByLabel to locate replacements).
30-34: Remove explanatory comments from test implementation.The spec contains many inline narrative comments (
// Send a message with a button...,// Fetch app logs and validate,// Verify room is present, etc.). Per coding guidelines for**/*.{ts,tsx,js}, avoid code comments in implementation. Wrapping the logical sections intest.step('...', ...)would both self-document the flow and satisfy the guideline for complex scenarios.♻️ Example with
test.step()replacing comments- // Send a message with a button via the slash command - await poHomeChannel.content.dispatchSlashCommand(`/open-uikit-room-test-modal message ${seed}`); - - // Wait for the message with the button to appear and click it - await page.getByRole('button', { name: 'Click!' }).click(); - - // Fetch app logs and validate - const logsResult = await getAppLogs(api, appId); + await test.step('dispatch slash command and click message button', async () => { + await poHomeChannel.content.dispatchSlashCommand(`/open-uikit-room-test-modal message ${seed}`); + await page.getByRole('button', { name: 'Click!' }).click(); + }); + + const logsResult = await test.step('fetch app logs', () => getAppLogs(api, appId));As per coding guidelines: "Avoid code comments in the implementation" and "Use
test.step()for complex test scenarios to improve organization in Playwright tests".Also applies to: 49-50, 52-53, 55-55, 62-62, 66-66, 70-70, 74-74, 78-78, 86-90, 95-95, 99-99, 103-103, 107-107, 111-111, 115-115, 119-119, 126-126, 129-129, 133-133, 137-137, 141-141, 145-145, 149-149, 156-156, 159-159, 162-162, 173-173, 181-181, 184-184, 187-187, 190-190, 194-194, 198-198, 202-202, 210-210, 213-213, 216-216, 223-223, 231-231, 234-234, 237-237, 240-240, 244-244, 248-248, 252-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts` around lines 30 - 34, The test file contains many inline narrative comments inside the Playwright test implementation; remove those comments and wrap each logical section in test.step(...) calls to self-document the flow instead. Locate the main test function (the test('...') in uikit-interactions.spec.ts) and for each commented block (e.g., steps for sending a message, fetching app logs, verifying room presence, asserting UI interactions) replace the comments with test.step('brief description', async () => { ...original code... }) so the code remains unchanged but organized into named steps; ensure you import/keep Playwright's test object in scope and do not leave any explanatory // comments in the implementation.apps/meteor/ee/server/apps/communication/uikit.ts (1)
252-271: Consider also propagatingtriggerId/actionIdonviewClosed.The
viewClosedaction object still omitstriggerIdandactionId, which the other branches do include. This is pre-existing, but since you're already extending viewClosed forroomand the test app logsview_closed_triggerId/view_closed_actionId, it may be worth carrying them through in the same PR (or a follow-up) to keep the handler data complete. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/ee/server/apps/communication/uikit.ts` around lines 252 - 271, The 'viewClosed' branch in apps/meteor/ee/server/apps/communication/uikit.ts builds an action payload that omits triggerId and actionId; update the case 'viewClosed' handler to extract triggerId and actionId from req.body.payload (like other branches do) and include them in the action.payload object so the constructed action (variable action) contains view, isCleared, triggerId, and actionId (handling undefined if absent).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`:
- Around line 123-151: The modal block-action test "should include correct data
in executeBlockActionHandler when triggered in a modal surface" is missing an
assertion for the room propagation (block_action_room); update the test to
locate the block_action_room entry on the found blockActionLog (use the existing
findLogItem/blockActionLog lookup) and assert its args[1] equals the expected
value (e.g., 'GENERAL' or 'no-room' per CORE-1264) so regressions are caught; if
room propagation is intentionally unspecified, add a short test.step or comment
inside the same test clarifying that no room assertion is expected to avoid
future confusion.
In `@apps/meteor/tests/e2e/utils/apps.ts`:
- Around line 14-17: The headers object in apps/meteor/tests/e2e/utils/apps.ts
(and the same bug in apps/meteor/tests/e2e/fixtures/addCustomOAuth.ts) uses
Users.admin.data.username for 'X-User-Id' which should be Users.admin.data._id;
update all occurrences so 'X-User-Id' is set to the admin _id, not username.
While here, DRY the bootstrap by extracting a shared helper that returns the
admin headers and a request.newContext() (used by insertDefaultTestApp,
installLocalTestPackage, and uninstallApp) and replace duplicated header/context
creation with that helper.
In `@packages/ui-kit/src/interactions/UserInteraction.ts`:
- Line 33: The core-app viewSubmit/viewClosed path is not passing room
context—ensure the payloads include rid and that the core-app handlers receive
it: add rid to the UiKit core view payload type (the existing optional rid?:
string in UserInteraction.ts or the specific
UiKitCoreAppViewSubmitPayload/UiKitCoreAppViewClosedPayload) and, in
apps/meteor/ee/server/apps/communication/uikit.ts where viewSubmit and
viewClosed are dispatched, extract rid from the request body (same way
blockAction does) and pass rid (or the derived room object if handlers expect
it) into the handler invocation so core-app handlers in
apps/meteor/server/modules/core-apps/*.ts get the room context.
---
Nitpick comments:
In @.changeset/tricky-kiwis-sit.md:
- Line 6: Update the changelog entry wording in .changeset/tricky-kiwis-sit.md
to read "Adds support for room information (`rid`) on ViewSubmit and ViewClose
events for the ContextualBar surface." — specifically replace the phrase "Adds
support to room information on..." with "Adds support for room information
(`rid`) on..." so the sentence reads clearly and includes the `rid` identifier.
In `@apps/meteor/ee/server/apps/communication/uikit.ts`:
- Around line 252-271: The 'viewClosed' branch in
apps/meteor/ee/server/apps/communication/uikit.ts builds an action payload that
omits triggerId and actionId; update the case 'viewClosed' handler to extract
triggerId and actionId from req.body.payload (like other branches do) and
include them in the action.payload object so the constructed action (variable
action) contains view, isCleared, triggerId, and actionId (handling undefined if
absent).
In `@apps/meteor/tests/data/apps/app-packages/README.md`:
- Around line 457-477: The snippet duplicates imports from the same modules;
consolidate the duplicated imports from
'@rocket.chat/apps-engine/definition/metadata' (IAppInfo, AppMethod) into a
single import and similarly merge the two imports from
'@rocket.chat/apps-engine/definition/uikit' (UIKitViewSubmitInteractionContext,
UIKitViewCloseInteractionContext, UIKitBlockInteractionContext,
IUIKitInteractionHandler, IUIKitResponse, UIKitSurfaceType) so each module is
imported once; update the import statements that reference symbols like
IAppInfo, AppMethod, UIKitSurfaceType, UIKitViewSubmitInteractionContext,
UIKitViewCloseInteractionContext, UIKitBlockInteractionContext,
IUIKitInteractionHandler, and IUIKitResponse accordingly.
- Line 451: Update the README sentence to use the canonical casing for the view
identifier: change "view id" to "view ID" (or use camelCase "viewId" to match
`triggerId`/`actionId`) in the description of the interaction handlers
(`executeBlockActionHandler`, `executeViewSubmitHandler`,
`executeViewClosedHandler`) so the paragraph consistently uses "room ID" and
"view ID"/"viewId".
- Around line 602-619: The chained message builder in the 'message' action
sample is misindented (the chained calls on modify.getCreator() ->
startMessage() -> setRoom() -> setBlocks() are indented with one tab), making
the block visually inconsistent; align the chaining indentation to match the
surrounding examples by indenting the chained methods for the msg variable the
same as other blocks so startMessage(), setRoom(), setBlocks() and their
array/object contents use the two-tab style, and ensure the await
modify.getCreator().finish(msg) call remains aligned with the surrounding
control flow.
In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`:
- Around line 96-97: The test uses inconsistent locator strategies for the same
surfaces (calling page.getByRole('dialog', { name: ... }) in one place and
page.getByLabel(...) elsewhere), making maintenance brittle; extract the surface
title strings into module-level constants (e.g., UIKIT_ROOM_TEST_CONTEXTUAL_BAR,
UIKIT_ROOM_TEST_MODAL) and standardize all lookups to a single strategy (prefer
page.getByRole('dialog', { name: ... }) for dialog surfaces) by replacing
occurrences of getByLabel(...) and inline strings and reusing the new constants
(refer to the existing surface variable and the button click lines that call
getByRole/getByLabel to locate replacements).
- Around line 30-34: The test file contains many inline narrative comments
inside the Playwright test implementation; remove those comments and wrap each
logical section in test.step(...) calls to self-document the flow instead.
Locate the main test function (the test('...') in uikit-interactions.spec.ts)
and for each commented block (e.g., steps for sending a message, fetching app
logs, verifying room presence, asserting UI interactions) replace the comments
with test.step('brief description', async () => { ...original code... }) so the
code remains unchanged but organized into named steps; ensure you import/keep
Playwright's test object in scope and do not leave any explanatory // comments
in the implementation.
In `@apps/meteor/tests/e2e/utils/apps.ts`:
- Around line 11-21: In insertDefaultTestApp, avoid the hard-coded string 'test'
and add assertions: replace the literal 'test' with a named constant (e.g.,
DEFAULT_VIDEO_CONF_PROVIDER or DEFAULT_PROVIDER) and capture the responses from
the two api.post calls (the apps POST and the settings POST for
VideoConf_Default_Provider) into variables, then add await
expect(response).toBeOK() (or equivalent) for each to ensure the requests
succeeded; reference function insertDefaultTestApp and the setting key
VideoConf_Default_Provider when making these changes.
🪄 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
Run ID: f680b6a2-20a8-4b33-8c51-37ce9523b544
⛔ Files ignored due to path filters (1)
apps/meteor/tests/data/apps/app-packages/uikit-room-test_0.0.1.zipis excluded by!**/*.zip
📒 Files selected for processing (12)
.changeset/angry-baboons-trade.md.changeset/tricky-kiwis-sit.mdapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/fixtures/insert-apps.tsapps/meteor/tests/e2e/utils/apps.tspackages/ui-kit/src/interactions/UserInteraction.ts
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/fixtures/insert-apps.ts
📜 Review details
⏰ 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). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tspackages/ui-kit/src/interactions/UserInteraction.tsapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/e2e/utils/apps.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
🧠 Learnings (44)
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/tricky-kiwis-sit.md.changeset/angry-baboons-trade.md
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
.changeset/tricky-kiwis-sit.md.changeset/angry-baboons-trade.mdapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/tricky-kiwis-sit.md.changeset/angry-baboons-trade.md
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
.changeset/tricky-kiwis-sit.mdapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/angry-baboons-trade.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-19T14:53:45.726Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39701
File: packages/apps-engine/tests/test-data/storage/storage.ts:13-23
Timestamp: 2026-03-19T14:53:45.726Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the `TestsAppStorage` singleton pattern (via `TestsAppStorage.getInstance()`) is intentional. The maintainer (d-gubert) has confirmed that potential cross-test state leakage from the process-wide singleton is not a concern for this test suite. Do not flag this as a state-leakage or flaky-test risk in future reviews.
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 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:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tspackages/ui-kit/src/interactions/UserInteraction.tsapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/e2e/utils/apps.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:
apps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/config/global-setup.tspackages/ui-kit/src/interactions/UserInteraction.tsapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
packages/ui-kit/src/interactions/UserInteraction.tsapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
packages/ui-kit/src/interactions/UserInteraction.ts
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
packages/ui-kit/src/interactions/UserInteraction.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/uikit/hooks/useContextualBarContextValue.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-04-13T16:40:55.864Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40100
File: apps/meteor/client/views/root/MainLayout/EmbeddedPreload.tsx:18-35
Timestamp: 2026-04-13T16:40:55.864Z
Learning: In Rocket.Chat's embedded mode (`?layout=embedded`), route changes are driven by the parent frame reloading the iframe URL, which causes a full remount of the React tree (including `EmbeddedPreload` at `apps/meteor/client/views/root/MainLayout/EmbeddedPreload.tsx`). The component never survives a navigation, so `roomParams` (computed via `useMemo`) is always fresh on mount. A `[router]` singleton as the sole `useMemo` dependency is therefore correct and intentional — do not flag it as a stale-dependency bug in this component.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-03-12T17:12:49.121Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:28-33
Timestamp: 2026-03-12T17:12:49.121Z
Learning: Rocket.Chat — apps/meteor/client/lib/chats/flows/uploadFiles.ts: When E2E_Enable_Encrypt_Files is disabled, plaintext file uploads are allowed in E2E rooms; this fallback is expected and should not be flagged as a security regression.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/ee/server/apps/communication/uikit.ts
🪛 LanguageTool
apps/meteor/tests/data/apps/app-packages/README.md
[style] ~451-~451: It’s more common nowadays to write this noun as one word.
Context: ...) logs detailed data including room ID, user name, view id, block value, triggerId, actio...
(RECOMMENDED_COMPOUNDS)
[grammar] ~451-~451: Ensure spelling is correct
Context: ...om ID, user name, view id, block value, triggerId, actionId, and container type. These logs are use...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
d-gubert
force-pushed
the
fix/uikit-interaction-room-data
branch
from
6da7cc1 to
b91e185
Compare
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/1d9fe17e-f376-436c-a62e-97a42942ac9e
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/1d9fe17e-f376-436c-a62e-97a42942ac9e
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/9124b5cb-6821-4135-a3ed-9ac62f9f0d24
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/4fce8021-7e0b-4eb1-8658-47de2f9629be
- Enhanced test app to support modal, contextual bar, and message surfaces - Added getAppLogs helper to e2e fixtures for fetching app logs - Updated installLocalTestPackage to return app data - Completed all 7 Playwright test cases validating room, user, triggerId, actionId, and container - Removed old non-applicable end-to-end test (uikit-room-context.ts) - Updated README documentation for the enhanced test app Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/ec71feb5-1416-4b80-8e2f-b03c571ccef4
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/ec71feb5-1416-4b80-8e2f-b03c571ccef4
Opening a contextual bar via the Apps Engine triggers a client-side
URL navigation (to /channel/{room}/app/{viewId}). Calling waitFor on
a locator while this navigation is still in progress causes Playwright
to throw "Target page, context or browser has been closed".
Add page.waitForURL(/\/app\//) after dispatching the contextual bar
slash command in all three contextual bar tests to ensure the
navigation completes before any locator operations.
Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/1998ca62-abf1-478a-835c-d3ea1053d273
test: improve tests
d-gubert
force-pushed
the
fix/uikit-interaction-room-data
branch
from
b91e185 to
4e6ecd6
Compare
Co-authored-by: Copilot <copilot@github.com>
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/tests/data/apps/app-packages/README.md (1)
486-495: Clarify expected room behavior per surface in this section.Please explicitly document that modal interactions can legitimately log no room context, while contextual bar/message interactions are expected to include room context. This avoids false assumptions when reading E2E logs.
Based on learnings: in RocketChat/Rocket.Chat, modal-originated UIKit interactions intentionally do not include room context (
ridis optional/absent for modals).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/data/apps/app-packages/README.md` around lines 486 - 495, Update the README description for the "open-uikit-room-test-modal" test app to explicitly state expected room-context behavior: note that modal-originated interactions may legitimately have no room ID (rid can be absent) while contextual bar and message interactions are expected to include room context; reference the interaction handlers executeBlockActionHandler, executeViewSubmitHandler, and executeViewClosedHandler so readers know which logs this applies to and add a short sentence that this aligns with Rocket.Chat behavior where modal interactions intentionally omit room context.apps/meteor/tests/e2e/utils/apps.ts (1)
11-49: Consider extracting the admin headers + context bootstrap.The
request.newContext()+ adminheadersblock is duplicated verbatim acrossinsertDefaultTestApp,installLocalTestPackage, anduninstallApp. A tiny helper would remove the triplication and centralize the auth convention.♻️ Example refactor
+async function getAdminApiContext() { + const api = await request.newContext(); + const headers = { + 'X-Auth-Token': Users.admin.data.loginToken, + 'X-User-Id': Users.admin.data._id, + }; + return { api, headers }; +} + export async function insertDefaultTestApp(): Promise<void> { - const api = await request.newContext(); - - const headers = { - 'X-Auth-Token': Users.admin.data.loginToken, - 'X-User-Id': Users.admin.data._id, - }; - + const { api, headers } = await getAdminApiContext(); await api.post(`${BASE_URL}/api/apps`, { data: { url: APP_URL }, headers }); await api.post(`${BASE_API_URL}/settings/VideoConf_Default_Provider`, { data: { value: 'test' }, headers }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/utils/apps.ts` around lines 11 - 49, Extract the repeated request.newContext() and admin headers creation into a small helper (e.g., getAdminApiContext or createAdminSession) that returns the context object and headers built from Users.admin.data (so it encapsulates request.newContext() and the 'X-Auth-Token'/'X-User-Id' values); then update insertDefaultTestApp, installLocalTestPackage, and uninstallApp to call that helper and use its returned { api, headers } instead of duplicating the bootstrapping logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/tests/data/apps/app-packages/README.md`:
- Line 494: The README uses inconsistent casing for "view id"; update all
occurrences to the standardized form "view ID" (or `viewId` if referring to code
identifiers) for consistency—specifically in the sentences describing
interaction handlers executeBlockActionHandler, executeViewSubmitHandler, and
executeViewClosedHandler where you list logged fields (room ID, user name, view
id, block value, triggerId, actionId, container type), change "view id" to "view
ID" (or `viewId`) and ensure the same form is used throughout the file.
In `@apps/meteor/tests/e2e/utils/apps.ts`:
- Around line 51-57: The getAppLogs function's declared return type
(ReturnType<Endpoints['/apps/:id/logs']['GET']> / PaginatedResult<{ logs:
ILogItem[] }>) doesn't match the actual HTTP body which is wrapped by
api.success and includes a success boolean; update the Promise return type of
getAppLogs to reflect the real shape (e.g. Promise<{ success: boolean; offset:
number; count: number; total: number; logs: ILogItem[] }>) or an equivalent
shared type so response.json() is typed correctly; locate and change the
signature on getAppLogs in apps.ts (and adjust any related type alias/import if
you prefer a reusable type) to fix the mismatch.
---
Nitpick comments:
In `@apps/meteor/tests/data/apps/app-packages/README.md`:
- Around line 486-495: Update the README description for the
"open-uikit-room-test-modal" test app to explicitly state expected room-context
behavior: note that modal-originated interactions may legitimately have no room
ID (rid can be absent) while contextual bar and message interactions are
expected to include room context; reference the interaction handlers
executeBlockActionHandler, executeViewSubmitHandler, and
executeViewClosedHandler so readers know which logs this applies to and add a
short sentence that this aligns with Rocket.Chat behavior where modal
interactions intentionally omit room context.
In `@apps/meteor/tests/e2e/utils/apps.ts`:
- Around line 11-49: Extract the repeated request.newContext() and admin headers
creation into a small helper (e.g., getAdminApiContext or createAdminSession)
that returns the context object and headers built from Users.admin.data (so it
encapsulates request.newContext() and the 'X-Auth-Token'/'X-User-Id' values);
then update insertDefaultTestApp, installLocalTestPackage, and uninstallApp to
call that helper and use its returned { api, headers } instead of duplicating
the bootstrapping logic.
🪄 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
Run ID: bc3d93a9-3373-4650-857c-b7d99ee00d69
⛔ Files ignored due to path filters (1)
apps/meteor/tests/data/apps/app-packages/uikit-room-test_0.0.1.zipis excluded by!**/*.zip
📒 Files selected for processing (12)
.changeset/angry-baboons-trade.md.changeset/tricky-kiwis-sit.mdapps/meteor/client/uikit/hooks/useContextualBarContextValue.tsapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/data/apps/app-packages/README.mdapps/meteor/tests/data/apps/app-packages/index.tsapps/meteor/tests/e2e/apps/uikit-interactions.spec.tsapps/meteor/tests/e2e/config/global-setup.tsapps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/fixtures/insert-apps.tsapps/meteor/tests/e2e/utils/apps.ts
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/fixtures/insert-apps.ts
✅ Files skipped from review due to trivial changes (4)
- .changeset/angry-baboons-trade.md
- apps/meteor/tests/data/apps/app-packages/index.ts
- .changeset/tricky-kiwis-sit.md
- apps/meteor/tests/e2e/config/global-setup.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/uikit/hooks/useContextualBarContextValue.ts
- apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📜 Review details
⏰ 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). (1)
- GitHub Check: 📦 Build Packages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/tests/e2e/utils/apps.tsapps/meteor/ee/server/apps/communication/uikit.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.ts
🧠 Learnings (44)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-04-10T21:17:22.932Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40096
File: apps/meteor/ee/server/apps/lib/redactor.ts:3-17
Timestamp: 2026-04-10T21:17:22.932Z
Learning: In RocketChat/Rocket.Chat, `X-User-Id` / `x-user-id` headers must NOT be added to redaction paths in apps log redaction (e.g., `apps/meteor/ee/server/apps/lib/redactor.ts`). The maintainer (d-gubert) has confirmed that X-User-Id is an identifier, not a credential — its presence in logs is useful for diagnostics, and `X-Auth-Token` is the only header that constitutes a real secret. Do not suggest redacting X-User-Id in future reviews of this area.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-09T23:46:52.173Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39492
File: apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts:22-24
Timestamp: 2026-03-09T23:46:52.173Z
Learning: In `apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts`, the `oAuth2ServerAuth` function's `authorization` field in `partialRequest` is exclusively expected to carry Bearer tokens. Basic authentication is not supported in this OAuth flow, so there is no need to guard against non-Bearer schemes when extracting the token from the `Authorization` header.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.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 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:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.tsapps/meteor/ee/server/apps/communication/uikit.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:
apps/meteor/tests/e2e/fixtures/addCustomOAuth.tsapps/meteor/tests/e2e/utils/apps.tsapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-04-17T17:38:15.994Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-04-17T18:33:27.211Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsxapps/meteor/ee/server/apps/communication/uikit.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-04-20T17:11:59.452Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40225
File: apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts:55-71
Timestamp: 2026-04-20T17:11:59.452Z
Learning: In `apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts`, the concern about an empty `?appId=` query param bypassing the truthy check and overriding the path `appId` in the `makeAppLogsQuery` spread is not relevant. The AJV query schema (`isAppLogsProps`) validates and rejects invalid/empty `appId` values before the action handler is reached, making the in-handler guard sufficient as-is. Do not flag this pattern as a vulnerability in future reviews of this file.
Applied to files:
apps/meteor/tests/e2e/utils/apps.tsapps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/tests/e2e/utils/apps.tsapps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/apps.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-04-23T18:10:53.335Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39857
File: apps/meteor/app/api/server/middlewares/metrics.ts:25-57
Timestamp: 2026-04-23T18:10:53.335Z
Learning: In RocketChat/Rocket.Chat, the route handler action wrapper in `apps/meteor/app/api/server/ApiClass.ts` (around line 892) contains a `catch` block that intercepts all route-handler errors and converts them into proper HTTP API response objects (e.g., `api.failure`, `api.unauthorized`, `api.tooManyRequests`). As a result, `next()` in Hono middleware (such as `metricsMiddleware` in `apps/meteor/app/api/server/middlewares/metrics.ts`) will never throw. Do not flag missing try/finally guards around `await next()` calls in this middleware for gauge/counter decrement safety, as the execution flow is never disrupted by route-level errors.
Applied to files:
apps/meteor/ee/server/apps/communication/uikit.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-19T14:53:45.726Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39701
File: packages/apps-engine/tests/test-data/storage/storage.ts:13-23
Timestamp: 2026-03-19T14:53:45.726Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the `TestsAppStorage` singleton pattern (via `TestsAppStorage.getInstance()`) is intentional. The maintainer (d-gubert) has confirmed that potential cross-test state leakage from the process-wide singleton is not a concern for this test suite. Do not flag this as a state-leakage or flaky-test risk in future reviews.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
📚 Learning: 2026-03-12T17:12:49.121Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:28-33
Timestamp: 2026-03-12T17:12:49.121Z
Learning: Rocket.Chat — apps/meteor/client/lib/chats/flows/uploadFiles.ts: When E2E_Enable_Encrypt_Files is disabled, plaintext file uploads are allowed in E2E rooms; this fallback is expected and should not be flagged as a security regression.
Applied to files:
apps/meteor/tests/data/apps/app-packages/README.md
🪛 LanguageTool
apps/meteor/tests/data/apps/app-packages/README.md
[style] ~494-~494: It’s more common nowadays to write this noun as one word.
Context: ...) logs detailed data including room ID, user name, view id, block value, triggerId, actio...
(RECOMMENDED_COMPOUNDS)
🔇 Additional comments (6)
apps/meteor/tests/data/apps/app-packages/README.md (1)
527-574: UIKit handler examples look aligned with interaction contracts.The documented handlers correctly use
context.getInteractionData()plus respondersuccessResponse(), and computedAppMethodkeys are consistent with Apps-Engine UIKit interaction handler patterns.apps/meteor/tests/e2e/fixtures/addCustomOAuth.ts (1)
11-11: LGTM —X-User-Idnow uses_id.Matches the pattern used in
apps/meteor/tests/e2e/utils/apps.tsand the admin user's stored_id, fixing the API auth mismatch.apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx (1)
28-89: LGTM — rid propagated consistently across submit/cancel/close.
room._idis threaded into the context value and into all threeemitInteractionpayloads (viewSubmit,viewClosedcleared/uncleared), matching the contextual-bar surface contract. SinceUiKitContextualBaronly renders inside a room,useRoom()is safe here.apps/meteor/ee/server/apps/communication/uikit.ts (3)
206-212: LGTM — handler binding is correct.Converting
routeHandlerfrom an arrow class-field to a regular async method and registering it with.bind(this)preservesthisinside the handler and avoids the field-initialization ordering pitfall. No behavior change for callers.
252-297: LGTM — symmetricrid → roomhandling forviewClosedandviewSubmit.Both branches guard with
rid ? ... : undefined, correctly handling the modal surface case whereridis absent.convertByIdmay returnundefined(perIAppRoomsConverter.convertById), and the downstreamaction.roomis already typed as optional, so passingundefinedis safe.
218-218: No action required—ridis already correctly typed as optional on all three interaction variants.The handler correctly destructures
ridfromreq.bodywithin each case.ViewBlockActionUserInteraction,ViewClosedUserInteraction, andViewSubmitUserInteractionall definerid?: stringinpackages/ui-kit/src/interactions/UserInteraction.ts. TypeScript will correctly inferridas optional in all three cases, and the conditional room lookup (rid ? await orch.getConverters()?.get('rooms').convertById(rid): undefined`) safely handles the optional value.> Likely an incorrect or invalid review comment.
Co-authored-by: Copilot <copilot@github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts (3)
161-164: Assertion wording is inconsistent with what is actually checked.The
test.steplabel says "room is intentionally absent for modal block actions" and theexpectcustom message says "Room entry should not be present for modal block action", but the assertion istoBe('no-room')— i.e. the entry must be present with the app's fallback value. The test is functionally correct (it verifies the app received no room and fell back), but the wording reads as if you were asserting the entry is missing, which would betoBeUndefined(). Consider aligning the message with the actual assertion to avoid confusing future readers.✏️ Suggested wording tweak
await test.step('room is intentionally absent for modal block actions (modals have no room context)', async () => { const roomEntry = blockActionLog?.entries.find((e) => e.args[0] === 'block_action_room'); - expect(roomEntry?.args[1], 'Room entry should not be present for modal block action').toBe('no-room'); + expect(roomEntry?.args[1], 'Modal block action should fall back to "no-room" (no room context)').toBe('no-room'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts` around lines 161 - 164, The test's wording implies the room entry is absent but the assertion checks it equals the fallback 'no-room'; update the labels/messages to match the actual check: in the test.step that references modal block actions and the expect call around blockActionLog/roomEntry (the variables roomEntry and blockActionLog are used), change the step title and the expect's custom message to state that the app should receive a fallback room value (e.g., "Room entry should be the fallback 'no-room' for modal block action") so the description aligns with the toBe('no-room') assertion.
30-34: Prefertest.step()over inline narrative comments.Most test bodies use single-line
//comments to narrate setup and each assertion group (e.g., "Send a message with a button via the slash command", "Verify room is present", "Verify user is present", "Wait for the client-side navigation..."). This conflicts with the repository's "avoid code comments in the implementation" rule and under-uses the project's preferred organization primitive (test.step). Only Line 161 actually wraps its intent in atest.step; the rest would be more consistent and more readable in the Playwright report if the narrative blocks were wrapped the same way. The JSDoc forfindLogItem(Lines 30–34) can also be replaced by a descriptive name and a type signature.Example refactor for one test (apply the same pattern to the others):
♻️ Proposed refactor — replace narrative comments with `test.step`
test('should include correct data in executeBlockActionHandler when triggered in a message', async ({ api, page }) => { const seed = Date.now().toString(); - // Send a message with a button via the slash command - await poHomeChannel.content.dispatchSlashCommand(`/open-uikit-room-test-modal message ${seed}`); - - const interactionRequest = page.waitForRequest('**/ui.interaction/*'); - - // Wait for the message with the button to appear and click it - await page.getByRole('button', { name: 'Click!' }).click(); - - await interactionRequest; - - // Fetch app logs and validate - const logsResult = await getAppLogs(api, appId); - expect(logsResult.logs).toBeDefined(); - - const blockActionLog = findLogItem(logsResult.logs, 'executeBlockActionHandler', ['block_action_value', seed]); - expect(blockActionLog, 'Block action handler log not found for message').toBeTruthy(); - - // Verify room is present (GENERAL room) - const roomEntry = blockActionLog?.entries.find((e) => e.args[0] === 'block_action_room'); - expect(roomEntry?.args[1], 'Room id should be present for message block action').toBe('GENERAL'); - - // Verify user is present - const userEntry = blockActionLog?.entries.find((e) => e.args[0] === 'block_action_user'); - expect(userEntry?.args[1], 'User should be present for message block action').toBe('user1'); - // ... + await test.step('dispatch slash command to send a message with a button', async () => { + await poHomeChannel.content.dispatchSlashCommand(`/open-uikit-room-test-modal message ${seed}`); + }); + + await test.step('click the button and wait for ui.interaction', async () => { + const interactionRequest = page.waitForRequest('**/ui.interaction/*'); + await page.getByRole('button', { name: 'Click!' }).click(); + await interactionRequest; + }); + + const blockActionLog = await test.step('fetch app logs for block action handler', async () => { + const logsResult = await getAppLogs(api, appId); + expect(logsResult.logs).toBeDefined(); + const log = findLogItem(logsResult.logs, 'executeBlockActionHandler', ['block_action_value', seed]); + expect(log, 'Block action handler log not found for message').toBeTruthy(); + return log; + }); + + await test.step('block action includes room, user, triggerId, actionId and message container', async () => { + expect(blockActionLog?.entries.find((e) => e.args[0] === 'block_action_room')?.args[1]).toBe('GENERAL'); + expect(blockActionLog?.entries.find((e) => e.args[0] === 'block_action_user')?.args[1]).toBe('user1'); + expect(blockActionLog?.entries.find((e) => e.args[0] === 'block_action_triggerId')?.args[1]).not.toBe('no-triggerId'); + expect(blockActionLog?.entries.find((e) => e.args[0] === 'block_action_actionId')?.args[1]).toBe('msg-button'); + expect(blockActionLog?.entries.find((e) => e.args[0] === 'block_action_container')?.args[1]).toBe('message'); + }); });As per coding guidelines:
**/*.{ts,tsx,js}→ "Avoid code comments in the implementation" andapps/meteor/tests/e2e/**/*.spec.ts→ "Usetest.step()for complex test scenarios to improve organization in Playwright tests".Also applies to: 49-49, 54-54, 59-59, 66-66, 70-70, 74-74, 78-78, 82-82, 90-90, 93-96, 101-101, 107-107, 111-111, 115-115, 119-119, 123-123, 127-127, 134-134, 139-139, 145-145, 149-149, 153-153, 157-157, 166-166, 173-173, 178-178, 183-183, 190-190, 194-194, 202-202, 205-205, 210-210, 215-215, 219-219, 223-223, 227-227, 235-235, 240-240, 245-245, 252-252, 260-260, 263-263, 268-268, 273-273, 277-277, 281-281, 285-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts` around lines 30 - 34, Replace inline narrative comments in apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts with Playwright test.step calls and convert the JSDoc for findLogItem into an explicit descriptive function name and TypeScript signature: locate each commented block (e.g., "Send a message with a button via the slash command", "Verify room is present", etc.) and wrap the code it describes in await test.step('short description', async () => { ... }), and rename/replace the JSDoc-decorated helper with a typed function name (e.g., findLogEntryByHandlerLabel(handler: string, label: string): LogItem | undefined) so the intent is clear without comments; apply this pattern to all listed line locations and any other narrative comments in the file.
102-103: Use a consistent locator for the dialog/modal surfaces.The same UI Kit surfaces are located two different ways across the file:
- Contextual bar:
page.getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' })at Line 102 vspage.getByLabel('UIKit Room Test Contextual Bar')at Lines 211 and 269.- Modal:
page.getByRole('dialog', { name: 'UIKit Room Test Modal' })at Line 140 vspage.getByLabel('UIKit Room Test Modal')at Lines 179 and 241.
getByRole('dialog', { name })is more specific (it scopes to the dialog role) and matches what the first block-action tests already use. Extracting the two surface locators into shared constants near the top of the describe block would make the tests more robust and remove the duplication.♻️ Suggested consolidation
test.describe.serial('Apps > UIKit interactions data', () => { test.skip(!IS_EE, 'Premium Only'); let poHomeChannel: HomeChannel; let appId: string; + + const contextualBar = (page: Page) => page.getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' }); + const modal = (page: Page) => page.getByRole('dialog', { name: 'UIKit Room Test Modal' });Then replace each
page.getByLabel(...)/page.getByRole('dialog', ...)site withcontextualBar(page)/modal(page).Also applies to: 140-141, 179-179, 211-211, 241-241, 269-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts` around lines 102 - 103, Tests use inconsistent locators for the same UI Kit surfaces; add two shared locator helpers (e.g., contextualBar(page) and modal(page)) near the top of the describe block that each return page.getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' }) and page.getByRole('dialog', { name: 'UIKit Room Test Modal' }) respectively, then replace every direct call to page.getByLabel(...) and the scattered page.getByRole('dialog', { name: ... }) usages (including where the variable surface is assigned and where modal/dialog are referenced) with these helpers so all tests consistently use contextualBar(page) and modal(page).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`:
- Around line 161-164: The test's wording implies the room entry is absent but
the assertion checks it equals the fallback 'no-room'; update the
labels/messages to match the actual check: in the test.step that references
modal block actions and the expect call around blockActionLog/roomEntry (the
variables roomEntry and blockActionLog are used), change the step title and the
expect's custom message to state that the app should receive a fallback room
value (e.g., "Room entry should be the fallback 'no-room' for modal block
action") so the description aligns with the toBe('no-room') assertion.
- Around line 30-34: Replace inline narrative comments in
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts with Playwright test.step
calls and convert the JSDoc for findLogItem into an explicit descriptive
function name and TypeScript signature: locate each commented block (e.g., "Send
a message with a button via the slash command", "Verify room is present", etc.)
and wrap the code it describes in await test.step('short description', async ()
=> { ... }), and rename/replace the JSDoc-decorated helper with a typed function
name (e.g., findLogEntryByHandlerLabel(handler: string, label: string): LogItem
| undefined) so the intent is clear without comments; apply this pattern to all
listed line locations and any other narrative comments in the file.
- Around line 102-103: Tests use inconsistent locators for the same UI Kit
surfaces; add two shared locator helpers (e.g., contextualBar(page) and
modal(page)) near the top of the describe block that each return
page.getByRole('dialog', { name: 'UIKit Room Test Contextual Bar' }) and
page.getByRole('dialog', { name: 'UIKit Room Test Modal' }) respectively, then
replace every direct call to page.getByLabel(...) and the scattered
page.getByRole('dialog', { name: ... }) usages (including where the variable
surface is assigned and where modal/dialog are referenced) with these helpers so
all tests consistently use contextualBar(page) and modal(page).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 577e29fa-e122-456c-9445-4978a986dc0a
📒 Files selected for processing (1)
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
🧠 Learnings (28)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
📚 Learning: 2026-04-17T18:33:24.670Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:24.670Z
Learning: In Rocket.Chat UI Kit e2e tests, when testing `executeBlockActionHandler` flows that originate from a **modal** surface, do not treat a missing `block_action_room` / room property in the interaction payload as a test-coverage failure. Modals are not room-scoped, so the room id should be unavailable in this context. Document this explicitly in the test by using `test.step` (or equivalent) to assert the room entry is `undefined`.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-04-17T17:38:15.994Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-04-20T17:11:59.452Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40225
File: apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts:55-71
Timestamp: 2026-04-20T17:11:59.452Z
Learning: In `apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts`, the concern about an empty `?appId=` query param bypassing the truthy check and overriding the path `appId` in the `makeAppLogsQuery` spread is not relevant. The AJV query schema (`isAppLogsProps`) validates and rejects invalid/empty `appId` values before the action handler is reached, making the in-handler guard sufficient as-is. Do not flag this pattern as a vulnerability in future reviews of this file.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-04-10T22:42:05.539Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Avoid using `page.locator()` in Playwright tests - always prefer semantic locators such as `page.getByRole()`, `page.getByLabel()`, `page.getByText()`, or `page.getByTitle()`
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.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 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:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.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:
apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts
tassoevan
approved these changes
May 5, 2026
tassoevan
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
This pull request enhances the contextual bar UI Kit integration by ensuring the room ID (rid) is consistently included and handled throughout the UI interaction flow. This allows the backend to associate UI Kit interactions with the correct room context, enabling more context-aware app behaviors.
Issue(s)
CORE-1264
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores