chore: Adds deprecation warning on livechat:getRoutingConfig with new endpoint to replace it #36897
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 7ea8769 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 |
lucas-a-pelegrino
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36897 +/- ##
===========================================
+ Coverage 66.54% 66.56% +0.02%
===========================================
Files 3344 3344
Lines 114626 114629 +3
Branches 21090 21251 +161
===========================================
+ Hits 76273 76303 +30
+ Misses 35666 35620 -46
- Partials 2687 2706 +19
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
lucas-a-pelegrino
changed the title
WalkthroughAdds GET /v1/livechat/config/routing and AJV typings; client code switches from the Meteor method to the new REST endpoint; the legacy Meteor method logs a deprecation warning; a changeset bumps patch versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client (Web/App)
participant REST as REST API (/v1/livechat/config/routing)
participant Router as RoutingManager
Note over Client,REST: New flow — client fetches routing config via REST
Client->>REST: GET /v1/livechat/config/routing
REST->>Router: RoutingManager.getConfig()
Router-->>REST: OmichannelRoutingConfig | undefined
REST-->>Client: 200 { success: true, config }
Note over Client: Client uses config (e.g., config.autoAssignAgent)
sequenceDiagram
autonumber
participant Client as Client (Legacy)
participant Method as Meteor Method
participant Logger as DeprecationLogger
participant Router as RoutingManager
Note over Client,Method: Legacy method is deprecated but still returns config
Client->>Method: call livechat:getRoutingConfig
Method->>Logger: method('livechat:getRoutingConfig','8.0.0','v1/livechat/config/routing')
Method->>Router: RoutingManager.getConfig()
Router-->>Method: OmichannelRoutingConfig | undefined
Method-->>Client: OmichannelRoutingConfig | undefined
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Comment |
lucas-a-pelegrino
changed the title
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/meteor/app/livechat/server/methods/getRoutingConfig.ts (1)
17-17: Use absolute path in deprecation hint.Prefer including a leading slash to match REST paths consistently.
- methodDeprecationLogger.method('livechat:getRoutingConfig', '8.0.0', 'v1/livechat/config/routing'); + methodDeprecationLogger.method('livechat:getRoutingConfig', '8.0.0', '/v1/livechat/config/routing');packages/rest-typings/src/v1/omnichannel.ts (1)
2141-2147: Align schema with runtime shape (avoid null if undefined is used).Server returns
undefined(and JSON omits the key). Markingconfigasnullableallowsnull, which diverges from the TS generic{ config: OmichannelRoutingConfig | undefined }. Removenullable: truehere.- config: { - type: 'object', - nullable: true, - properties: { + config: { + type: 'object', + properties: {apps/meteor/app/livechat/server/api/v1/config.ts (1)
44-57: Guard against missing routing method to avoid 500s.
RoutingManager.getConfig()can throw when no method is set. Consider catching and returning{ config: undefined }to keep clients simple.- async function action() { - const config = RoutingManager.getConfig(); - return API.v1.success({ config }); - }, + async function action() { + try { + const config = RoutingManager.getConfig(); + return API.v1.success({ config }); + } catch { + return API.v1.success({ config: undefined }); + } + },.changeset/itchy-news-design.md (1)
6-6: Tighten wording and include HTTP verb.Clearer, concise phrasing.
-Adds deprecation warning on `livechat:getRoutingConfig`, as well as it adds new endpoint to replace it; `livechat/config/routing` +Deprecates `livechat:getRoutingConfig` and introduces the replacement endpoint: `GET /v1/livechat/config/routing`.apps/meteor/app/livechat/client/lib/stream/queueManager.ts (1)
121-126: Handle fetch failures to avoid subscribing when config is unavailable.If the request fails (e.g., 401/timeout), we currently proceed and subscribe. Safer to bail out.
- const { config } = await sdk.rest.get('/v1/livechat/config/routing'); - if (config?.autoAssignAgent) { - return; - } + try { + const { config } = await sdk.rest.get('/v1/livechat/config/routing'); + if (config?.autoAssignAgent) { + return; + } + } catch { + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/itchy-news-design.md(1 hunks)apps/meteor/app/livechat/client/lib/stream/queueManager.ts(1 hunks)apps/meteor/app/livechat/server/api/v1/config.ts(2 hunks)apps/meteor/app/livechat/server/methods/getRoutingConfig.ts(2 hunks)apps/meteor/client/providers/OmnichannelProvider.tsx(3 hunks)packages/rest-typings/src/v1/omnichannel.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/meteor/app/livechat/client/lib/stream/queueManager.ts (1)
apps/meteor/app/utils/client/lib/SDKClient.ts (1)
sdk(271-271)
apps/meteor/app/livechat/server/api/v1/config.ts (4)
packages/rest-typings/src/v1/omnichannel.ts (1)
GETLivechatConfigRouting(2176-2176)apps/meteor/app/livechat/server/lib/RoutingManager.ts (1)
RoutingManager(73-346)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(71-75)packages/rest-typings/src/index.ts (1)
Endpoints(52-100)
apps/meteor/client/providers/OmnichannelProvider.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)
🔇 Additional comments (2)
apps/meteor/app/livechat/server/api/v1/config.ts (1)
59-64: Typings hookup LGTM.Extending
EndpointsviaExtractRoutesFromAPIis correct and keeps client typings in sync.apps/meteor/client/providers/OmnichannelProvider.tsx (1)
110-114: REST switch and state update look good.Using
useEndpoint('GET', '/v1/livechat/config/routing')with guardedsetRouteConfig(config)is correct; error path is handled.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/meteor/app/livechat/client/lib/stream/queueManager.ts (1)
123-154: Nice improvement: initial request is now handled with try/catch and user feedbackThis addresses the earlier concern about request failures by surfacing a toast and propagating the error. 👍
🧹 Nitpick comments (6)
apps/meteor/app/livechat/client/lib/stream/queueManager.ts (6)
7-7: Normalize error toast payload to avoid “[object Object]” and improve UXEnsure the toast message is always a string; otherwise users may see a generic object printout.
- dispatchToastMessage({ type: 'error', message: error }); + const message = error instanceof Error ? error.message : String(error); + dispatchToastMessage({ type: 'error', message });Also applies to: 151-154
104-110: Duplicate global-listener cleanup (removeGlobalListener and globalCleanup) — call only once
addGlobalListenerreturnsremoveGlobalListener, and the cleanup currently calls bothremoveGlobalListener()andglobalCleanup?.(). Drop one to avoid double stop and potential no-op warnings in the SDK.return () => { useLivechatInquiryStore.getState().discardAll(); - removeGlobalListener(); cleanAgentListener?.(); cleanDepartmentListeners?.(); globalCleanup?.(); departments.clear(); computation.stop(); };Also applies to: 142-150
136-141: Avoid async Tracker.autorun; catch fetch errors inside to prevent unhandled rejectionsTracker doesn’t await async callbacks. Refactor to a sync autorun that spawns the promise and handles errors locally.
- const computation = Tracker.autorun(async () => { - const inquiriesFromAPI = await getInquiriesFromAPI(); - - await updateInquiries(inquiriesFromAPI); - }); + const computation = Tracker.autorun(() => { + void getInquiriesFromAPI() + .then((inquiriesFromAPI) => updateInquiries(inquiriesFromAPI)) + .catch((e) => dispatchToastMessage({ type: 'error', message: e instanceof Error ? e.message : String(e) })); + });
123-128: Type the REST response for compile-time safetyLeverage REST typings so refactors to the endpoint shape are caught at build time.
+import type { GETLivechatConfigRouting } from '@rocket.chat/rest-typings'; … - const { config } = await sdk.rest.get('/v1/livechat/config/routing'); + const { config } = await sdk.rest.get<GETLivechatConfigRouting>('/v1/livechat/config/routing');
125-127: Return a no-op cleanup when auto-assign is enabled to keep API symmetryCallers always receive a function, simplifying upstream logic (even though optional chaining is used).
- if (config?.autoAssignAgent) { - return; - } + if (config?.autoAssignAgent) { + return () => {}; + }
78-81: Avoid shadowing ‘departments’ identifier for readabilityThe parameter name in
addListenerForeachDepartmentshadows the module-leveldepartmentsSet. Rename for clarity.-const addListenerForeachDepartment = (departments: ILivechatDepartment['_id'][] = []) => { - const cleanupFunctions = departments.map((department) => appendListenerToDepartment(department)); +const addListenerForeachDepartment = (departmentIds: ILivechatDepartment['_id'][] = []) => { + const cleanupFunctions = departmentIds.map((departmentId) => appendListenerToDepartment(departmentId));Also applies to: 129-135
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/app/livechat/client/lib/stream/queueManager.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/app/livechat/client/lib/stream/queueManager.ts (3)
apps/meteor/app/utils/client/lib/SDKClient.ts (1)
sdk(271-271)apps/meteor/client/hooks/useLivechatInquiryStore.ts (1)
useLivechatInquiryStore(6-42)apps/meteor/client/lib/toast.ts (1)
dispatchToastMessage(22-25)
⏰ 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). (21)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (1/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (1/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (3/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (2/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (2/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 7.0 (2/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (4/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (3/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (5/5) - Alpine (Official)
- GitHub Check: 🔨 Test API (EE) / MongoDB 5.0 (1/1) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (1/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (2/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (5/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (3/5) - Alpine (Official)
- GitHub Check: 🔨 Test API (CE) / MongoDB 5.0 (1/1) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (1/5) - Alpine (Official)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 7.0 [legacy watchers] (4/5) - Alpine (Official)
- GitHub Check: 🔨 Test API (CE) / MongoDB 7.0 (1/1) - Alpine (Official)
lucas-a-pelegrino
added
stat: ready to merge
Proposed changes (including videos or screenshots)
This PR adds a deprecation warning to
livechat:getRoutingConfig, and adds a new endpoint that replaces the deprecated method with thelivechat/config/routingpath.Issue(s)
CTZ-56
Steps to test or reproduce
Further comments
Tests will be implemented on this task
Summary by CodeRabbit