chore: Adds deprecation warning on livechat:removeAllClosedRooms with new endpoint replacing it #36921
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 22c5685 The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 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 |
WalkthroughAdds a new REST endpoint POST /v1/livechat/rooms.removeAllClosedRooms to remove closed livechat rooms (optionally by department), logs deprecation for the Meteor method livechat:removeAllClosedRooms, migrates clients to the REST endpoint, extends REST typings with request/response validators, and adds a changeset. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as UI
participant REST as POST /v1/livechat/rooms.removeAllClosedRooms
participant Auth as Auth & Permissions
participant Validator as Request/Response Validator
participant Rooms as LivechatRooms
participant Logic as removeOmnichannelRoom
Client->>REST: POST { departmentIds? }
REST->>Auth: verify user & permission
Auth-->>REST: authorized
REST->>Validator: validate request
Validator-->>REST: valid
REST->>Rooms: findClosedRooms(departmentIds, restrictions)
Rooms-->>REST: [rooms]
loop per room
REST->>Logic: removeOmnichannelRoom(room._id)
Logic-->>REST: removed
end
REST->>Validator: validate response
REST-->>Client: 200 { removedRooms, success: true }
sequenceDiagram
autonumber
actor Client
participant Method as Meteor method livechat:removeAllClosedRooms
note over Method: Deprecated — logs replacement endpoint
Client->>Method: invoke()
Method-->>Client: removedRooms count (legacy)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36921 +/- ##
===========================================
- Coverage 66.58% 66.56% -0.03%
===========================================
Files 3346 3346
Lines 114666 114666
Branches 21097 21118 +21
===========================================
- Hits 76350 76326 -24
- Misses 35617 35650 +33
+ Partials 2699 2690 -9
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: 2
🧹 Nitpick comments (6)
apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts (1)
26-28: Fix typo in error payload method name.The
methodfield sayslivechat:removeAllClosedRoom(missing “s”). Align it with the actual method key.- method: 'livechat:removeAllClosedRoom', + method: 'livechat:removeAllClosedRooms',packages/rest-typings/src/v1/omnichannel.ts (2)
3845-3861: Avoid ambiguous empty-array semantics in request schema.If clients send
departmentIds: [], it’s unclear whether that means “no filter” or “match none.” Enforce at least one item so callers either omit the field or pass a non-empty list. Coordinate with server and clients.departmentIds: { type: 'array', items: { type: 'string', }, - nullable: true, + minItems: 1, + nullable: true, },
3865-3882: Type thesuccessflag in the validator’s generic for stronger DX (optional).Runtime allows
success: true, but TypeScript consumers won’t see it. Consider including it in the generic for the compiled validator.-export const isPOSTLivechatRoomsCloseAllSuccessResponse = ajv.compile<{ removedRooms: number }>( +export const isPOSTLivechatRoomsCloseAllSuccessResponse = ajv.compile<{ removedRooms: number; success: true }>( POSTLivechatRoomsCloseAllSuccessResponseSchema, );.changeset/ninety-rocks-hope.md (1)
6-6: Clarify the replacement endpoint format in the changeset.Spell out the HTTP method and full path for clarity: “POST /v1/livechat/rooms.removeAllClosedRooms”.
-Adds deprecation warning on livechat:removeAllClosedRooms with new endpoint replacing it; `livechat/rooms.removeAllClosedRooms` +Adds deprecation warning on livechat:removeAllClosedRooms with new endpoint replacing it; `POST /v1/livechat/rooms.removeAllClosedRooms`apps/meteor/app/livechat/server/api/v1/room.ts (2)
441-449: Consider a basic rate limit for this destructive endpoint.A small limiter (e.g., 1–2 requests/min per user) reduces accidental repeated bulk deletions.
457-463: Optional: add bounded concurrency instead of fully sequential or unbounded parallelism.If you want faster execution without spikes, wrap deletions with a small concurrency limiter (e.g., p-limit 5–10). Keep per-request memory bounded.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/ninety-rocks-hope.md(1 hunks)apps/meteor/app/livechat/server/api/v1/room.ts(4 hunks)apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts(2 hunks)apps/meteor/client/views/omnichannel/currentChats/FilterByText.tsx(2 hunks)apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTableFilter.tsx(2 hunks)packages/rest-typings/src/v1/omnichannel.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTableFilter.tsx (2)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)apps/meteor/client/views/omnichannel/directory/contexts/ChatsContext.ts (1)
useChatsContext(64-71)
apps/meteor/client/views/omnichannel/currentChats/FilterByText.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)
apps/meteor/app/livechat/server/api/v1/room.ts (6)
apps/meteor/app/api/server/api.ts (1)
API(42-77)packages/rest-typings/src/v1/omnichannel.ts (2)
isPOSTLivechatRoomsCloseAllSuccessResponse(3880-3882)isPOSTLivechatRoomsCloseAll(3863-3863)apps/meteor/app/livechat/server/lib/logger.ts (1)
livechatLogger(5-5)apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts (1)
departmentIds(20-43)apps/meteor/app/livechat/server/lib/rooms.ts (1)
removeOmnichannelRoom(257-292)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(71-75)
⏰ 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/app/livechat/server/methods/removeAllClosedRooms.ts (1)
21-21: Deprecation instrumentation LGTM.The deprecation notice clearly points callers to the replacement endpoint and is versioned for 8.0.0.
apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTableFilter.tsx (1)
17-25: Don't send departmentIds: [] — omit the field or pass selected department IDsEmpty arrays are often treated as "match none". Omit departmentIds to act on all permitted departments or pass the selected department IDs instead.
Locations:
- Client: apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTableFilter.tsx (lines 17-25)
- Server: apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts (calls LivechatRooms.findClosedRooms), apps/meteor/app/livechat/server/api/v1/room.ts (calls LivechatRooms.findClosedRooms)
- const removeClosedRooms = useEndpoint('POST', '/v1/livechat/rooms.removeAllClosedRooms'); + const removeClosedRooms = useEndpoint('POST', '/v1/livechat/rooms.removeAllClosedRooms'); @@ - await removeClosedRooms({ departmentIds: [] }); + // No department filter here — request server default (all permitted) + const { removedRooms } = await removeClosedRooms({}); + // Optional: surface count to the user (keep existing i18n if preferred) + // dispatchToastMessage({ type: 'success', message: t('Chats_removed_count', { count: removedRooms }) });
aleksandernsilva
left a comment
There was a problem hiding this comment.
Suggestion: Upgrading the logic to useMutation or useEndpointMutation would be a good addition to ensure these components are following best practices. Could be done here or in a separate PR since the change is minimal.
Proposed changes (including videos or screenshots)
This PR adds a deprecation warning log on
livechat:removeAllClosedRooms, as well as it adds a new endpoint (livechat/rooms.removeAllClosedRooms) that replaces it using the same logic and business rules.Issue(s)
CTZ-58
Steps to test or reproduce
Further comments
Tests will be implemented on this task
Summary by CodeRabbit