chore: add contact filter to call history endpoint #37896
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant MongoDB
Client->>Server: GET /v1/call-history?filter=TEXT&direction=&state=
Note right of Server: parse & trim query params\nbuild base query with direction/state
Server->>Server: if filter non-empty -> construct $or with\n- contactName (case-insensitive) for non-external\n- contactUsername (case-insensitive) for non-external\n- contactExtension (case-insensitive) for external
Server->>MongoDB: find(query)
MongoDB-->>Server: results
Server-->>Client: paginated results with matching items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 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 19:59", "12/19 20:21 (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]
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]
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]
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, 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]
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, 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.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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 25 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Fri, 19 Dec 2025 20:21:59 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 (1)
apps/meteor/app/api/server/v1/call-history.ts (1)
115-115: Consider more explicit filter parsing.The expression
typeof filter === 'string' && filter.trim()works correctly but could be more readable. Consider:const filterText = filter?.trim() || undefined;This makes the intent clearer while maintaining the same behavior (falsy for empty/whitespace strings).
📜 Review details
Configuration used: Organization 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 (3)
apps/meteor/app/api/server/v1/call-history.ts(3 hunks)apps/meteor/server/startup/callHistoryTestData.ts(4 hunks)apps/meteor/tests/end-to-end/api/call-history.ts(8 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/callHistoryTestData.tsapps/meteor/app/api/server/v1/call-history.tsapps/meteor/tests/end-to-end/api/call-history.ts
🧠 Learnings (3)
📚 Learning: 2025-12-18T15:18:23.819Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:23.819Z
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/server/startup/callHistoryTestData.tsapps/meteor/app/api/server/v1/call-history.tsapps/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/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 : 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 (1)
apps/meteor/tests/data/api-data.ts (2)
request(10-10)credentials(39-42)
🔇 Additional comments (8)
apps/meteor/app/api/server/v1/call-history.ts (1)
17-21: LGTM!The type definition correctly adds the optional
filterfield to support text-based search across contact fields.apps/meteor/server/startup/callHistoryTestData.ts (3)
28-29: LGTM: Test data includes searchable contact fields.The added
contactNameandcontactUsernamefields use patterns (Pineapple, Apple, Grapefruit, Pasta, fruit-001, fruit-002) that enable comprehensive testing of exact matches, partial matches, and cross-field searches.Also applies to: 43-44, 58-59, 73-74
100-100: LGTM: contactExtension change aligns with test expectations.Changing from '1001' to '1002' creates better test coverage for partial matches and ensures two distinct external extensions for filtering tests.
9-13: Incomplete test data setup: CallHistory entries reference non-existent MediaCalls entries.Lines 9-10 define
extraCallId1andextraCallId2, which are inserted into CallHistory (lines 49, 64), butMediaCalls.insertMany(lines 104-233) only creates entries forcallId1-4. The cleanup at line 13 also omits these extra IDs.The
call-history.infoAPI endpoint gracefully handles missing MediaCalls (returnsnullfor the optionalcallproperty), but the inconsistent test data setup creates orphaned history entries. Either addextraCallId1andextraCallId2to the cleanup at line 13, or create corresponding MediaCalls entries for these IDs in theinsertManyblock.⛔ Skipped due to learnings
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.Learnt from: gabriellsh Repo: RocketChat/Rocket.Chat PR: 37773 File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34 Timestamp: 2025-12-18T15:18:23.819Z 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.apps/meteor/tests/end-to-end/api/call-history.ts (4)
35-37: LGTM: Test expectations correctly updated for new test data.The updated counts (4→6) and assertions for the two new history entries (
outbound.2,inbound.2) correctly reflect the expanded test data. ThecontactNameandcontactUsernameassertions properly validate the new searchable fields.Also applies to: 44-45, 55-56, 66-67, 69-79
211-231: LGTM: Exact match filter tests are comprehensive.Tests correctly verify exact matches for contact name, username, and extension, expecting 1 result each with the correct history ID.
Also applies to: 256-276, 325-345
233-254: LGTM: Partial match tests correctly validate substring search.These tests comprehensively verify that the filter matches substrings across contact name, username, and extension fields. The expected counts and history IDs align with the test data patterns.
Also applies to: 278-299, 301-323, 347-368
370-392: LGTM: Combined and edge case filter tests are correct.Lines 370-392 correctly verify cross-field matching (filter '002' matches
fruit-002,Grapefruit 002,1002).Lines 394-418 correctly test that a falsy-but-non-empty filter value ('0') still applies the search, matching 5 items that contain '0' in their contact fields while excluding items without '0' (e.g., 'Pasta 1', 'meal').
Also applies to: 394-418
pierre-lehnen-rc
force-pushed
the
chore/history-filter
branch
from
54e6776 to
7e15d92
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/call-history.ts (1)
211-418: Good coverage of filter scenarios.The filter tests comprehensively validate exact and partial matching across contact names, usernames, and extensions, including cross-field queries and edge cases with falsy values. The test logic is correct and expectations match the test data.
Optional: Consider adding case-insensitivity and combined filter tests
Two optional test enhancements to consider:
- Case-insensitive matching: Explicitly verify case-insensitive behavior:
it('should return items with case-insensitive filter matching', async () => { await request .get(api('call-history.list')) .set(credentials) .query({ filter: 'PINEAPPLE' }) .expect(200) .expect((res: Response) => { expect(res.body.items).to.have.lengthOf(1); expect(res.body.items[0]._id).to.equal('rocketchat.internal.history.test.outbound'); }); });
- Combined filters: Test filter combined with state/direction:
it('should apply filter combined with state and direction', async () => { await request .get(api('call-history.list')) .set(credentials) .query({ filter: 'fruit', state: ['ended'], direction: 'outbound' }) .expect(200) .expect((res: Response) => { expect(res.body.items).to.have.lengthOf(1); expect(res.body.items[0]._id).to.equal('rocketchat.internal.history.test.outbound'); }); });
📜 Review details
Configuration used: Organization 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 (3)
apps/meteor/app/api/server/v1/call-history.ts(3 hunks)apps/meteor/server/startup/callHistoryTestData.ts(4 hunks)apps/meteor/tests/end-to-end/api/call-history.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/server/startup/callHistoryTestData.ts
- apps/meteor/app/api/server/v1/call-history.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/call-history.ts
🧠 Learnings (3)
📚 Learning: 2025-12-18T15:18:23.819Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:23.819Z
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/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 : 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 (1)
apps/meteor/tests/data/api-data.ts (2)
request(10-10)credentials(39-42)
⏰ 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)
35-79: LGTM! Test data and assertions updated correctly.The updated counts (4→6 items) and new assertions for
contactNameandcontactUsernamecorrectly reflect the expanded test data needed to validate the new filter functionality.
177-184: LGTM! Direction filter test updated correctly.The test now correctly expects 3 inbound items and includes the new test data ID.
445-446: LGTM! Contact fields verified in info endpoint.The
contactNameandcontactUsernameassertions are correctly added to the/call-history.infotests and are consistent with the list endpoint expectations.Also applies to: 492-493
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37896 +/- ##
===========================================
+ Coverage 67.68% 67.69% +0.01%
===========================================
Files 3474 3475 +1
Lines 113852 113863 +11
Branches 20938 20942 +4
===========================================
+ Hits 77065 77085 +20
+ Misses 34604 34592 -12
- Partials 2183 2186 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
gabriellsh
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Issue(s)
VGA-115
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Tests
Chore
✏️ Tip: You can customize this high-level summary in your review settings.