fix(ux): standardize form validation#39590
Merged
dionisio-bot[bot] merged 22 commits intodevelopfrom Mar 25, 2026
Merged
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:
WalkthroughRemoved explicit react-hook-form Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #39590 +/- ##
===========================================
- Coverage 70.61% 70.59% -0.02%
===========================================
Files 3256 3257 +1
Lines 115778 115789 +11
Branches 21057 21021 -36
===========================================
- Hits 81759 81744 -15
- Misses 31967 31981 +14
- Partials 2052 2064 +12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
juliajforesti
force-pushed
the
fix/form-validation
branch
2 times, most recently
from
ced4662 to
4b2dbb9
Compare
coderabbitai
bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 32 files
Prompt for AI agents (unresolved 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/fragments/export-messages-tab.ts">
<violation number="1" location="apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts:64">
P2: This adds a duplicate locator for the same field. Keep a single getter name in the page object so tests do not split between two accessors for the same element.</violation>
</file>
<file name="apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx:97">
P2: Set `reValidateMode` explicitly here. With `mode: 'onSubmit'`, React Hook Form falls back to `onChange` re-validation, so these async email/phone validators will hit `contacts.checkExistence` on every keystroke after the first failed submit.</violation>
</file>
<file name="apps/meteor/client/views/account/profile/AccountProfilePage.tsx">
<violation number="1" location="apps/meteor/client/views/account/profile/AccountProfilePage.tsx:42">
P2: `mode: 'onSubmit'` makes the async username availability check re-run on every keypress after the first failed submit. Add `reValidateMode: 'onBlur'` if you want submit-only initial validation without turning username edits into per-keystroke API calls.</violation>
</file>
<file name="apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts:56">
P2: This assertion no longer tests the numeric-only restriction, because the same error is already visible from the earlier empty-form submission. Assert that the field value stays empty (or otherwise unchanged) after typing `'a'`.</violation>
</file>
<file name="apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx:68">
P2: Restore the trim-based validation here; with only `required`, whitespace-only names can still be submitted to `onSave`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/account/profile/AccountProfilePage.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
Outdated
Show resolved
Hide resolved
![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: 1
🧹 Nitpick comments (3)
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx (1)
49-52: Remove the no-opfinallyblock and dead comment.This block has no runtime effect and leaves stale implementation residue.
🧹 Suggested cleanup
} catch (e) { const { error } = e as PrioritySaveException; if (error) { setError('name', { message: t(error) }); } - } finally { - // setSaving(false); } };As per coding guidelines,
**/*.{ts,tsx,js}: Avoid code comments in the implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx` around lines 49 - 52, Remove the no-op finally block and the dead comment in PriorityEditForm.tsx: delete the entire finally { // setSaving(false); } block inside the save/update handler (the function that calls setSaving and handles the save flow in PriorityEditForm component) so no empty/ commented finally remains; ensure the surrounding control flow and indentation remain correct after removal.apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx (1)
31-35: Guard Save during submission to avoid duplicate API writes.Because
handleSaveis async, the button should reflectisSubmittingand block repeated submits while the request is in flight.🔒 Suggested update
- formState: { errors, isDirty }, + formState: { errors, isDirty, isSubmitting }, ... - <Button primary mie='none' type='submit' flexGrow={1} disabled={isNew ? false : !isDirty}> + <Button + primary + mie='none' + type='submit' + flexGrow={1} + disabled={isSubmitting || (isNew ? false : !isDirty)} + loading={isSubmitting} + > {t('Save')} </Button>Also applies to: 141-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx` around lines 31 - 35, The form currently destructures formState but doesn't use isSubmitting to prevent duplicate submits; update the useForm destructure to include formState: { errors, isDirty, isSubmitting } and then (1) disable the Save/Submit buttons by passing disabled={isSubmitting || /* existing disabled logic */} where the save buttons are rendered (refer to the Save button components around the submit handlers at lines referenced), and (2) guard the async handler handleSave by returning early if isSubmitting is true (i.e., at the start of handleSave check if (isSubmitting) return;), ensuring duplicate API writes are blocked while the request is in flight.apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts (1)
64-66: Deduplicate the additional-emails locator to avoid selector drift.
inputAdditionalEmailsduplicatestoAdditionalEmailsInput. Reusing the existing getter keeps locator maintenance in one place.♻️ Suggested refactor
get inputAdditionalEmails() { - return this.root.getByRole('textbox', { name: 'To additional emails' }); + return this.toAdditionalEmailsInput; }🤖 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/export-messages-tab.ts` around lines 64 - 66, The getter inputAdditionalEmails duplicates the existing toAdditionalEmailsInput locator; remove the duplicate by having inputAdditionalEmails delegate to or return the existing toAdditionalEmailsInput getter (or delete inputAdditionalEmails and update call sites to use toAdditionalEmailsInput) so the selector is maintained in one place and avoids selector drift.
🤖 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/omnichannel/omnichannel-sla-policies.spec.ts`:
- Around line 69-70: The "only allow numbers" check is a false positive because
the required-error was already triggered; after calling
poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime.pressSequentially('a')
you should assert the field's value did not accept the letter (e.g., expect the
input value to be '' or unchanged) before or in addition to checking
poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.estimatedWaitTimeRequired)
so the test verifies non-numeric rejection; update the spec to read the input's
value via poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime (or
its value/getter) and assert it remains empty (or unchanged) after typing 'a'.
---
Nitpick comments:
In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx`:
- Around line 49-52: Remove the no-op finally block and the dead comment in
PriorityEditForm.tsx: delete the entire finally { // setSaving(false); } block
inside the save/update handler (the function that calls setSaving and handles
the save flow in PriorityEditForm component) so no empty/ commented finally
remains; ensure the surrounding control flow and indentation remain correct
after removal.
In `@apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx`:
- Around line 31-35: The form currently destructures formState but doesn't use
isSubmitting to prevent duplicate submits; update the useForm destructure to
include formState: { errors, isDirty, isSubmitting } and then (1) disable the
Save/Submit buttons by passing disabled={isSubmitting || /* existing disabled
logic */} where the save buttons are rendered (refer to the Save button
components around the submit handlers at lines referenced), and (2) guard the
async handler handleSave by returning early if isSubmitting is true (i.e., at
the start of handleSave check if (isSubmitting) return;), ensuring duplicate API
writes are blocked while the request is in flight.
In `@apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts`:
- Around line 64-66: The getter inputAdditionalEmails duplicates the existing
toAdditionalEmailsInput locator; remove the duplicate by having
inputAdditionalEmails delegate to or return the existing toAdditionalEmailsInput
getter (or delete inputAdditionalEmails and update call sites to use
toAdditionalEmailsInput) so the selector is maintained in one place and avoids
selector drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7477aa6-bde1-411a-b5e7-388a262dcd6e
📒 Files selected for processing (32)
apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsxapps/meteor/client/views/omnichannel/tags/TagEdit.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsxapps/meteor/client/views/omnichannel/units/UnitEdit.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsxapps/meteor/client/views/root/MainLayout/RegisterUsername.tsxapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tspackages/web-ui-registration/src/LoginForm.tsxpackages/web-ui-registration/src/RegisterForm.tsxpackages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsxpackages/web-ui-registration/src/ResetPasswordForm.tsx
💤 Files with no reviewable changes (3)
- apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsx
- apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx
- packages/web-ui-registration/src/LoginForm.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). (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/client/views/root/MainLayout/RegisterUsername.tsxapps/meteor/client/views/omnichannel/tags/TagEdit.tsxpackages/web-ui-registration/src/RegisterForm.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsxapps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/tests/e2e/export-messages.spec.tspackages/web-ui-registration/src/ResetPasswordForm.tsxapps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tspackages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.tsapps/meteor/client/views/omnichannel/units/UnitEdit.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.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/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
🧠 Learnings (41)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/client/views/root/MainLayout/RegisterUsername.tsxpackages/web-ui-registration/src/RegisterForm.tsx
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.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/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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.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/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.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/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-03-11T15:56:18.579Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/client/views/omnichannel/directory/components/ContactField.tsx:40-46
Timestamp: 2026-03-11T15:56:18.579Z
Learning: In `apps/meteor/client/views/omnichannel/directory/components/ContactField.tsx`, `shortName` is intentionally built from the visitor's internal `username` and `phone` (not from `externalIds`). `ContactField` shows the visitor's internal system identifier. Per-channel external identifiers (e.g. WhatsApp usernames from `externalIds[].username`) are only displayed in `ContactInfoChannelsItem`, which serves a different UI context.
Applied to files:
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
📚 Learning: 2025-12-18T15:18:31.688Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:31.688Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, for internal call history items, the item.contactId is guaranteed to always match either the caller.id or callee.id in the call data, so the contact resolution in getContact will never result in undefined.
Applied to files:
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.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 : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.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/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.tsapps/meteor/tests/e2e/export-messages.spec.ts
📚 Learning: 2026-02-24T19:22:52.984Z
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:52.984Z
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/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.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/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.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 `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2025-09-18T17:32:33.969Z
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.
Applied to files:
apps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 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/export-messages.spec.tsapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/export-messages.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts
📚 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/export-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 : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.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/omnichannel/omnichannel-priorities.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/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-units.spec.ts
📚 Learning: 2026-03-11T18:17:53.972Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
📚 Learning: 2026-03-16T22:56:50.405Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:50.405Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
🔇 Additional comments (36)
apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.ts (1)
66-68: Good update to match submit-driven validation UX.Clearing the input and explicitly clicking Save before asserting the required-field error makes this test align with the new on-submit validation behavior and avoids relying on old disabled-button semantics.
apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts (1)
50-55: Good update for submit-driven validation flow.Adding the save click before assertions correctly validates the new
onSubmitbehavior and keeps this test aligned with the UX objective.apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts (1)
116-121: Validation assertions now correctly match submit + revalidate-on-change UX.Good update: the test now validates error appearance after submit and disappearance after correcting the field value, which aligns with the intended form behavior.
As per coding guidelines, this keeps web-first assertions and structured
test.step()usage in Playwright e2e tests.apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts (1)
60-67: LGTM! Test correctly reflects updated form validation behavior.The assertion change from
toBeDisabled()totoBeEnabled()accurately reflects the component update inUnitEdit.tsxwhere the Save button's disabled state isdisabled={_id ? !isDirty : false}. When creating a new unit (_idis falsy), the button is enabled by default, consistent with the PR's objective to shift validation toonSubmitmode.apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts (2)
132-137: LGTM - Correctly triggers initial validation via form submit.The change from Tab to Enter aligns with the
mode: 'onSubmit'form configuration. Since validation now runs on submit rather than blur, pressing Enter triggers the form submission attempt, which invokesvalidateEmailFormatand displays the error for invalid input.
208-212: LGTM - Consistent with the validation mode change.Same rationale as the "Add new contact" test: Enter triggers form submission which runs validation under
onSubmitmode. After this initial submit attempt, subsequent field changes will revalidate automatically, which is why the following test steps can continue using Tab.apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx (1)
41-41: LGTM!The mode change from
onBlurtoonSubmitaligns with the PR objective to standardize form validation. The submit button correctly usesisSubmitting || isValidatingfor the loading state, and validation errors will display after submission attempts.apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx (1)
63-63: LGTM!The mode change to
onSubmitis consistent with the broader PR pattern. Required field validation forparentRoomandnamewill now trigger on form submission, aligning with the standardized validation UX.apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx (1)
114-114: LGTM!The mode change is consistent with the PR's standardization effort. The async
validateChannelNamevalidation will now run on submit, which is the intended behavior. The submit button correctly remains enabled, allowing validation to trigger on submission.apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx (1)
57-57: LGTM!The mode change to
onSubmitis appropriate. The form correctly handles both client-side required validation and server-side error responses (blocked username, field unavailable) through the mutation'sonErrorhandler.packages/web-ui-registration/src/ResetPasswordForm.tsx (1)
26-26: LGTM!The mode change aligns with the PR's standardization of form validation. Email validation (required field and format pattern) will trigger on submission, and the submit button correctly uses
isSubmittingfor the loading state.apps/meteor/client/views/admin/users/AdminUserForm.tsx (1)
123-123: LGTM!The mode change to
onSubmitis consistent with the PR's standardization. The Save button'sdisabled={!isDirty}logic is appropriate—it prevents submitting unchanged forms while allowing validation to run on submission when changes are made.packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx (1)
60-60: LGTM!The mode change to
onSubmitis well-supported by the existing UX design. ThePasswordVerifiercomponent (line 129) provides real-time visual feedback on password complexity requirements, compensating for the delayed error display inherent inonSubmitmode. The password confirmation field'sdisabledstate continues to work correctly sincepasswordIsValidis evaluated reactively viawatch.apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx (2)
92-99: LGTM!The form state extraction now includes
isDirty, and the mode change toonSubmitaligns with the PR's standardization effort. The async validation for email format and phone/email uniqueness will trigger on submission.
327-327: LGTM!The conditional disabled logic is well-designed:
- Edit mode (
contactDataexists): Button disabled until changes are made (!isDirty)- Create mode (no
contactData): Button always enabled, validation triggers on submitThis provides appropriate UX for both scenarios.
packages/web-ui-registration/src/RegisterForm.tsx (1)
72-72: Validation trigger change is consistent and safe.Switching to submit-first validation here aligns with the UX standardization objective while keeping current error handling intact.
apps/meteor/client/views/omnichannel/tags/TagEdit.tsx (2)
47-47: Mode update matches the form validation standardization.This is consistent with submit-first validation behavior across the PR.
133-133: Save enablement logic is clear for create vs edit flows.Keeping Save enabled for new tags and requiring dirty state for edits is a good UX fit with submit-time validation.
apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts (1)
51-51: Test assertion correctly reflects the new default Save state.Good alignment between e2e expectation and updated create-tag form behavior.
apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx (1)
109-109: Submit-first validation change is appropriate here.The update is consistent with the PR-wide validation timing shift.
apps/meteor/client/views/omnichannel/units/UnitEdit.tsx (2)
85-85: Validation mode update is aligned with the new UX contract.
266-266: Create/edit Save-state logic is consistent and readable.apps/meteor/client/views/account/profile/AccountProfilePage.tsx (1)
42-42: Good consistency update for profile form validation timing.apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx (2)
71-72: Form-state update and submit-mode switch look correct.This keeps the department form aligned with the standardized validation behavior.
141-141: Save button gating now matches create/edit intent cleanly.apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx (2)
69-72: Form initialization change is consistent with the new validation model.
227-227: Save behavior for existing vs new custom fields is correctly expressed.apps/meteor/tests/e2e/export-messages.spec.ts (1)
76-101: Good update for submit-driven, field-level validation coverage.This test now validates both recipient fields and error multiplicity after submit, which is consistent with the new UX contract.
apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx (1)
93-95: Validation mode and save gating look consistent with the new UX direction.
onSubmitplus dirty-only disablement for existing entities is coherent and avoids blocking initial submit for new records.Also applies to: 210-210
apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.ts (1)
17-17: Nice selector upgrade to an accessibility-first locator.Using
getByRole('spinbutton', { name: ... })is clearer and more resilient than attribute-based targeting.apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts (1)
53-57: Good shift to submit-triggered required-field validation.This step matches the new form behavior and explicitly validates both required errors at submit time.
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx (5)
64-72: Good submit-first validation setup.Switching to
mode: 'onSubmit'and includingisSubmittedinformStatecleanly supports the intended validation UX change.
149-151:messagesCountrevalidation timing looks correct.Gating
shouldValidatewithisSubmittedavoids early validation noise while still revalidating after first submit.
302-312: Recipient requirement is consistently enforced across both fields.Using
validateRecipienton bothtoUsersandadditionalEmailskeeps the cross-field rule symmetric and predictable.Also applies to: 357-363
313-335: Controller wiring and error accessibility are well-structured.Explicit field bindings plus
aria-invalid/aria-describedbyandFieldErrorrendering are coherent and improve form error UX.Also applies to: 366-389
430-430: Submit button behavior aligns with on-submit validation.Removing the dirty-state gate from submit is consistent with the new validation flow.
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved 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/omnichannel/omnichannel-sla-policies.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts:56">
P2: This assertion is a no-op for the empty-field case and no longer verifies the estimated wait time validation. The test can now pass even if submitting an empty SLA stops showing the required-field error for that input.</violation>
</file>
<file name="apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx:68">
P1: The whitespace check is attached to the translation call instead of the form rules, so names containing only spaces will bypass validation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
Outdated
Show resolved
Hide resolved
coderabbitai
bot
added
type: bug
and removed
type: feature
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts (1)
53-57: Consider asserting the estimated wait time error for completeness.The step name indicates validation of both required fields, but only the name error is asserted. For consistency with the step description and to verify that the estimated wait time validation also triggers on submit, add an assertion for the estimated wait time error.
✨ Suggested assertion addition
await test.step('should not submit form with empty required fields: name and estimated wait time', async () => { await poOmnichannelSlaPolicies.manageSlaPolicy.btnSave.click(); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.nameRequired)).toBeVisible(); + await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.estimatedWaitTimeRequired)).toBeVisible(); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime).toHaveValue(''); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts` around lines 53 - 57, The test step verifies name validation but omits asserting the estimated wait time validation; update the step in the test (inside test.step) to also assert the estimated wait time error is shown by calling poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(...) for the estimated wait time error (e.g. ERROR.estimatedWaitTimeRequired) and expect it toBeVisible(), keeping the existing checks for inputEstimatedWaitTime value and name error intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts`:
- Around line 53-57: The test step verifies name validation but omits asserting
the estimated wait time validation; update the step in the test (inside
test.step) to also assert the estimated wait time error is shown by calling
poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(...) for the estimated
wait time error (e.g. ERROR.estimatedWaitTimeRequired) and expect it
toBeVisible(), keeping the existing checks for inputEstimatedWaitTime value and
name error intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9ad9509-9757-4b8d-a617-0374b13a0e1d
📒 Files selected for processing (5)
apps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
- apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
- apps/meteor/client/views/account/profile/AccountProfilePage.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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 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/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.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/export-messages-tab.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/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-sla-policies.spec.ts
🧠 Learnings (23)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.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/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.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/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2026-02-24T19:22:52.984Z
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:52.984Z
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/export-messages-tab.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/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.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/export-messages-tab.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-sla-policies.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 `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-sla-policies.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/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
🔇 Additional comments (3)
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts (2)
52-54: Good locator consolidation insetAdditionalEmail.Line 53 correctly routes input through the page-object getter, which improves maintainability and keeps selector usage centralized.
60-66: Nice addition of reusable input getters.Lines 60-66 cleanly expose field locators for per-field validation flows and keep the Page Object Model consistent. As per coding guidelines, “Store commonly used locators in variables/constants for reuse” and “Follow Page Object Model pattern consistently in Playwright tests.”
apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts (1)
68-71:only allow numbersstep is currently a false-positive.At Line 70, the required-error assertion is already true from the earlier empty-submit state, so this does not prove non-numeric input rejection. Add a value assertion after typing
ato validate the numeric constraint itself.✅ Suggested assertion hardening
await test.step('only allow numbers on estimated wait time field', async () => { await poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime.pressSequentially('a'); + await expect(poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime).toHaveValue(''); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.estimatedWaitTimeRequired)).toBeVisible(); });
juliajforesti
force-pushed
the
fix/form-validation
branch
from
c37e468 to
e97e7f4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx (1)
40-48:⚠️ Potential issue | 🟠 MajorHandle unknown save errors instead of silently swallowing them.
Line 44 assumes every thrown value matches
PrioritySaveException; whenonSaverejects with a regularError, no field error is set and the failure is hidden from users.Suggested fix
- const handleSave = async ({ name }: { name: string }) => { + const handleSave = async ({ name }: { name: string }) => { try { await onSave({ name, reset: name === defaultName }); - } catch (e) { - const { error } = e as PrioritySaveException; - - if (error) { - setError('name', { message: t(error) }); - } + } catch (e: unknown) { + const translationKey = (e as Partial<PrioritySaveException>)?.error; + const message = translationKey ? t(translationKey) : e instanceof Error ? e.message : t('Error'); + setError('name', { message }); } finally { - // setSaving(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx` around lines 40 - 48, The catch in handleSave currently assumes the thrown value is a PrioritySaveException and swallows other errors; update handleSave to detect non-PrioritySaveException rejections and surface them (e.g., set a generic form-level error or rethrow) so failures aren’t hidden: in handleSave, after casting e as PrioritySaveException and handling error via setError('name', { message: t(error) }), add a branch that handles unknown errors (Error or unknown) by calling setError with a generic message (or calling a provided onError handler) or rethrowing the error so it bubbles up; reference handleSave, onSave, PrioritySaveException, setError and t when locating the change.
🤖 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/views/omnichannel/priorities/PriorityEditForm.tsx`:
- Around line 49-51: In the PriorityEditForm component remove the dead commented
code in the empty finally block: delete the commented-out "setSaving(false);"
line and remove the now-empty finally { } block so there is no empty finally in
the async operation; locate the finally that contains "// setSaving(false);"
within PriorityEditForm.tsx and delete it (leaving surrounding try/catch logic
intact).
---
Outside diff comments:
In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx`:
- Around line 40-48: The catch in handleSave currently assumes the thrown value
is a PrioritySaveException and swallows other errors; update handleSave to
detect non-PrioritySaveException rejections and surface them (e.g., set a
generic form-level error or rethrow) so failures aren’t hidden: in handleSave,
after casting e as PrioritySaveException and handling error via setError('name',
{ message: t(error) }), add a branch that handles unknown errors (Error or
unknown) by calling setError with a generic message (or calling a provided
onError handler) or rethrowing the error so it bubbles up; reference handleSave,
onSave, PrioritySaveException, setError and t when locating the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f904cc7-7b5b-4bbe-ba0a-78da4d28155b
📒 Files selected for processing (5)
apps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
- apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
- apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsx
🧠 Learnings (7)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2025-09-18T17:32:33.969Z
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.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
🔇 Additional comments (2)
apps/meteor/client/views/account/profile/AccountProfilePage.tsx (1)
40-44: VerifyreValidateMode: 'onBlur'aligns with PR objective.The PR objective states "validation on form submit and revalidating fields on change," which would suggest
reValidateMode: 'onChange'(the default). This form explicitly setsreValidateMode: 'onBlur'instead.This may be intentional given the async
validateUsernameinAccountProfileFormto avoid excessive API calls on every keystroke. If so, this is a sensible deviation. Please confirm this is the intended behavior for this form.apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx (1)
67-93: Validation and ARIA linkage look solid here.Required + trim validation and the
aria-describedby/FieldErrorid pairing are correctly wired for inline feedback.
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
Outdated
Show resolved
Hide resolved
juliajforesti
force-pushed
the
fix/form-validation
branch
from
e97e7f4 to
e251b8e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx (1)
81-91: Consider makingaria-describedbyconditional.The
aria-describedbyon line 81 always references${nameFieldId}-error, but the FieldError element is only rendered when an error exists. This meansaria-describedbypoints to a non-existent element when validation passes.♻️ Optional fix to conditionally set aria-describedby
- aria-describedby={`${nameFieldId}-error`} + aria-describedby={errors.name ? `${nameFieldId}-error` : undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx` around lines 81 - 91, The aria-describedby currently always references `${nameFieldId}-error` even when no FieldError is rendered; update the input in PriorityEditForm (the element using nameFieldId and aria-describedby) to set aria-describedby only when errors.name exists (e.g., use a conditional that yields `${nameFieldId}-error` when errors.name is truthy and undefined/null otherwise) so the input only points to the FieldError element when it is actually rendered.
🤖 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/views/omnichannel/contactInfo/EditContactInfo.tsx`:
- Around line 97-99: In EditContactInfo.tsx update the react-hook-form
configuration used when initializing the form (the object containing mode,
reValidateMode, defaultValues) to use reValidateMode: 'onChange' instead of
'onBlur' so fields revalidate as the user types; locate the form setup where
defaultValues: initialValue is passed and change only the reValidateMode value
to 'onChange'.
---
Nitpick comments:
In `@apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx`:
- Around line 81-91: The aria-describedby currently always references
`${nameFieldId}-error` even when no FieldError is rendered; update the input in
PriorityEditForm (the element using nameFieldId and aria-describedby) to set
aria-describedby only when errors.name exists (e.g., use a conditional that
yields `${nameFieldId}-error` when errors.name is truthy and undefined/null
otherwise) so the input only points to the FieldError element when it is
actually rendered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: acc83494-59cf-4e62-929a-d0b5c307c812
📒 Files selected for processing (5)
apps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.tsapps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts
- apps/meteor/client/views/account/profile/AccountProfilePage.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). (3)
- GitHub Check: 📦 Build Packages
- 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/export-messages-tab.tsapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
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/export-messages-tab.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/export-messages-tab.ts
🧠 Learnings (24)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.tsapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.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/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.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/export-messages-tab.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/export-messages-tab.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/export-messages-tab.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/export-messages-tab.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/export-messages-tab.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/export-messages-tab.ts
📚 Learning: 2026-02-24T19:22:52.984Z
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:52.984Z
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/export-messages-tab.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.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/export-messages-tab.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/export-messages-tab.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/export-messages-tab.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts
📚 Learning: 2025-09-18T17:32:33.969Z
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.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.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 **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-11T15:56:18.579Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/client/views/omnichannel/directory/components/ContactField.tsx:40-46
Timestamp: 2026-03-11T15:56:18.579Z
Learning: In `apps/meteor/client/views/omnichannel/directory/components/ContactField.tsx`, `shortName` is intentionally built from the visitor's internal `username` and `phone` (not from `externalIds`). `ContactField` shows the visitor's internal system identifier. Per-channel external identifiers (e.g. WhatsApp usernames from `externalIds[].username`) are only displayed in `ContactInfoChannelsItem`, which serves a different UI context.
Applied to files:
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
📚 Learning: 2025-12-18T15:18:31.688Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:31.688Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, for internal call history items, the item.contactId is guaranteed to always match either the caller.id or callee.id in the call data, so the contact resolution in getContact will never result in undefined.
Applied to files:
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
🔇 Additional comments (7)
apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts (2)
52-54: Good encapsulation insetAdditionalEmail.Using
this.inputAdditionalEmailshere keeps interactions centralized in the page object and avoids duplicated locator logic.
60-66: Nice addition of reusable field locators.These getters make field-level validation assertions cleaner and keep selector ownership in the page object.
As per coding guidelines, "Store commonly used locators in variables/constants for reuse" and "Follow Page Object Model pattern consistently in Playwright tests."
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx (1)
92-93: Good edit-mode Save gating withisDirty.This avoids no-op updates for existing contacts while keeping the create flow behavior unchanged.
Also applies to: 328-328
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx (4)
27-38: LGTM!The form setup with
mode: 'onSubmit'aligns with the PR objective to standardize validation on submit. UsinguseId()for generating the field ID is the correct approach for accessibility.
40-50: LGTM!Clean implementation of the save handler with proper error handling. The dead commented code issue from previous review has been addressed.
65-84: LGTM!The validation rules complement each other well:
requiredcatches empty values whilevalidatecatches whitespace-only strings. ARIA attributes are properly wired up for accessibility.
93-102: LGTM!The footer implementation correctly uses
ContextualbarFooter, and button states are properly tied to form state (isDirtyfor Save,isSubmittingfor both buttons).
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
Outdated
Show resolved
Hide resolved
dougfabris
requested changes
Mar 19, 2026
apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
Outdated
Show resolved
Hide resolved
coderabbitai
bot
added
type: feature
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx (1)
316-319:⚠️ Potential issue | 🟡 MinorReplace clearErrors with revalidation using trigger.
Lines 316-319 and 369-372 unconditionally clear errors on the counterpart field when either
toUsersoradditionalEmailschanges. This hides real validation errors (e.g., invalid email inadditionalEmails) until the next submit.Both fields have interdependent
validateRecipientrules. If a user enters an invalid email and then changestoUsers, the email error gets cleared even though the field remains invalid. Instead, revalidate the counterpart field only after form submission.Suggested fix
const { formState, control, watch, register, handleSubmit, - clearErrors, + trigger, reset, } = useForm<ExportMessagesFormValues>({ @@ onChange={(value) => { onChange(value); - clearErrors('additionalEmails'); + if (formState.isSubmitted) { + void trigger('additionalEmails'); + } }} @@ onChange={(e) => { onChange(e); - clearErrors('toUsers'); + if (formState.isSubmitted) { + void trigger('toUsers'); + } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx` around lines 316 - 319, Replace the unconditional clearErrors calls in the onChange handlers with form revalidation by calling trigger for the counterpart field (e.g., in the onChange for toUsers call trigger('additionalEmails') instead of clearErrors('additionalEmails'), and vice versa for additionalEmails -> trigger('toUsers')); locate the onChange handlers in ExportMessages.tsx (the ones that currently call clearErrors) and invoke the react-hook-form trigger function (await or then-able as appropriate) to re-run the validateRecipient rule on the counterpart field rather than wiping its errors.
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx (1)
301-311: Extract shared recipient predicate to avoid validator drift.
validateRecipientlogic is duplicated in both fields. A shared helper keeps both validators aligned as rules evolve.♻️ Suggested refactor
+ const hasRecipient = (users: string[] | undefined, emails: string): boolean => users?.length > 0 || emails.trim() !== ''; @@ rules={{ validate: { validateRecipient: (toUsers) => { const additionalEmails = watch('additionalEmails'); - if (toUsers?.length > 0 || additionalEmails !== '') { + if (hasRecipient(toUsers, additionalEmails)) { return undefined; } return t('Mail_Message_Missing_to'); }, }, @@ validateRecipient: (additionalEmails) => { - if (additionalEmails !== '' || toUsers?.length > 0) { + if (hasRecipient(toUsers, additionalEmails)) { return undefined; } return t('Mail_Message_Missing_to'); },Also applies to: 356-363
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx` around lines 301 - 311, Extract the duplicated recipient-check logic into a shared predicate (e.g., isRecipientPresent) and use it in both validator rules instead of duplicating the logic; specifically, create a helper that accepts the current toUsers value and the watched additionalEmails (use the same watch('additionalEmails') source) and returns true/false, then update validateRecipient (and the other validator at the second occurrence) to call that helper and return undefined on true or t('Mail_Message_Missing_to') on false so both validators remain in sync as rules evolve.apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts (1)
10-16: Drop the inline hook docblock.This repo avoids implementation comments, and the exported name plus signature already make the behavior clear.
As per coding guidelines "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts` around lines 10 - 16, Remove the inline docblock above the exported hook useFormSubmitWithDirtyCheck: delete the multi-line comment that describes behavior and patterns, leaving only the implementation and export; ensure no other implementation comments are introduced and that the hook name/signature remains as-is to convey its purpose.
🤖 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/views/omnichannel/priorities/PriorityEditForm.tsx`:
- Around line 73-87: PriorityEditForm is passing aria-describedby and
aria-invalid into StringSettingInput but StringSettingInput (in
apps/meteor/.../StringSettingInput.tsx) does not forward these props to its
inner <input>, so the FieldError is not programmatically linked; fix by updating
StringSettingInput to accept and forward arbitrary input props (including
aria-describedby and aria-invalid) to its inner TextInput/input, or
alternatively replace the StringSettingInput usage in PriorityEditForm with a
plain TextInput and pass nameFieldId-derived aria-describedby and aria-invalid
directly; ensure the FieldError component remains referenced by the same
aria-describedby id (nameFieldId + '-error') so assistive tech can link the
error to the field.
In `@apps/meteor/client/views/omnichannel/tags/TagEdit.tsx`:
- Around line 54-75: The handler registered with useFormSubmitWithDirtyCheck
(handleSave) currently calls onClose() in the finally block, which closes the
contextual bar even when saveTag fails; change the control flow so onClose() is
invoked only after a successful await saveTag(...) (i.e., move the onClose()
call into the try block after dispatchToastMessage success and
queryClient.invalidateQueries), and remove it from the catch/finally so that on
error the error toast is shown but the form remains open for correction.
In `@apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx`:
- Around line 124-130: The submit handler passed into
useFormSubmitWithDirtyCheck (handleSave) calls saveTriggerMutation.mutateAsync
without awaiting it, causing isSubmitting to clear prematurely; update the
handler in EditTrigger.tsx so the async function awaits and returns
saveTriggerMutation.mutateAsync({...}) (for example: await
saveTriggerMutation.mutateAsync(...); or return the promise) so the form
submission lifecycle (isSubmitting) reflects the actual network write and
prevents duplicate submits.
---
Outside diff comments:
In
`@apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx`:
- Around line 316-319: Replace the unconditional clearErrors calls in the
onChange handlers with form revalidation by calling trigger for the counterpart
field (e.g., in the onChange for toUsers call trigger('additionalEmails')
instead of clearErrors('additionalEmails'), and vice versa for additionalEmails
-> trigger('toUsers')); locate the onChange handlers in ExportMessages.tsx (the
ones that currently call clearErrors) and invoke the react-hook-form trigger
function (await or then-able as appropriate) to re-run the validateRecipient
rule on the counterpart field rather than wiping its errors.
---
Nitpick comments:
In `@apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts`:
- Around line 10-16: Remove the inline docblock above the exported hook
useFormSubmitWithDirtyCheck: delete the multi-line comment that describes
behavior and patterns, leaving only the implementation and export; ensure no
other implementation comments are introduced and that the hook name/signature
remains as-is to convey its purpose.
In
`@apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx`:
- Around line 301-311: Extract the duplicated recipient-check logic into a
shared predicate (e.g., isRecipientPresent) and use it in both validator rules
instead of duplicating the logic; specifically, create a helper that accepts the
current toUsers value and the watched additionalEmails (use the same
watch('additionalEmails') source) and returns true/false, then update
validateRecipient (and the other validator at the second occurrence) to call
that helper and return undefined on true or t('Mail_Message_Missing_to') on
false so both validators remain in sync as rules evolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d44ba464-2495-4ef1-a9c1-228cfdaeb66c
📒 Files selected for processing (22)
apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsxapps/meteor/client/hooks/useFormSubmitWithDirtyCheck.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsxapps/meteor/client/views/omnichannel/tags/TagEdit.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsxapps/meteor/client/views/omnichannel/units/UnitEdit.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsxapps/meteor/client/views/root/MainLayout/RegisterUsername.tsxpackages/i18n/src/locales/en.i18n.jsonpackages/web-ui-registration/src/LoginForm.tsxpackages/web-ui-registration/src/RegisterForm.tsxpackages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsxpackages/web-ui-registration/src/ResetPasswordForm.tsx
💤 Files with no reviewable changes (3)
- apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx
- apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx
- apps/meteor/client/views/admin/users/AdminUserForm.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/src/locales/en.i18n.json
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx
- apps/meteor/client/views/account/profile/AccountProfilePage.tsx
- packages/web-ui-registration/src/RegisterForm.tsx
- apps/meteor/client/views/omnichannel/units/UnitEdit.tsx
- apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx
- packages/web-ui-registration/src/ResetPasswordForm.tsx
- packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx
- apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx
- packages/web-ui-registration/src/LoginForm.tsx
- apps/meteor/client/views/omnichannel/departments/EditDepartment.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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/omnichannel/tags/TagEdit.tsxapps/meteor/client/hooks/useFormSubmitWithDirtyCheck.tsapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx
🧠 Learnings (16)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39590
File: apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx:97-99
Timestamp: 2026-03-18T16:08:17.800Z
Learning: In `apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx`, `reValidateMode: 'onBlur'` is intentionally used (not 'onChange') because the `validateEmailFormat` and `validatePhone` functions are async and call the `checkExistenceEndpoint` API to check for duplicates. Using 'onChange' would trigger excessive network requests on every keystroke. The combination of `mode: 'onSubmit'` with `reValidateMode: 'onBlur'` is a deliberate design decision to minimize API calls while still providing revalidation feedback.
📚 Learning: 2026-03-18T16:08:17.800Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39590
File: apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx:97-99
Timestamp: 2026-03-18T16:08:17.800Z
Learning: In `apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx`, `reValidateMode: 'onBlur'` is intentionally used (not 'onChange') because the `validateEmailFormat` and `validatePhone` functions are async and call the `checkExistenceEndpoint` API to check for duplicates. Using 'onChange' would trigger excessive network requests on every keystroke. The combination of `mode: 'onSubmit'` with `reValidateMode: 'onBlur'` is a deliberate design decision to minimize API calls while still providing revalidation feedback.
Applied to files:
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/omnichannel/tags/TagEdit.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsxapps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts
📚 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/views/omnichannel/customFields/EditCustomFields.tsx
📚 Learning: 2026-03-11T18:17:53.972Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsxapps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 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/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx
📚 Learning: 2025-09-18T17:32:33.969Z
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.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
📚 Learning: 2026-03-18T22:07:19.687Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/departments.ts:59-70
Timestamp: 2026-03-18T22:07:19.687Z
Learning: In `apps/meteor/app/apps/server/converters/departments.ts`, the `convertAppDepartment` method uses non-null assertions (`!`) on `department.name`, `department.email`, and `department.offlineMessageChannelName` when constructing `newDepartment: ILivechatDepartment`. These fields are optional on `IAppsDepartment`, but the App framework flow guarantees their presence at the call site. Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.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 **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
🔇 Additional comments (4)
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx (1)
37-41: LGTM!Removing
mode: 'onBlur'correctly adopts react-hook-form's defaults (mode: 'onSubmit',reValidateMode: 'onChange'), which aligns with the PR objective to standardize validation behavior. Since this form only has synchronous validation (required check and max users count), using the defaultreValidateMode: 'onChange'is appropriate and won't cause performance issues.apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx (1)
148-150: Good submit-gated revalidation formessagesCount.Using
isSubmittedto gateshouldValidatekeeps initial UX quiet while still revalidating after first submit.apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx (1)
69-71: Good submit-path consolidation here.Keeping Save enabled and short-circuiting unchanged edits inside
handleSavematches the new validation UX without duplicating the dirty-state check in the footer.Also applies to: 80-101, 229-230
apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx (1)
62-64: Nice accessibility cleanup on the inline errors.The stable ids and matching
htmlFor/aria-describedby/FieldErrorids make the new submit-triggered validation flow much easier to follow with assistive tech.Also applies to: 87-145
apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx
Outdated
Show resolved
Hide resolved
juliajforesti
force-pushed
the
fix/form-validation
branch
3 times, most recently
from
07179cc to
a22ec21
Compare
juliajforesti
force-pushed
the
fix/form-validation
branch
from
4671f21 to
4907bf1
Compare
juliajforesti
added
the
stat: QA assured
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.
WA-83
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit