Conversation
|
Contributor
|
Looks like this PR is ready to merge! 🎉 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces many test-only Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
juliajforesti
force-pushed
the
chore/message-data-qa
branch
from
6792b46 to
664e0f0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38493 +/- ##
===========================================
+ Coverage 70.64% 70.69% +0.05%
===========================================
Files 3189 3195 +6
Lines 112715 113085 +370
Branches 20363 20502 +139
===========================================
+ Hits 79627 79951 +324
- Misses 31044 31085 +41
- Partials 2044 2049 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/10 21:44 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 10 Feb 2026 21:44:09 GMT |
juliajforesti
force-pushed
the
chore/message-data-qa
branch
2 times, most recently
from
8e516ba to
44c316f
Compare
juliajforesti
force-pushed
the
chore/message-data-qa
branch
4 times, most recently
from
a88124e to
d00fa21
Compare
juliajforesti
force-pushed
the
chore/message-data-qa
branch
from
d00fa21 to
3e84fc0
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 39 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/tests/e2e/page-objects/encrypted-room.ts">
<violation number="1" location="apps/meteor/tests/e2e/page-objects/encrypted-room.ts:19">
P2: lastNthMessage now indexes from the start of the message list, so it no longer returns the nth-from-last message. This changes behavior (e.g., lastNthMessage(1) resolves to the first message) and can break tests relying on the previous semantics.</violation>
</file>
<file name="apps/meteor/tests/e2e/page-objects/fragments/home-content.ts">
<violation number="1" location="apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:81">
P2: `lastUserMessageNotSequential` filters by a descendant `[data-sequential="false"]`, but `data-sequential` is on the message listitem itself. This makes the locator return no elements. Filter on the listitem attribute directly so non‑sequential messages are found.</violation>
</file>
<file name="apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.ts">
<violation number="1" location="apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.ts:15">
P2: This new locator drops the role-based checkbox lookup and exact matching. The Playwright guidelines in this repo recommend `getByRole(..., exact: true)` when possible to avoid ambiguous matches. Use the checkbox role locator (or a label `has` locator) instead of `label` + `hasText`.</violation>
<violation number="2" location="apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.ts:19">
P2: This locator no longer targets the checkbox role or uses exact matching, which can make the test flaky if similar label text exists. Prefer the role-based checkbox locator with `exact: true`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
273-288:⚠️ Potential issue | 🟡 MinorApply filename filtering for thread messages too
Same concern as the main list: selecting
.last()before filtering can miss the intended file message if other thread messages appear.♻️ Suggested filtering
getLastThreadMessageByFileName(filename: string): Locator { - return this.threadMessageListItems.last().getByRole('link', { name: filename }); + return this.threadMessageListItems + .filter({ has: this.page.getByRole('link', { name: filename }) }) + .last() + .getByRole('link', { name: filename }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 273 - 288, The thread-message locators call .last() before applying filename-specific selection which can pick the wrong item; update getLastThreadMessageByFileName, lastThreadMessageFileDescription and lastThreadMessageTextAttachment to first filter threadMessageListItems for the element that contains the file link (use threadMessageListItems.filter or threadMessageListItems.locator(...).first/last on the filtered set) using the link role with name=filename, then call .last() on that filtered locator so you always pick the last matching thread message; reference threadMessageListItems, getLastThreadMessageByFileName, lastThreadMessageFileDescription and lastThreadMessageTextAttachment when making the change.
🧹 Nitpick comments (4)
apps/meteor/client/components/message/IgnoredContent.tsx (1)
20-20: Consider addingaria-labelif this element needs an accessible name.
aria-roledescriptiononly affects how the role is announced and does not contribute to the element's accessible name. If an accessible name is required, usearia-labeloraria-labelledbyalongsidearia-roledescription.Suggested change
- <MessageBody role='document' aria-roledescription={t('message_body')} dir='auto'> + <MessageBody + role='document' + aria-roledescription={t('message_body')} + aria-label={t('message_body')} + dir='auto' + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/message/IgnoredContent.tsx` at line 20, The MessageBody element in IgnoredContent.tsx currently sets aria-roledescription but not an accessible name; if this region needs to be announced with a name, add an accessible name via aria-label or aria-labelledby on MessageBody (or supply a nearby element ID and use aria-labelledby) in addition to aria-roledescription; update the MessageBody JSX to include aria-label="…" or aria-labelledby="someId" and ensure the labeled text is localized via t('...') when appropriate.apps/meteor/client/components/message/content/attachments/structure/AttachmentText.tsx (1)
7-7:aria-roledescriptionis hardcoded English while sibling components use i18n.
MessageContentBody.tsxsetsaria-roledescription={t('message_body')}with a proper i18n key, butAttachmentTexthard-codes'message attachment'. Non-English locales will render an untranslated ARIA description on attachments.Consider adding a translation key (e.g.,
message_attachment) and switching toaria-roledescription={t('message_attachment')}for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/message/content/attachments/structure/AttachmentText.tsx` at line 7, The ARIA roledescription in AttachmentText is hardcoded to 'message attachment'; update the Box in AttachmentText.tsx to use the i18n translator (t) instead of the literal string, e.g. replace aria-roledescription='message attachment' with aria-roledescription={t('message_attachment')}, add the new i18n key "message_attachment" to the translation files, and ensure AttachmentText imports/uses the same t function as MessageContentBody (or receives it via props) so accessibility labels are localized.apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (2)
519-523: Reuse the Reply-in-thread locator to avoid duplicate lookupsCreate a local locator and use it for wait/click.
♻️ Suggested refactor
async openReplyInThread(): Promise<void> { await this.lastUserMessage.hover(); - await this.lastUserMessage.getByRole('button', { name: 'Reply in thread' }).waitFor(); - await this.lastUserMessage.getByRole('button', { name: 'Reply in thread' }).click(); + const replyButton = this.lastUserMessage.getByRole('button', { name: 'Reply in thread' }); + await replyButton.waitFor(); + await replyButton.click(); }As per coding guidelines: Store commonly used locators in variables/constants for reuse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 519 - 523, In openReplyInThread, avoid performing duplicate lookups on this.lastUserMessage.getByRole('button', { name: 'Reply in thread' }) — create a local Locator (e.g., const replyButton = this.lastUserMessage.getByRole('button', { name: 'Reply in thread' })) and then call replyButton.waitFor() and replyButton.click(); this reduces repeated DOM queries and follows the locator-reuse guideline.
44-59: Keep list locators locale-stableThese rely on accessible names ("Message list", "Thread message list"). If those labels are localized, tests become language-dependent. Consider anchoring on non-localized attributes or centralizing the label source so tests track the same i18n key. Based on learnings: 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.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 44 - 59, The getByRole selectors (mainMessageList, threadMessageList, messageListItems, threadMessageListItems) rely on localized accessible names and should be replaced with locale-stable selectors; update these getters to target a non-localized attribute (e.g. data-qa-id or data-testid) placed on the list and listitem elements, or centralize the accessible name in a single i18n helper used by tests, so tests use stable attributes for interaction and only assert localized text separately when needed.
🤖 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/client/components/message/MessageContentBody.tsx`:
- Line 18: The test is brittle because MessageContentBody.tsx sets
aria-roledescription via t('message_body') (runtime/localized) while the
page-object body getter in message.ts expects a hard-coded English string;
change the selector to a stable, non-localized attribute and update the
component to emit it: in MessageContentBody.tsx (MessageBody element) add a
static attribute like data-message-type="message-body" (or use a constant
English string for aria-roledescription) and then update the body getter in
message.ts to query [data-message-type="message-body"] instead of matching the
localized aria-roledescription; ensure symbol references are MessageBody,
t('message_body') and the body getter in message.ts.
In `@apps/meteor/client/components/message/variants/SystemMessage.tsx`:
- Around line 89-93: The aria-roledescription on MessageSystemBody is hardcoded
to 'system message body' (in the conditional rendering block where messageType
is used) which breaks i18n; change the prop to use the translation function t
like the other components by replacing aria-roledescription='system message
body' with aria-roledescription={t('message_body')} so MessageSystemBody (used
with messageType.text(t, message)) is localized and consistent with
RoomMessageContent/ThreadMessageContent.
In `@apps/meteor/tests/e2e/channel-management.spec.ts`:
- Around line 252-268: The test uses a localized label in
poHomeChannel.content.lastUserMessageBody.getByRole('button', { name: 'This
message was ignored' }) which will break with different locales; update the app
component that renders the ignored-message button to include a stable attribute
(e.g., data-qa="ignored-message-button" or data-testid) and then change the test
to query that attribute instead of localized text (update the locator call in
the test within the 'should ignore user1 messages' test to use the new
data-qa/testid selector via the HomeChannel/poHomeChannel locator APIs).
In `@apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts`:
- Around line 192-196: The openLastMessageMenu function is targeting
this.lastThreadMessageText causing failures when no thread panel exists; change
the selector to operate on this.lastUserMessage instead: after hovering
this.lastUserMessage, call getByRole('button', { name: 'More', exact: true }) on
this.lastUserMessage (waitFor then click) so the "More" button is located within
the last user message element rather than the thread message element.
In `@apps/meteor/tests/e2e/messaging.spec.ts`:
- Around line 65-79: Inside the test.step, avoid calling
poHomeChannel.content.getMessageByText('msg1') twice: create a local variable
(e.g., msgLocator) assigned to poHomeChannel.content.getMessageByText('msg1') at
the top of the step and reuse msgLocator for the toolbar locator chain and the
expect assertion; update the two places that currently call
getMessageByText('msg1') to use that variable instead to improve readability and
follow the locator reuse guideline.
- Line 45: Create a Page Object method to encapsulate the system-message
selector instead of calling page.locator() directly in the test: add a method
like getFirstSystemMessage() on poHomeChannel.content (home-content.ts) that
returns the locator for '[role="listitem"][aria-roledescription="system
message"]' and then update the test in messaging.spec.ts to call
poHomeChannel.content.getFirstSystemMessage().first() and assert toBeFocused()
on that returned locator.
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts`:
- Around line 66-67: The assertions use localized UI strings via
agent.poHomeChannel.content.lastUserMessage.getByText('PDF Transcript
successfully generated') and getByRole('link', { name: 'Transcript' }) which is
brittle; change the checks to use stable selectors instead (e.g., a data-qa-id
on the transcript message or attachment, or assert the link's deterministic href
pattern) by updating the expectations to query by that stable attribute (replace
getByText/getByRole usage on lastUserMessage with getByAttribute/getBySelector
targeting the chosen data-qa-id or matching href) so the test doesn't depend on
localized text.
In `@apps/meteor/tests/e2e/page-objects/encrypted-room.ts`:
- Around line 18-20: The method lastNthMessage currently calls nthMessage(index
- 1) which counts from the start; restore original semantics by selecting from
the end using Playwright's negative nth indexing (nth(-index - 1)). Replace the
body of lastNthMessage to return a Message built from the underlying locator's
nth(-index - 1) (i.e., use the locator.nth(-index - 1) or directly call nth with
negative index) so lastNthMessage(1) yields the second-to-last message as
before.
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 201-211: getLastMessageByFileName assumes the last message
contains the given filename which can fail if newer messages appear; update the
locator in getLastMessageByFileName to first filter messageListItems for those
that contain a link with the specified filename (use the link role/name filter
on each item) and only then call .last() so you select the most recent message
that actually includes that filename (refer to messageListItems and
getLastMessageByFileName to locate the code).
- Around line 60-82: The locators are too broad and match unrelated elements
causing flakiness: update lastThreadMessagePreview to scope the thread-preview
selector to messageListItems (e.g., use
this.messageListItems.locator('[role="link"][aria-roledescription="thread
message preview"]').last()) and update lastUserMessageAttachment to be scoped to
the current message (e.g., use
this.lastUserMessage.locator('[role="document"][aria-roledescription="message
attachment"]') or this.messageListItems.locator(...).nth(...) as appropriate),
so both only query inside the intended message elements.
In `@apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.ts`:
- Around line 14-20: The getters doNotPrunePinned and filesOnly currently return
the label elements via this.form.locator('label', { hasText: ... }) which causes
.check()/.uncheck() to fail; update both getters to return the actual checkbox
input control by using this.form.getByLabel('Do not prune pinned messages') and
this.form.getByLabel('Only remove the attached files, keep messages')
respectively (or the equivalent localized text), and consider switching to
data-qa-id attributes in the UI and using those IDs in the selectors for locale
resilience.
In `@apps/meteor/tests/e2e/page-objects/fragments/message.ts`:
- Line 7: The selector in the message page-object uses a locale-dependent
aria-roledescription value; update the locator in
apps/meteor/tests/e2e/page-objects/fragments/message.ts so it no longer matches
'[aria-roledescription="message body"]' but instead uses a stable,
locale-independent hook (for example switch to a data-qa-id like
data-qa-id="message-body" or another agreed test-only attribute) that
MessageContentBody.tsx sets when rendering; change the MessageContentBody
component to render that same data-qa-id (or export a constant for the aria
value and use it in both places) and update the root.locator(...) call to query
the new attribute so tests work in non-English locales.
In `@apps/meteor/tests/e2e/system-messages.spec.ts`:
- Around line 19-20: findSysMes currently uses page.locator with a hardcoded
aria-roledescription string; change it to use semantic Playwright locators and
rely only on the stable data attribute: implement findSysMes to return
page.getByRole('listitem').filter({ has:
page.locator(`[data-system-message-type="${id}"]`) }) (reference the findSysMes
function and the data-system-message-type attribute) so you remove the localized
aria-roledescription and use a semantic role-based locator scoped to the stable
attribute.
---
Outside diff comments:
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 273-288: The thread-message locators call .last() before applying
filename-specific selection which can pick the wrong item; update
getLastThreadMessageByFileName, lastThreadMessageFileDescription and
lastThreadMessageTextAttachment to first filter threadMessageListItems for the
element that contains the file link (use threadMessageListItems.filter or
threadMessageListItems.locator(...).first/last on the filtered set) using the
link role with name=filename, then call .last() on that filtered locator so you
always pick the last matching thread message; reference threadMessageListItems,
getLastThreadMessageByFileName, lastThreadMessageFileDescription and
lastThreadMessageTextAttachment when making the change.
---
Nitpick comments:
In
`@apps/meteor/client/components/message/content/attachments/structure/AttachmentText.tsx`:
- Line 7: The ARIA roledescription in AttachmentText is hardcoded to 'message
attachment'; update the Box in AttachmentText.tsx to use the i18n translator (t)
instead of the literal string, e.g. replace aria-roledescription='message
attachment' with aria-roledescription={t('message_attachment')}, add the new
i18n key "message_attachment" to the translation files, and ensure
AttachmentText imports/uses the same t function as MessageContentBody (or
receives it via props) so accessibility labels are localized.
In `@apps/meteor/client/components/message/IgnoredContent.tsx`:
- Line 20: The MessageBody element in IgnoredContent.tsx currently sets
aria-roledescription but not an accessible name; if this region needs to be
announced with a name, add an accessible name via aria-label or aria-labelledby
on MessageBody (or supply a nearby element ID and use aria-labelledby) in
addition to aria-roledescription; update the MessageBody JSX to include
aria-label="…" or aria-labelledby="someId" and ensure the labeled text is
localized via t('...') when appropriate.
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 519-523: In openReplyInThread, avoid performing duplicate lookups
on this.lastUserMessage.getByRole('button', { name: 'Reply in thread' }) —
create a local Locator (e.g., const replyButton =
this.lastUserMessage.getByRole('button', { name: 'Reply in thread' })) and then
call replyButton.waitFor() and replyButton.click(); this reduces repeated DOM
queries and follows the locator-reuse guideline.
- Around line 44-59: The getByRole selectors (mainMessageList,
threadMessageList, messageListItems, threadMessageListItems) rely on localized
accessible names and should be replaced with locale-stable selectors; update
these getters to target a non-localized attribute (e.g. data-qa-id or
data-testid) placed on the list and listitem elements, or centralize the
accessible name in a single i18n helper used by tests, so tests use stable
attributes for interaction and only assert localized text separately when
needed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
apps/meteor/client/components/message/IgnoredContent.tsxapps/meteor/client/components/message/MessageContentBody.tsxapps/meteor/client/components/message/MessageHeader.tsxapps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsxapps/meteor/client/components/message/content/attachments/structure/AttachmentText.tsxapps/meteor/client/components/message/toolbar/MessageToolbarItem.tsxapps/meteor/client/components/message/toolbar/items/actions/ForwardMessageAction.tsxapps/meteor/client/components/message/toolbar/items/actions/JumpToMessageAction.tsxapps/meteor/client/components/message/toolbar/items/actions/QuoteMessageAction.tsxapps/meteor/client/components/message/toolbar/items/actions/ReactionMessageAction.tsxapps/meteor/client/components/message/toolbar/items/actions/ReplyInThreadMessageAction.tsxapps/meteor/client/components/message/variants/RoomMessage.tsxapps/meteor/client/components/message/variants/SystemMessage.tsxapps/meteor/client/components/message/variants/ThreadMessage.tsxapps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/file-upload.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-transcript.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/threads.spec.tspackages/i18n/src/locales/en.i18n.json
💤 Files with no reviewable changes (10)
- apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-transcript.ts
- apps/meteor/client/components/message/variants/RoomMessage.tsx
- apps/meteor/client/components/message/toolbar/items/actions/JumpToMessageAction.tsx
- apps/meteor/client/components/message/variants/ThreadMessage.tsx
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
- apps/meteor/client/components/message/toolbar/items/actions/QuoteMessageAction.tsx
- apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx
- apps/meteor/client/components/message/toolbar/items/actions/ReplyInThreadMessageAction.tsx
- apps/meteor/client/components/message/toolbar/items/actions/ForwardMessageAction.tsx
- apps/meteor/client/components/message/toolbar/items/actions/ReactionMessageAction.tsx
📜 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). (10)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/file-upload.spec.tsapps/meteor/client/components/message/content/attachments/structure/AttachmentText.tsxapps/meteor/client/components/message/IgnoredContent.tsxapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/client/components/message/MessageHeader.tsxapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/client/components/message/MessageContentBody.tsxapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/client/components/message/toolbar/MessageToolbarItem.tsxapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/client/components/message/variants/SystemMessage.tsxapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/tests/e2e/page-objects/fragments/home-content.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/file-upload.spec.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.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/file-upload.spec.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🧠 Learnings (29)
📓 Common learnings
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR `#36972` through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
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.
📚 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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.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/file-upload.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/file-upload.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/file-upload.spec.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.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/file-upload.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.ts
📚 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/e2e/file-upload.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.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/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/prune-messages.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/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/quote-attachment.spec.ts
📚 Learning: 2025-12-16T17:29:40.430Z
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:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-pruneMessages.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/system-messages.spec.tsapps/meteor/client/components/message/MessageHeader.tsxapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/client/components/message/variants/SystemMessage.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 `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests
Applied to files:
apps/meteor/tests/e2e/system-messages.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts
📚 Learning: 2026-01-19T18:17:46.433Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
Applied to files:
apps/meteor/client/components/message/MessageHeader.tsx
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts
📚 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:
apps/meteor/client/components/message/MessageContentBody.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
apps/meteor/client/components/message/toolbar/MessageToolbarItem.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/threads.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 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/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/prune-messages.spec.ts
📚 Learning: 2026-02-12T15:39:28.416Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 32703
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:52-58
Timestamp: 2026-02-12T15:39:28.416Z
Learning: In `apps/meteor/client/lib/chats/flows/uploadFiles.ts`, when E2E encryption is required but not allowed (e.g., `E2E_Enable_Encrypt_Files` setting is disabled), the function intentionally abandons the entire upload queue and displays a toast error. This fail-fast behavior prevents partial uploads when encryption requirements cannot be met and is the expected behavior, not a bug.
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 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:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
🧬 Code graph analysis (2)
apps/meteor/client/components/message/MessageContentBody.tsx (2)
packages/ui-contexts/src/index.ts (1)
useTranslation(81-81)packages/message-parser/src/definitions.ts (1)
Markup(98-98)
apps/meteor/tests/e2e/channel-management.spec.ts (1)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(7-141)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/tests/e2e/messaging.spec.ts (1)
65-79: Reuse themsg1locator within the step.
You resolve the same locator twice; caching it improves readability and matches the locator-reuse guideline.As per coding guidelines: "Store commonly used locators in variables/constants for reuse".♻️ Suggested refactor
- await poHomeChannel.content - .getMessageByText('msg1') + const msg1 = poHomeChannel.content.getMessageByText('msg1'); + await msg1 .locator('[role=toolbar][aria-label="Message actions"]') .getByRole('button', { name: 'Add reaction' }) .waitFor(); @@ - await expect( - poHomeChannel.content - .getMessageByText('msg1') - .locator('[role=toolbar][aria-label="Message actions"]') - .getByRole('button', { name: 'Add reaction' }), - ).toBeFocused(); + await expect( + msg1.locator('[role=toolbar][aria-label="Message actions"]').getByRole('button', { name: 'Add reaction' }), + ).toBeFocused();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/messaging.spec.ts` around lines 65 - 79, The test repeats the same locator for msg1 twice; store the locator returned by poHomeChannel.content.getMessageByText('msg1') in a local constant inside the test.step and reuse it for the toolbar lookup, waitFor, and expect calls (i.e., replace duplicate calls with a single variable like const msg1 = poHomeChannel.content.getMessageByText('msg1') and use msg1.locator(...).getByRole(...).waitFor() and expect(msg1.locator(...).getByRole(...)).toBeFocused()).apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
76-86:⚠️ Potential issue | 🟡 MinorScope
lastUserMessageAttachmentto the last message.
A page-wide attachment locator can match an older attachment if the latest message has none. Scoping tolastUserMessageavoids cross-message matches.🔧 Suggested scoping
get lastUserMessageAttachment(): Locator { - return this.page.locator('[role="document"][aria-roledescription="message attachment"]').last(); + return this.lastUserMessage.locator('[role="document"][aria-roledescription="message attachment"]').last(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 76 - 86, The page-wide locator lastUserMessageAttachment can match attachments from older messages; update it to be scoped under the last message by using the existing lastUserMessage locator instead of this.page.locator. Replace the body that defines lastUserMessageAttachment to use this.lastUserMessage.locator('[role="document"][aria-roledescription="message attachment"]') (no additional .last() needed) so the attachment lookup is confined to the latest message.
🤖 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/page-objects/fragments/home-content.ts`:
- Around line 284-286: The current getLastThreadMessageByFileName returns the
last list item then looks for a link with the filename, which fails if a newer
non-file message exists; change it to first filter threadMessageListItems to
only those that contain a link with the given filename and then take .last() on
that filtered set. Locate the code around getLastThreadMessageByFileName and
threadMessageListItems and replace the selection order so you filter by the link
role/name (the filename) and then call .last() on the filtered Locator.
---
Duplicate comments:
In `@apps/meteor/tests/e2e/messaging.spec.ts`:
- Around line 65-79: The test repeats the same locator for msg1 twice; store the
locator returned by poHomeChannel.content.getMessageByText('msg1') in a local
constant inside the test.step and reuse it for the toolbar lookup, waitFor, and
expect calls (i.e., replace duplicate calls with a single variable like const
msg1 = poHomeChannel.content.getMessageByText('msg1') and use
msg1.locator(...).getByRole(...).waitFor() and
expect(msg1.locator(...).getByRole(...)).toBeFocused()).
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 76-86: The page-wide locator lastUserMessageAttachment can match
attachments from older messages; update it to be scoped under the last message
by using the existing lastUserMessage locator instead of this.page.locator.
Replace the body that defines lastUserMessageAttachment to use
this.lastUserMessage.locator('[role="document"][aria-roledescription="message
attachment"]') (no additional .last() needed) so the attachment lookup is
confined to the latest message.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/encrypted-room.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/page-objects/encrypted-room.ts
- apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.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: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.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/messaging.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🧠 Learnings (18)
📓 Common learnings
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 RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR `#36972` through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
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:47.551Z
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').
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/Rocket.Chat e2e tests, never prefer data-qa attributes to locate elements. Always prefer semantic locators (getByRole, getByLabel, getByText, getByTitle) and ARIA-based selectors over data-qa locators.
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.
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()`
📚 Learning: 2026-02-24T19:36:47.551Z
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:47.551Z
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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.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/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.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/messaging.spec.ts
📚 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/e2e/messaging.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 `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests
Applied to files:
apps/meteor/tests/e2e/messaging.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 RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
📚 Learning: 2025-12-16T17:29:40.430Z
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:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (2)
apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts (1)
text(325-327)apps/meteor/app/push/server/methods.ts (1)
id(134-144)
🔇 Additional comments (17)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (13)
44-62: Good consolidation of list-level locators.
Centralizing the message/system/thread list accessors will make downstream selectors more consistent and easier to maintain.
64-73: LGTM: message accessors now build on list-item getters.
UsingmessageListItemsfor last/nth access keeps the API consistent.
141-143: Forward hover now uses list-based selector.
This aligns the action with the new message list accessors.
195-197: File description locator update looks good.
Using the message body role keeps it aligned with the new ARIA semantics.
205-209: Nice: filename lookup filters messages before.last().
This makes the helper resilient to newer unrelated messages.
212-218: Attachment locators are now list-item scoped.
This keeps attachment lookups consistent with the message list approach.
264-274: Thread message list accessors look consistent.
The thread list getters and last/first accessors align well with the main list pattern.
280-282: Thread file description selector is aligned with ARIA roles.
293-295: Thread attachment locator update is consistent with new semantics.
432-434: System message body locator updated cleanly.
500-503: System message search now uses ARIA-based selector.
513-515: Message id lookup now uses the new role-based selector.
526-529: Thread reply actions correctly scope to the last message.apps/meteor/tests/e2e/messaging.spec.ts (4)
37-63: Keyboard navigation steps now use page-object locators.
This keeps selector logic centralized and consistent with the new ARIA-based accessors.
82-91: Composer focus step aligns with the new message locators.
125-140: Click-focus tests now rely on message page-object selectors.
146-159: List focus assertions correctly uselastUserMessage.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (2)
80-82:⚠️ Potential issue | 🟡 MinorScope lastUserMessageAttachment to the last message.
The global locator can pick attachments from other messages (e.g., thread panel), causing flaky assertions.🔧 Suggested scoping
get lastUserMessageAttachment(): Locator { - return this.page.locator('[role="document"][aria-roledescription="message attachment"]').last(); + return this.lastUserMessage + .locator('[role="document"][aria-roledescription="message attachment"]') + .last(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 80 - 82, The global locator for lastUserMessageAttachment can match attachments outside the last message; change it to first locate the last message container then scope the attachment lookup inside it. Update the getter lastUserMessageAttachment to call this.page.locator(...) to select the last message element (e.g., the message container selector used elsewhere like the message/article role or test-id) with .last(), then call .locator('[role="document"][aria-roledescription="message attachment"]') on that result so attachments are limited to the last message only.
280-286:⚠️ Potential issue | 🟡 MinorFilter thread messages by filename before taking
.last().
If the newest thread message isn’t a file, the current locator resolves to nothing. Filter first, then select the most recent matching item.🔧 Suggested filtering
getLastThreadMessageByFileName(filename: string): Locator { - return this.threadMessageListItems.last().getByRole('link', { name: filename }); + return this.threadMessageListItems + .filter({ has: this.page.getByRole('link', { name: filename }) }) + .last() + .getByRole('link', { name: filename }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 280 - 286, The current locators call .last() on threadMessageListItems before filtering by filename which can return nothing if the newest message isn't a file; update getLastThreadMessageByFileName to select the matching link inside the list then take .last() (e.g., use threadMessageListItems.getByRole('link', { name: filename }).last()), and change lastThreadMessageFileDescription to filter the threadMessageListItems for the item(s) that contain that filename link and then return the '[role="document"][aria-roledescription="message body"]' locator from the matching item’s .last() (i.e., locate the file link first, then select the last matching parent and then the message body).
🧹 Nitpick comments (2)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (2)
264-266: Align thread attachment “EqualsText” selector with the message-body element.
Using the message-body node keeps assertions focused on the text payload rather than extra metadata.♻️ Suggested alignment
get lastThreadMessageTextAttachmentEqualsText(): Locator { - return this.threadMessageListItems.last().locator('.rcx-attachment__details'); + return this.threadMessageListItems.last().locator('.rcx-attachment__details .rcx-message-body'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 264 - 266, The getter lastThreadMessageTextAttachmentEqualsText currently targets '.rcx-attachment__details' which captures attachment metadata; update it to target the message-body node instead so assertions focus on the text payload—modify the implementation of lastThreadMessageTextAttachmentEqualsText (the getter on this fragment) to return this.threadMessageListItems.last().locator('.message-body') (or the project's message body selector such as '.rcx-message__body') so the locator points at the message body element rather than attachment details.
432-434: Scope lastSystemMessageBody to the system‑message list.
A page‑wide.last()can match system messages outside the main list; scoping tosystemMessageListItemskeeps it deterministic.♻️ Suggested scoping
get lastSystemMessageBody(): Locator { - return this.page.locator('[role=document][aria-roledescription="system message body"]').last(); + return this.systemMessageListItems + .last() + .locator('[role=document][aria-roledescription="system message body"]'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts` around lines 432 - 434, The page-wide locator in the lastSystemMessageBody getter can match messages outside the main list; change it to scope within the system message list selector by using the existing systemMessageListItems locator (e.g., this.systemMessageListItems.locator('[role=document][aria-roledescription="system message body"]').last()) so the returned Locator only searches items inside the main system-message list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 80-82: The global locator for lastUserMessageAttachment can match
attachments outside the last message; change it to first locate the last message
container then scope the attachment lookup inside it. Update the getter
lastUserMessageAttachment to call this.page.locator(...) to select the last
message element (e.g., the message container selector used elsewhere like the
message/article role or test-id) with .last(), then call
.locator('[role="document"][aria-roledescription="message attachment"]') on that
result so attachments are limited to the last message only.
- Around line 280-286: The current locators call .last() on
threadMessageListItems before filtering by filename which can return nothing if
the newest message isn't a file; update getLastThreadMessageByFileName to select
the matching link inside the list then take .last() (e.g., use
threadMessageListItems.getByRole('link', { name: filename }).last()), and change
lastThreadMessageFileDescription to filter the threadMessageListItems for the
item(s) that contain that filename link and then return the
'[role="document"][aria-roledescription="message body"]' locator from the
matching item’s .last() (i.e., locate the file link first, then select the last
matching parent and then the message body).
---
Nitpick comments:
In `@apps/meteor/tests/e2e/page-objects/fragments/home-content.ts`:
- Around line 264-266: The getter lastThreadMessageTextAttachmentEqualsText
currently targets '.rcx-attachment__details' which captures attachment metadata;
update it to target the message-body node instead so assertions focus on the
text payload—modify the implementation of
lastThreadMessageTextAttachmentEqualsText (the getter on this fragment) to
return this.threadMessageListItems.last().locator('.message-body') (or the
project's message body selector such as '.rcx-message__body') so the locator
points at the message body element rather than attachment details.
- Around line 432-434: The page-wide locator in the lastSystemMessageBody getter
can match messages outside the main list; change it to scope within the system
message list selector by using the existing systemMessageListItems locator
(e.g.,
this.systemMessageListItems.locator('[role=document][aria-roledescription="system
message body"]').last()) so the returned Locator only searches items inside the
main system-message list.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/tests/e2e/page-objects/fragments/home-content.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: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/page-objects/fragments/home-content.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.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/page-objects/fragments/home-content.ts
🧠 Learnings (14)
📓 Common learnings
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 RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
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/Rocket.Chat e2e tests, never prefer data-qa attributes to locate elements. Always prefer semantic locators (getByRole, getByLabel, getByText, getByTitle) and ARIA-based selectors over data-qa locators.
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:47.551Z
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').
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR `#36972` through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
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.
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()`
📚 Learning: 2026-02-24T19:36:47.551Z
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:47.551Z
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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/e2e/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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 RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.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/page-objects/fragments/home-content.ts
📚 Learning: 2025-12-16T17:29:40.430Z
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:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🔇 Additional comments (13)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (13)
44-66: Good consolidation of message/thread list locators.
Centralizing list and item locators should make the page object easier to reuse.
68-70: Thread preview locator looks good.
72-78: Nice reuse of message list items for accessors.
84-86: LGTM: non‑sequential message selector.
141-143: Forward message now anchors on the latest message item.
195-197: File description now scoped to the last message body.
205-210: Good filename filtering before selecting the last message.
212-218: Attachment accessors aligned with list-based message items.
268-274: Thread message accessors look good.
293-295: Thread attachment locator is properly scoped.
500-503: System message lookup updated to ARIA selector — LGTM.
513-515: Message‑by‑id selector updated to listitem ARIA — LGTM.
526-529: Reply‑in‑thread flow correctly anchors on the last user message.
dougfabris
requested changes
Feb 26, 2026
juliajforesti
force-pushed
the
chore/message-data-qa
branch
from
2f867e4 to
eda2725
Compare
juliajforesti
force-pushed
the
chore/message-data-qa
branch
from
eda2725 to
c3a08c7
Compare
dougfabris
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
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
3 participants
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.
CORE-1593
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Accessibility Improvements
Internationalization
Tests
UI Cleanup