chore: add endpoints for retrieving call history data #37702
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughAdds v1 REST endpoints for call history (list and info), new CallHistory model query helpers, test-data seeding during initialization, end-to-end tests for the endpoints, and typing/schema updates to include IMediaCall and CallHistoryItem. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Server (v1/call-history)
participant Model as CallHistory Model
participant DB as MongoDB / MediaCalls
Client->>API: GET /v1/call-history.list (auth, query)
API->>Model: query with uid + pagination filters
Model->>DB: find documents (CallHistory) & count
DB-->>Model: return items + count
Model-->>API: items, count, offset, total
API-->>Client: 200 { items, count, offset, total }
Client->>API: GET /v1/call-history.info (auth, historyId|callId)
alt historyId provided
API->>Model: findOneByIdAndUid(historyId, uid)
else callId provided
API->>Model: findOneByCallIdAndUid(callId, uid)
end
Model->>DB: findOne matching query
DB-->>Model: item or null
alt item is media-call and has callId
API->>DB: fetch MediaCalls by callId
DB-->>API: IMediaCall (or null)
end
API-->>Client: 200 { item, call? } or 404
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧬 Code graph analysis (1)packages/models/src/models/CallHistory.ts (1)
🔇 Additional comments (3)
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 |
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 13:34", "12/09 13:53 (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]
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]
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]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
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]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-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]
Statistics (last 16 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 09 Dec 2025 13:53:54 GMT |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/tests/end-to-end/api/call-history.ts (1)
15-20: Consolidate duplicatebeforehooks.Biome correctly flags duplicate
beforehooks. Consolidate into a single hook for cleaner test setup.- before((done) => getCredentials(done)); - - before(async () => { + before(async function (done) { + getCredentials(done); + }); + + before(async () => { user2 = await createUser(); userCredentials = await login(user2.username, password); });Alternatively, if both need to remain separate due to async handling, consider using
beforeEachfor the second or restructuring to avoid the linter warning.apps/meteor/app/api/server/v1/call-history.ts (1)
167-169: Redundant validation check.The
oneOfschema validation inisCallHistoryInfoPropsalready ensures exactly one ofhistoryIdorcallIdis provided. This manual check is redundant but harmless as a defensive measure.If you prefer to remove the redundancy:
async function action() { - if (!this.queryParams.historyId && !this.queryParams.callId) { - return API.v1.failure(); - } - const item = await (this.queryParams.historyId
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/meteor/app/api/server/index.ts(1 hunks)apps/meteor/app/api/server/v1/call-history.ts(1 hunks)apps/meteor/server/startup/callHistoryTestData.ts(1 hunks)apps/meteor/server/startup/initialData.js(2 hunks)apps/meteor/tests/end-to-end/api/call-history.ts(1 hunks)packages/core-typings/src/Ajv.ts(1 hunks)packages/model-typings/src/models/ICallHistoryModel.ts(1 hunks)packages/models/src/models/CallHistory.ts(2 hunks)
🧰 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/server/startup/initialData.jsapps/meteor/server/startup/callHistoryTestData.tspackages/model-typings/src/models/ICallHistoryModel.tsapps/meteor/tests/end-to-end/api/call-history.tsapps/meteor/app/api/server/index.tsapps/meteor/app/api/server/v1/call-history.tspackages/models/src/models/CallHistory.tspackages/core-typings/src/Ajv.ts
🧠 Learnings (11)
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
apps/meteor/server/startup/initialData.js
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/server/startup/initialData.js
📚 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/server/startup/callHistoryTestData.tsapps/meteor/tests/end-to-end/api/call-history.tsapps/meteor/app/api/server/index.tsapps/meteor/app/api/server/v1/call-history.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/server/startup/callHistoryTestData.tsapps/meteor/tests/end-to-end/api/call-history.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/server/startup/callHistoryTestData.tsapps/meteor/tests/end-to-end/api/call-history.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/call-history.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/call-history.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/call-history.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/call-history.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/call-history.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/call-history.ts
🧬 Code graph analysis (4)
apps/meteor/server/startup/initialData.js (1)
apps/meteor/server/startup/callHistoryTestData.ts (1)
addCallHistoryTestData(3-197)
apps/meteor/server/startup/callHistoryTestData.ts (1)
packages/models/src/index.ts (2)
CallHistory(149-149)MediaCalls(186-186)
packages/model-typings/src/models/ICallHistoryModel.ts (1)
packages/core-typings/src/ICallHistoryItem.ts (1)
CallHistoryItem(51-51)
apps/meteor/tests/end-to-end/api/call-history.ts (3)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/tests/data/api-data.ts (3)
getCredentials(68-83)request(10-10)credentials(39-42)apps/meteor/tests/data/users.helper.ts (1)
createUser(45-78)
🪛 Biome (2.1.2)
apps/meteor/server/startup/initialData.js
[error] 5-6: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 6-7: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
apps/meteor/tests/end-to-end/api/call-history.ts
[error] 17-20: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (9)
packages/core-typings/src/Ajv.ts (1)
3-15: LGTM!The schema expansion correctly includes
IMediaCallandCallHistoryItemtypes, aligning with the new call history endpoints that return these types in their responses.apps/meteor/app/api/server/index.ts (1)
12-12: LGTM!The import is correctly placed in alphabetical order and registered before OpenAPI generation.
apps/meteor/server/startup/initialData.js (1)
255-258: LGTM!Test data seeding is correctly gated behind
TEST_MODEand follows the proper order by awaitingaddUserToDefaultChannelsbefore creating call history data. The static analysis hints about import declarations are false positives in the Meteor context.apps/meteor/server/startup/callHistoryTestData.ts (1)
3-197: LGTM!The test data setup is well-structured with:
- Proper cleanup before insertion ensuring idempotency
- Comprehensive coverage of scenarios (internal/external calls, inbound/outbound directions)
- Correct cross-linking between
CallHistoryandMediaCallsviacallIdpackages/model-typings/src/models/ICallHistoryModel.ts (1)
6-18: LGTM!The interface correctly defines the new query methods with proper type safety using indexed access types (
CallHistoryItem['_id'], etc.) and aligns with the implementation inCallHistory.ts.apps/meteor/tests/end-to-end/api/call-history.ts (1)
24-236: Test coverage is comprehensive.The test suite effectively covers:
- Listing history entries with field validation
- Access control verification (user isolation)
- Info retrieval by both
historyIdandcallId- Error handling for invalid and cross-user access attempts
packages/models/src/models/CallHistory.ts (1)
16-30: LGTM!The implementations are clean and correctly delegate to
findOnewith compound filters. The existing indexes ({ uid: 1, callId: 1 }and{ uid: 1, ts: -1 }) adequately support these queries.apps/meteor/app/api/server/v1/call-history.ts (2)
34-97: LGTM!The list endpoint correctly implements pagination, filters by the authenticated user's ID for access control, and returns the expected response shape.
132-191: LGTM!The info endpoint correctly:
- Supports lookup by either
historyIdorcallId- Enforces user-scoped access control via
findOneByIdAndUid/findOneByCallIdAndUid- Returns 404 when item is not found or belongs to another user
- Enriches response with associated
MediaCalldata when applicable
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/meteor/tests/end-to-end/api/call-history.ts (1)
83-83: Fix typo in test description."nost" should be "not".
- it('should not list history entries from other users', async () => { + it('should not list history entries from other users', async () => {
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/call-history.ts (1)
39-79: Consider using typed interfaces instead ofanyfor better type safety.The test uses
anyfor item objects throughout. While functional, using proper types from@rocket.chat/core-typingswould provide better type safety and IDE support.Example refactor:
+import type { ICallHistoryItem } from '@rocket.chat/core-typings'; -const historyIds = res.body.items.map((item: any) => item._id); +const historyIds = res.body.items.map((item: ICallHistoryItem) => item._id); -const internalItem1 = res.body.items.find((item: any) => item._id === 'rocketchat.internal.history.test'); +const internalItem1 = res.body.items.find((item: ICallHistoryItem) => item._id === 'rocketchat.internal.history.test');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/tests/end-to-end/api/call-history.ts(1 hunks)
🧰 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/call-history.ts
🧠 Learnings (13)
📚 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/call-history.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/call-history.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/call-history.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/call-history.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/call-history.ts
📚 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/tests/end-to-end/api/call-history.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/call-history.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/call-history.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/call-history.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/call-history.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/end-to-end/api/call-history.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/call-history.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/call-history.ts
🧬 Code graph analysis (1)
apps/meteor/tests/end-to-end/api/call-history.ts (3)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/tests/data/api-data.ts (3)
getCredentials(68-83)request(10-10)credentials(39-42)apps/meteor/tests/data/users.helper.ts (1)
createUser(45-78)
🪛 Biome (2.1.2)
apps/meteor/tests/end-to-end/api/call-history.ts
[error] 17-20: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
apps/meteor/tests/end-to-end/api/call-history.ts (3)
1-10: LGTM!Imports are well-organized and include all necessary types and utilities for the test suite.
11-23: LGTM!Test setup correctly creates a secondary user for access-control validation and properly cleans up in the after hook. Multiple
beforehooks are valid in Mocha and run sequentially.Note: The static analysis warning about duplicate hooks is a false positive.
100-234: LGTM!The test suite for
/call-history.infois comprehensive with excellent coverage of:
- Positive scenarios (retrieve by historyId and callId)
- Error handling (404 for invalid IDs)
- Access control (cross-user access properly returns 404)
All critical fields are validated for both the history item and associated call objects.
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37702 +/- ##
===========================================
- Coverage 67.80% 67.72% -0.08%
===========================================
Files 3448 3449 +1
Lines 113918 113927 +9
Branches 20915 20915
===========================================
- Hits 77239 77158 -81
- Misses 34548 34645 +97
+ Partials 2131 2124 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/meteor/app/api/server/v1/call-history.ts (1)
107-132:nullable: falseis redundant on required string properties.Since
historyIdandcallIdare already in therequiredarray and typed asstring, they cannot be null by default. Thenullable: falseannotations on lines 114 and 125 can be removed.const CallHistoryInfoSchema = { oneOf: [ { type: 'object', properties: { historyId: { type: 'string', - nullable: false, }, }, required: ['historyId'], additionalProperties: false, }, { type: 'object', properties: { callId: { type: 'string', - nullable: false, }, }, required: ['callId'], additionalProperties: false, }, ], };
🧹 Nitpick comments (2)
apps/meteor/app/api/server/v1/call-history.ts (2)
151-156: Inconsistent schema definition:type: 'object'alongside$refis redundant.The
callproperty (lines 151-155) uses bothtype: 'object'and$ref, while theitemproperty (lines 147-149) correctly uses only$ref. For consistency and clarity, removetype: 'object'since the referenced schema already defines the type.call: { - type: 'object', $ref: '#/components/schemas/IMediaCall', description: 'The call information for the requested call history item.', nullable: true, },
172-174: Redundant validation check with unclear failure response.The
oneOfschema validation (lines 107-132) already ensures that exactly one ofhistoryIdorcallIdis provided. This manual check is redundant. If kept for defensive coding,API.v1.failure()should include a descriptive error message indicating what's wrong.Either remove the redundant check (relying on schema validation) or improve the error message:
- if (!this.queryParams.historyId && !this.queryParams.callId) { - return API.v1.failure(); - } + if (!this.queryParams.historyId && !this.queryParams.callId) { + return API.v1.failure('Either historyId or callId is required'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/call-history.ts(1 hunks)
🧰 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/app/api/server/v1/call-history.ts
🧠 Learnings (1)
📚 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/app/api/server/v1/call-history.ts
🔇 Additional comments (4)
apps/meteor/app/api/server/v1/call-history.ts (4)
1-15: LGTM!Imports and type definitions are appropriate for the endpoint functionality.
17-34: LGTM!The schema correctly defines optional pagination parameters with proper type constraints.
36-101: LGTM!The endpoint correctly implements pagination, filters by the authenticated user's ID, and includes proper error response definitions. The parallel execution of cursor and count is efficient.
200-206: LGTM!The module augmentation correctly extends the
Endpointsinterface to include both endpoint definitions.
Proposed changes (including videos or screenshots)
Issue(s)
VGA-73
VGA-74
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Tests
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.