feat: add custom-sounds.getOne endpoint, adapt client to consume it and add Contextualbar bug fixes#38610
Merged
kodiakhq[bot] merged 33 commits intodevelopfrom Feb 23, 2026
Merged
feat: add custom-sounds.getOne endpoint, adapt client to consume it and add Contextualbar bug fixes#38610kodiakhq[bot] merged 33 commits intodevelopfrom
kodiakhq[bot] merged 33 commits intodevelopfrom
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 2f565c1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
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:
WalkthroughAdds a new REST endpoint Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Server as API Server
participant DB as Database
Client->>Server: GET /v1/custom-sounds.getOne { _id }
Server->>Server: validate payload (isCustomSoundsGetOneProps)
alt invalid
Server-->>Client: 400 Bad Request
else valid
Server->>DB: findOne customSounds by _id
alt found
DB-->>Server: sound { _id, name, ... }
Server-->>Client: 200 { success: true, sound: { ... } }
else not found
DB-->>Server: null
Server-->>Client: 404 Not Found
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38610 +/- ##
===========================================
- Coverage 70.63% 70.63% -0.01%
===========================================
Files 3189 3189
Lines 112717 112717
Branches 20390 20440 +50
===========================================
- Hits 79621 79619 -2
- Misses 31048 31049 +1
- Partials 2048 2049 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/12 22:57", "02/13 22:38", "02/16 14:04", "02/18 23:15", "02/19 23:23", "02/20 15:44", "02/20 16:45 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Fri, 20 Feb 2026 16:45:18 GMT |
KevLehman
requested changes
Feb 11, 2026
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@coderabbitai review. |
KevLehman
reviewed
Feb 12, 2026
Co-authored-by: Kevin Aleman <[email protected]>
Co-authored-by: Kevin Aleman <[email protected]>
Contributor
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/app/api/server/v1/custom-sounds.ts`:
- Around line 82-89: The OpenAPI/JSON Schema object in
apps/meteor/app/api/server/v1/custom-sounds.ts has the "required" array
incorrectly nested inside "properties" (alongside the "sounds" property and its
$ref to ICustomSound), so AJV treats it as a property instead of enforcing
required fields; fix this by moving the required:
['count','offset','total','sounds','success'] out of the "properties" object and
make it a sibling of "properties" within the same schema definition (the schema
block that defines "sounds" / references '#/components/schemas/ICustomSound').
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/custom-sounds.ts (1)
95-104: Redundant cast:nameis already typed asstringvia query params.Line 103 casts
name as string, butnameis alreadystring | undefinedfrom the validatedCustomSoundsListtype. Thename ?guard ensures it'sstringwithin the ternary's truthy branch.Proposed fix
- ...(name ? { name: { $regex: escapeRegExp(name as string), $options: 'i' } } : {}), + ...(name ? { name: { $regex: escapeRegExp(name), $options: 'i' } } : {}),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/twenty-colts-flash.mdapps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/tests/end-to-end/api/custom-sounds.tspackages/rest-typings/src/index.tspackages/rest-typings/src/v1/customSounds.ts
💤 Files with no reviewable changes (1)
- apps/meteor/tests/end-to-end/api/custom-sounds.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/rest-typings/src/v1/customSounds.tsapps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsxpackages/rest-typings/src/index.ts
🧠 Learnings (1)
📚 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/admin/customSounds/EditCustomSound.tsx
🧬 Code graph analysis (4)
packages/rest-typings/src/v1/customSounds.ts (1)
apps/meteor/app/api/server/v1/custom-sounds.ts (1)
CustomSoundEndpoints(168-168)
apps/meteor/app/api/server/v1/custom-sounds.ts (2)
packages/models/src/index.ts (1)
CustomSounds(144-144)packages/rest-typings/src/v1/customSounds.ts (1)
isCustomSoundsGetOneProps(18-18)
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(34-34)
packages/rest-typings/src/index.ts (2)
apps/meteor/app/api/server/v1/custom-sounds.ts (1)
CustomSoundEndpoints(168-168)packages/rest-typings/src/v1/customSounds.ts (1)
CustomSoundEndpoints(20-24)
⏰ 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). (7)
- GitHub Check: 📦 Meteor Build (coverage)
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
.changeset/twenty-colts-flash.md (1)
1-6: LGTM!Changeset accurately describes the new endpoint and affected packages.
packages/rest-typings/src/index.ts (1)
15-15: LGTM!The import, interface extension, and re-export of
CustomSoundEndpointsfollow the established pattern used by other endpoint typings in this file.Also applies to: 58-58, 215-215
packages/rest-typings/src/v1/customSounds.ts (1)
20-24: Consider whether the response type is intentionally minimal.The endpoint returns only
{ _id, name }— if consumers (like the edit form) need additionalICustomSoundfields, this type would need to be expanded to match the actual server response. This ties into the concern raised onEditCustomSound.tsx.apps/meteor/app/api/server/v1/custom-sounds.ts (1)
121-166: ThegetOneendpoint looks well-structured.Proper 404 handling, input validation via
isCustomSoundsGetOneProps, and a clean response shape. However, as noted in other files, returning only_idandnamemay be insufficient for theEditSoundconsumer — please verify.apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
15-23: No issue:EditSoundis correctly designed to work with the API response.The component's
dataprop type definesextensionas optional (extension?: string), and the UI only displays thenamefield and a sound file upload. The component gracefully handles cases whereextensionis absent and doesn't break without it.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
KevLehman
reviewed
Feb 12, 2026
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/meteor/app/api/server/v1/custom-sounds.ts`:
- Around line 95-104: The handler action() currently accepts an arbitrary
"query" param but the endpoint only uses "name" for filtering; either remove
"query" from the endpoint schema (so only name/offset/count/sort are allowed) or
explicitly restrict allowed query fields by defining a queryFields list and
wiring it into parseJsonQuery()/schema parsing so only those fields are
accepted; update the schema declaration that currently includes "query" (remove
it) or add a queryFields array (e.g., ['name', 'createdAt', ...]) and pass that
to parseJsonQuery/validation so the constructed filter (used in action() as
filter = {...query, ...(name ? {...} : {})}) cannot contain arbitrary
operators/fields.
- Around line 121-166: The response type for the 'custom-sounds.getOne' route is
missing the success field; update the ajv.compile generic type (currently { _id:
ICustomSound['_id']; name: ICustomSound['name'] }) to include success: boolean
so it becomes { _id: ICustomSound['_id']; name: ICustomSound['name']; success:
boolean } to match the JSON schema and the object returned by the action in
custom-sounds.getOne (the API.v1.success call that returns _id, name, and
success).
In `@apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx`:
- Around line 30-32: The empty-state message in EditCustomSound.tsx uses the
wrong translation key; replace the call to t('Call_info_could_not_be_loaded')
inside the ContextualbarEmptyContent return with a sound-appropriate key (e.g.,
t('Custom_Sound_Not_Found')) and add that key to the i18n translation files;
update the component reference (ContextualbarEmptyContent) accordingly so the
message reads correctly for custom sounds.
In `@apps/meteor/tests/end-to-end/api/custom-sounds.ts`:
- Line 212: Fix the typo in the inline comment that reads "// ensures there are
no extra properites (because of additionalProperties: false)" by changing
"properites" to "properties" so the comment reads "// ensures there are no extra
properties (because of additionalProperties: false)"; locate and update this
comment in apps/meteor/tests/end-to-end/api/custom-sounds.ts (the comment shown
in the diff) to correct the spelling.
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
66-78: Test cleanup only deletesfileId, leavingfileId2behind.The
beforehook creates two sounds (fileIdandfileId2), but theafterhook only deletesfileId. This leaks test data. While pre-existing, it's now more relevant sincefileId2is actively used in the name-filter test (line 126).Proposed fix
after(() => - request + Promise.all([request .post(api('method.call/deleteCustomSound')) .set(credentials) .send({ message: JSON.stringify({ msg: 'method', id: '33', method: 'deleteCustomSound', params: [fileId], }), - }), + }), + request + .post(api('method.call/deleteCustomSound')) + .set(credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: '34', + method: 'deleteCustomSound', + params: [fileId2], + }), + })]), );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/tests/end-to-end/api/custom-sounds.ts
🧰 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/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
🧠 Learnings (11)
📚 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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
🧬 Code graph analysis (2)
apps/meteor/app/api/server/v1/custom-sounds.ts (2)
packages/models/src/index.ts (1)
CustomSounds(144-144)packages/rest-typings/src/v1/customSounds.ts (1)
isCustomSoundsGetOneProps(18-18)
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (2)
packages/ui-contexts/src/index.ts (2)
useEndpoint(34-34)useTranslation(81-81)packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
⏰ 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 comments (2)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
188-240: Good coverage of the newgetOneendpoint.The test suite covers authentication, input validation, not-found handling, successful retrieval with shape assertions, and multiple regex injection vectors. Well structured.
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
16-24: Implementation looks clean with proper guard on the query.The
enabled: !!_idguard on line 23 correctly prevents the query from firing when_idis undefined, making the non-null assertion on line 22 safe. The switch from list-based fetching to the dedicatedgetOneendpoint is a good improvement.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
66-78:⚠️ Potential issue | 🟡 Minor
fileId2is never cleaned up after tests.The
beforehook creates two sounds (fileIdandfileId2), but theafterhook only deletesfileId. This leaves orphaned test data in the database.Proposed fix
after(() => - request + Promise.all([request .post(api('method.call/deleteCustomSound')) .set(credentials) .send({ message: JSON.stringify({ msg: 'method', id: '33', method: 'deleteCustomSound', params: [fileId], }), - }), + }), + request + .post(api('method.call/deleteCustomSound')) + .set(credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: '34', + method: 'deleteCustomSound', + params: [fileId2], + }), + }), + ]), );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/tests/end-to-end/api/custom-sounds.ts
🧰 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/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
🧠 Learnings (22)
📚 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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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 **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.ts
📚 Learning: 2026-01-26T18:26:01.279Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38227
File: apps/meteor/app/api/server/router.ts:44-49
Timestamp: 2026-01-26T18:26:01.279Z
Learning: In apps/meteor/app/api/server/router.ts, when retrieving bodyParams and queryParams from the Hono context via c.get(), do not add defensive defaults (e.g., ?? {}). The code should fail fast if these parameters are missing, as endpoint handlers expect them to be present and breaking here helps surface parsing problems rather than hiding them.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
🧬 Code graph analysis (2)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
packages/core-services/src/index.ts (1)
api(55-55)
apps/meteor/app/api/server/v1/custom-sounds.ts (2)
packages/models/src/index.ts (1)
CustomSounds(144-144)packages/rest-typings/src/v1/customSounds.ts (1)
isCustomSoundsGetOneProps(18-18)
🔇 Additional comments (2)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
188-240: Good test coverage for the newgetOneendpoint.The test suite covers auth, validation, not-found, success with response shape verification, and three regex injection variants. Well done.
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
16-24: No action needed. TheEditSoundcomponent is properly designed to handle the limitedgetOneresponse.The
EditSoundPropstype definesextensionas optional (extension?: string), andEditSoundonly requires_idandnamefrom thedataprop—both of which thegetOneendpoint provides. The component safely handles the case whereextensionis missing.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
nazabucciarelli
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
43-43:...propsspread is always{}— dead code.All properties of
EditCustomSoundProps(_id,onChange,close) are destructured, sopropsis always an empty object. The spread onto<EditSound>is a no-op.♻️ Proposed fix
-function EditCustomSound({ _id, onChange, close, ...props }: EditCustomSoundProps): ReactElement | null { +function EditCustomSound({ _id, onChange, close }: EditCustomSoundProps): ReactElement | null { ... - return <EditSound data={data.sound} close={close} onChange={handleChange} {...props} />; + return <EditSound data={data.sound} close={close} onChange={handleChange} />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx` at line 43, The spread {...props} in the return of EditCustomSound is dead because EditCustomSound destructures all EditCustomSoundProps (including _id, onChange, close), so props is always {} — remove the unnecessary spread; update the JSX returned by EditCustomSound to only pass the needed props (data={data.sound}, close={close}, onChange={handleChange}) to <EditSound>, or alternatively change the component signature to use rest-collection (e.g., ...props) if you actually intended to forward additional props to EditSound.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/rude-plums-think.mdapps/meteor/app/api/server/v1/custom-sounds.tsapps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/tests/end-to-end/api/custom-sounds.tspackages/rest-typings/src/v1/customSounds.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/rude-plums-think.md
🧰 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/admin/customSounds/EditCustomSound.tsxapps/meteor/tests/end-to-end/api/custom-sounds.tspackages/rest-typings/src/v1/customSounds.tsapps/meteor/app/api/server/v1/custom-sounds.ts
🧠 Learnings (23)
📚 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/admin/customSounds/EditCustomSound.tsx
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsxapps/meteor/app/api/server/v1/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.tsapps/meteor/app/api/server/v1/custom-sounds.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/end-to-end/api/custom-sounds.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/end-to-end/api/custom-sounds.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 **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/custom-sounds.ts
📚 Learning: 2026-01-26T18:26:01.279Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38227
File: apps/meteor/app/api/server/router.ts:44-49
Timestamp: 2026-01-26T18:26:01.279Z
Learning: In apps/meteor/app/api/server/router.ts, when retrieving bodyParams and queryParams from the Hono context via c.get(), do not add defensive defaults (e.g., ?? {}). The code should fail fast if these parameters are missing, as endpoint handlers expect them to be present and breaking here helps surface parsing problems rather than hiding them.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/app/api/server/v1/custom-sounds.ts
🧬 Code graph analysis (3)
apps/meteor/tests/end-to-end/api/custom-sounds.ts (1)
packages/core-services/src/index.ts (1)
api(55-55)
packages/rest-typings/src/v1/customSounds.ts (1)
apps/meteor/app/api/server/v1/custom-sounds.ts (1)
CustomSoundEndpoints(160-160)
apps/meteor/app/api/server/v1/custom-sounds.ts (2)
packages/models/src/index.ts (1)
CustomSounds(136-136)packages/rest-typings/src/v1/customSounds.ts (1)
isCustomSoundsGetOneProps(18-18)
⏰ 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). (5)
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build (coverage)
🔇 Additional comments (3)
apps/meteor/app/api/server/v1/custom-sounds.ts (2)
82-90:requiredarray is now correctly positioned as a sibling ofproperties— previous issue resolved.
121-158:getOneendpoint implementation is correct.
findOneById+notFoundfallback + returning the fullICustomSoundobject undersoundkey is the right design. The 404 mapping andisCustomSoundsGetOnePropsquery validation are properly wired.apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx (1)
10-14: No issues found: thecloseprop is required and all callers provide it.The single call site in
CustomSoundsPage.tsx(line 70) correctly passesclose={handleClose}when renderingEditCustomSound.
🤖 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/admin/customSounds/EditCustomSound.tsx`:
- Around line 20-33: The component EditCustomSound uses useQuery with enabled:
!!_id but checks isPending to render <FormSkeleton>, which stays true when the
query is disabled; change the render condition to use the derived isLoading flag
(or use isPending && isFetching) from the useQuery result instead of isPending
alone so the skeleton only shows during an actual load; update the check around
FormSkeleton to reference isLoading (from the object returned by useQuery) and
keep the existing queryFn/getSound and enabled: !!_id logic intact.
In `@apps/meteor/tests/end-to-end/api/custom-sounds.ts`:
- Around line 197-208: The test for the custom-sounds.getOne endpoint is
asserting top-level _id and name but the response nests the sound object under
res.body.sound; update the assertions in the test (the it block calling
api('custom-sounds.getOne') that uses fileId and fileName) to assert
res.body.sound._id === fileId and res.body.sound.name === fileName while keeping
the existing check for res.body.success.
In `@packages/rest-typings/src/v1/customSounds.ts`:
- Around line 20-24: The CustomSoundEndpoints return type for
'/v1/custom-sounds.getOne' is missing the success flag and conflicts with the
server-side augmentation; update the GET return type in CustomSoundEndpoints so
it matches the server schema by including success: boolean alongside sound:
ICustomSound (i.e., change the return shape for GET (params: CustomSoundsGetOne)
to include both sound and success). Ensure you modify the CustomSoundEndpoints
declaration (the '/v1/custom-sounds.getOne' GET signature) so client typings and
server-derived Endpoints agree.
---
Nitpick comments:
In `@apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx`:
- Line 43: The spread {...props} in the return of EditCustomSound is dead
because EditCustomSound destructures all EditCustomSoundProps (including _id,
onChange, close), so props is always {} — remove the unnecessary spread; update
the JSX returned by EditCustomSound to only pass the needed props
(data={data.sound}, close={close}, onChange={handleChange}) to <EditSound>, or
alternatively change the component signature to use rest-collection (e.g.,
...props) if you actually intended to forward additional props to EditSound.
apps/meteor/client/views/admin/customSounds/EditCustomSound.tsx
Outdated
Show resolved
Hide resolved
gabriellsh
previously approved these changes
Feb 19, 2026
Member
gabriellsh
left a comment
gabriellsh
left a comment
There was a problem hiding this comment.
LGTM! Thanks for adressing the requested changes.
KevLehman
requested changes
Feb 19, 2026
KevLehman
requested changes
Feb 20, 2026
KevLehman
approved these changes
Feb 20, 2026
aleksandernsilva
approved these changes
Feb 23, 2026
nazabucciarelli
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
ahmed-n-abdeltwab
pushed a commit
to ahmed-n-abdeltwab/Rocket.Chat
that referenced
this pull request
Feb 24, 2026
…nd add Contextualbar bug fixes (RocketChat#38610) Co-authored-by: gabriellsh <[email protected]>
Closed
Open
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
7 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.
Proposed changes (including videos or screenshots)
Contextualbarbug fixes reverted on [38818].(Revert "fix: ContextualBar state and refresh behavior on custom sounds (#38442)" #38818)custom-sounds.getOneAPI endpoint with its validators extracted in therest-typingspackage (a task was created to do the same withcustom-sounds.list)._idexists and correct handling when the resource is not found by returning<ContextualbarEmptyContent>.Issue(s)
CORE-1819 - Address technical debt following CORE-1789 (custom-sounds single resource fetch)
Steps to test or reproduce
This can be tested by getting into the Custom Sounds admin page, creating some, trying to edit them and observing in the network tab how the API call to
custom-sounds.getOneis made successfully.Further comments
Summary by CodeRabbit
New Features
Refactor
Tests
Chores