chore: remove deprecated methods for 8.0 #6852
Conversation
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughAdds new Omnichannel and Rooms REST endpoints and updates service and EE/UI call sites to use them with server-version guards; several functions gain extra parameters (serverVersion, departmentId, userId) and branch between new REST calls and legacy method wrappers. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant Service as restApi.ts
participant EE as EE helper (omnichannel lib)
participant ServerREST as Server (REST)
participant ServerLegacy as Server (legacy RPC)
UI->>Service: call returnLivechat(rid, departmentId?) / toggleMuteUserInRoom(...) / getRoutingConfig()
alt calling takeInquiry flow
UI->>EE: takeInquiry(inquiryId, serverVersion)
EE->>Service: delegate to REST or legacy based on serverVersion
end
alt serverVersion >= threshold
Service->>ServerREST: POST /livechat/inquiries.take or /livechat/inquiries.returnAsInquiry or /rooms.muteUser
ServerREST-->>Service: HTTP response
else older server
Service->>ServerLegacy: sdk.methodCallWrapper(...) (legacy RPC)
ServerLegacy-->>Service: RPC response
end
Service-->>UI: resolved result or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/views/RoomView/index.tsx (1)
1087-1110: Consider adding a null check forserverVersion.The
serverVersionprop can benull(as seen fromstate.server.version), but line 1095 casts it withas string. WhilecompareServerVersionhandles null values gracefully (returning false), the type assertion masks this possibility.Consider a defensive check:
if (this.isOmnichannel) { if ('_id' in room) { - await takeInquiry(room._id, serverVersion as string); + if (serverVersion) { + await takeInquiry(room._id, serverVersion); + } } this.onJoin(); }Alternatively, update the
takeInquirysignature to explicitly acceptstring | nullif the fallback behavior is intended.
📜 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)
app/definitions/rest/v1/omnichannel.ts(2 hunks)app/definitions/rest/v1/rooms.ts(1 hunks)app/ee/omnichannel/lib/index.ts(1 hunks)app/lib/services/restApi.ts(3 hunks)app/views/RoomActionsView/index.tsx(1 hunks)app/views/RoomMembersView/helpers.ts(1 hunks)app/views/RoomView/RightButtons.tsx(1 hunks)app/views/RoomView/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/views/RoomActionsView/index.tsx (2)
app/lib/methods/helpers/info.ts (1)
showConfirmationAlert(30-54)app/lib/services/restApi.ts (1)
returnLivechat(433-442)
app/ee/omnichannel/lib/index.ts (2)
app/ee/omnichannel/sagas/inquiry.js (1)
serverVersion(17-17)app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)
app/views/RoomMembersView/helpers.ts (1)
app/lib/services/restApi.ts (1)
toggleMuteUserInRoom(532-559)
app/views/RoomView/index.tsx (2)
app/ee/omnichannel/lib/index.ts (1)
takeInquiry(24-30)app/ee/omnichannel/sagas/inquiry.js (1)
serverVersion(17-17)
🔇 Additional comments (7)
app/definitions/rest/v1/omnichannel.ts (1)
22-34: LGTM!The new endpoint definitions for
livechat/config/routing,livechat/inquiries.take, andlivechat/inquiries.returnAsInquiryare well-typed and align with the server-version-based routing implementation inrestApi.ts.Also applies to: 138-144
app/views/RoomActionsView/index.tsx (1)
448-463: LGTM!The
handleReturnLivechatmethod correctly extractsdepartmentIdfrom the room state and passes it to the updatedreturnLivechatAPI, enabling department-aware inquiry return for newer server versions.app/views/RoomMembersView/helpers.ts (1)
50-59: LGTM!The
handleMutefunction correctly passes the newuserIdparameter totoggleMuteUserInRoom, enabling the server-version-based routing to use the new REST endpoints (rooms.muteUser/rooms.unmuteUser) on servers >= 6.8.0.app/views/RoomView/RightButtons.tsx (1)
234-249: LGTM!The
returnLivechatmethod correctly extractsdepartmentIdfrom props and passes it to the service function, consistent with the pattern used inRoomActionsView.app/lib/services/restApi.ts (3)
433-442: LGTM!The
returnLivechatfunction correctly implements server-version-based routing, using the new REST endpoint for servers >= 7.12.0 while maintaining backward compatibility with the method call for older servers. The optionaldepartmentIdparameter is properly forwarded.
469-488: LGTM!The
getRoutingConfigfunction is properly converted to async and implements server-version-based routing with appropriate fallback to the method call for older servers.
532-559: LGTM!The
toggleMuteUserInRoomfunction correctly adds the newuserIdparameter for the REST endpoints on servers >= 6.8.0 while preserving theusernameparameter for backward compatibility with older servers using method calls.
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/lib/services/restApi.ts (1)
471-490: Consider adding a try/catch around the new routing-config REST call.If
sdk.get('livechat/config/routing')rejects (network/404/etc.),getRoutingConfigwill currently fail without ever trying the legacylivechat:getRoutingConfigmethod, even on servers where it still exists. A small try/catch would make this more robust:export const getRoutingConfig = async (): Promise<{ @@ }> => { const serverVersion = reduxStore.getState().server.version; if (compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.11.0')) { - const result = await sdk.get('livechat/config/routing'); - if (result.success) { - return result.config; - } + try { + const result = await sdk.get('livechat/config/routing'); + if (result.success) { + return result.config; + } + } catch { + // fall through to legacy method + } } // RC 2.0.0 return sdk.methodCallWrapper('livechat:getRoutingConfig'); };
📜 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 (4)
app/definitions/rest/v1/rooms.ts(1 hunks)app/ee/omnichannel/lib/index.ts(1 hunks)app/lib/services/restApi.ts(3 hunks)app/views/RoomMembersView/helpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/definitions/rest/v1/rooms.ts
- app/views/RoomMembersView/helpers.ts
🧰 Additional context used
🧬 Code graph analysis (2)
app/lib/services/restApi.ts (6)
app/ee/omnichannel/sagas/inquiry.js (1)
serverVersion(17-17)app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)app/sagas/login.js (3)
result(75-75)userId(134-134)userId(341-341)app/sagas/messages.js (1)
result(22-22)app/sagas/room.js (3)
result(96-96)result(122-122)result(146-146)app/sagas/inviteLinks.js (3)
result(20-20)result(29-29)result(51-55)
app/ee/omnichannel/lib/index.ts (2)
app/ee/omnichannel/sagas/inquiry.js (1)
serverVersion(17-17)app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)
⏰ 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: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (3)
app/ee/omnichannel/lib/index.ts (1)
23-29: Server-version gating fortakeInquirynow aligns with the deprecation path.Routing
>= 7.11.0tolivechat/inquiries.takeand older servers tolivechat:takeInquiryfixes the earlier inversion and avoids the removed method on 8.0+. Please just confirm 7.11.0 is indeed the minimum version exposing the REST endpoint.app/lib/services/restApi.ts (2)
433-442:returnLivechatcorrectly prefers the new REST endpoint for modern servers.Using
livechat/inquiries.returnAsInquirywhenserverVersion >= 7.12.0and falling back tolivechat:returnAsInquiryotherwise looks correct and keeps 8.0+ off the removed method. The optionaldepartmentIdis passed only where supported. Please double-check that 7.12.0 is the right introduction version for this endpoint.
534-542:toggleMuteUserInRoomversion gating and payloads are correctly implemented.The single call site in
app/views/RoomMembersView/helpers.ts(line 52) correctly passes all four arguments including theuserIdparameter (user?._id). The version gating logic is sound:>= 6.8.0usesrooms.muteUser/rooms.unmuteUserwith{ roomId, userId }, and the fallback uses the oldermuteUserInRoom/unmuteUserInRoomwith{ rid, username }for older servers.
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
official_android_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
OtavioStasiak
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/lib/services/restApi.ts (1)
433-442: Consider using conditional spread for optional departmentId parameter.The
departmentIdparameter is always passed to the POST request even when undefined. While this is acceptable (most REST implementations handle undefined fields correctly), it could be more explicit to only include it when defined.Apply this diff for more explicit optional parameter handling:
export const returnLivechat = (rid: string, departmentId?: string): Promise<any> => { const serverVersion = reduxStore.getState().server.version; if (compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.12.0')) { - return sdk.post('livechat/inquiries.returnAsInquiry', { roomId: rid, departmentId }); + return sdk.post('livechat/inquiries.returnAsInquiry', { + roomId: rid, + ...(departmentId && { departmentId }) + }); } // RC 0.72.0 return sdk.methodCallWrapper('livechat:returnAsInquiry', rid); };
📜 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 (2)
app/lib/services/restApi.ts(3 hunks)app/views/RoomMembersView/helpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/RoomMembersView/helpers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
app/lib/services/restApi.ts (1)
app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)
⏰ 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: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (2)
app/lib/services/restApi.ts (2)
534-541: Call sites correctly updated with userId parameter.The single call site in
app/views/RoomMembersView/helpers.ts:52has been properly updated to pass all four parameters:toggleMuteUserInRoom(rid, user?.username, user?._id, !user.muted). The migration to userId-based identification improves reliability over username-based lookups.
471-490: The error handling approach here is consistent with similar version-gated functions in the codebase.The function already includes
result.successcheck to handle endpoint-level failures. Notably, similar version-gated functions likereturnLivechat(7.12.0+) andtoggleMuteUserInRoom(6.8.0+) return the SDK call directly without any success checking, making them more susceptible to unhandled failures thangetRoutingConfig. The codebase architecture intentionally avoids try-catch wrapping for SDK calls, distinguishing between:
- Endpoint failures: handled via
result.successcheck (as done here)- Network/request errors: allowed to propagate (consistent across all version-gated functions)
This pattern is uniform throughout the file and appears intentional.
Likely an incorrect or invalid review comment.
Proposed changes
Remove deprecated methods based on https://docs.rocket.chat/docs/deprecated-and-phasing-out-features.
New endpoints
livechat/inquiries.takelivechat/inquiries.returnAsInquirylivechat/config/routingrooms.muteUserrooms.unmuteUserDeprecated methods
livechat:takeInquirylivechat:returnAsInquirylivechat:getRoutingConfigmuteUserInRoomunmuteUserInRoomIssue(s)
https://rocketchat.atlassian.net/browse/CORE-1560
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.