chore: Adds deprecation warning on livechat:saveBusinessHour with new endpoint replacing it
#37029
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 63e4b89 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 an authenticated REST endpoint POST /v1/livechat/business-hours.save, updates client and tests to call it, logs deprecation for the legacy DDP method, introduces ISaveLivechatBusinessHour payload type, updates manager/abstract signatures and validators, and adds REST typings and a changeset. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Client UI (EditBusinessHours)
participant API as REST API /v1/livechat/business-hours.save
participant SVC as businessHourManager
participant DB as Data Store
User->>UI: Save business hours
UI->>API: POST /v1/livechat/business-hours.save (ISaveLivechatBusinessHour)
API->>SVC: saveBusinessHour(params)
SVC->>DB: Upsert business hour (timezone string & workHours entries)
DB-->>SVC: Upsert result
SVC-->>API: Success / Error
alt Success
API-->>UI: { success: true } (200)
else Error
API-->>UI: { success: false, error } (4xx)
end
sequenceDiagram
autonumber
participant UI as Client UI
participant RPC as DDP Method `livechat:saveBusinessHour` (Deprecated)
participant API as REST `/v1/livechat/business-hours.save`
rect #fff4f0
note over RPC: Old flow (deprecated)
UI->>RPC: DDP method call
RPC->>RPC: Log deprecation metadata, call manager
RPC-->UI: Result (still supported)
end
rect #f0fff7
note over API: New flow
UI->>API: POST business-hour payload
API-->UI: { success: true }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37029 +/- ##
===========================================
- Coverage 67.34% 67.26% -0.08%
===========================================
Files 3339 3340 +1
Lines 113178 113357 +179
Branches 20535 20570 +35
===========================================
+ Hits 76217 76251 +34
- Misses 34355 34499 +144
- Partials 2606 2607 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.changeset/warm-ants-thank.md (1)
6-7: Fix typo and clarify endpoint path in changeset entryThe endpoint name has a typo and lacks the HTTP method and versioned prefix.
Apply this diff:
-Adds deprecation warning on `livechat:saveBusinessHour` with new endpoint replacing it; `livechat/business-houra.save` +Adds a deprecation warning for `livechat:saveBusinessHour` and introduces the replacement endpoint `POST /v1/livechat/business-hours.save`.apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts (1)
13-35: Payload shape OK; consider trimming to required fieldsSchema doesn’t require
daysOpen/daysTimeifworkHoursis present. Keeping only required fields reduces test fragility.
📜 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 (7)
.changeset/warm-ants-thank.md(1 hunks)apps/meteor/app/livechat/imports/server/rest/businessHours.ts(2 hunks)apps/meteor/app/livechat/server/methods/saveBusinessHour.ts(2 hunks)apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx(2 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts(0 hunks)apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts(1 hunks)packages/rest-typings/src/v1/omnichannel.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts
🧬 Code graph analysis (3)
apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)
apps/meteor/app/livechat/imports/server/rest/businessHours.ts (4)
packages/rest-typings/src/v1/omnichannel.ts (2)
POSTLivechatBusinessHoursSaveSuccessResponse(3150-3150)isPOSTLivechatBusinessHoursSaveParams(3139-3139)packages/rest-typings/src/v1/Ajv.ts (2)
validateBadRequestErrorResponse(46-46)validateUnauthorizedErrorResponse(69-69)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(73-77)packages/rest-typings/src/index.ts (1)
Endpoints(52-100)
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts (1)
apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts (1)
departmentIds(20-43)
🔇 Additional comments (3)
apps/meteor/app/livechat/imports/server/rest/businessHours.ts (2)
51-56: Typing augmentation LGTMModule augmentation to publish the new endpoint in
Endpointsis correct.
28-38: Require management permission for saving business hoursPOST "livechat/business-hours.save" currently only has authRequired; restrict it by adding permissionsRequired: ['manage-livechat-business-hours'] in apps/meteor/app/livechat/imports/server/rest/businessHours.ts (≈ lines 28–38). I did not find an existing 'manage-livechat-business-hours' permission—declare it in apps/meteor/app/authorization/server/constant/permissions.ts (and add i18n) or choose an appropriate existing manage permission and update tests/EE equivalents.
apps/meteor/app/livechat/server/methods/saveBusinessHour.ts (1)
5-5: Good deprecation signal — confirm external callers migrated'8.0.0' is consistent with other method deprecations. ripgrep finds only the method definition at apps/meteor/app/livechat/server/methods/saveBusinessHour.ts and no other uses in apps/meteor. Verify web/mobile/third‑party callers and integrations have switched to /v1/livechat/business-hours.save before removing the method in 8.0.0.
apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
📜 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 (5)
apps/meteor/app/livechat/server/business-hour/AbstractBusinessHour.ts(1 hunks)apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts(1 hunks)apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx(3 hunks)apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts(1 hunks)packages/rest-typings/src/v1/omnichannel.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts
🧬 Code graph analysis (3)
apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts (1)
apps/meteor/app/livechat/server/methods/saveBusinessHour.ts (1)
businessHourData(16-23)
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts (2)
apps/meteor/app/livechat/server/methods/removeAllClosedRooms.ts (1)
departmentIds(20-43)apps/meteor/app/livechat/server/methods/getDepartmentForwardRestrictions.ts (1)
departmentId(15-27)
packages/rest-typings/src/v1/omnichannel.ts (1)
packages/rest-typings/src/v1/Ajv.ts (1)
ajv(23-23)
🔇 Additional comments (5)
packages/rest-typings/src/v1/omnichannel.ts (1)
3165-3165: LGTM!The AJV compilation correctly uses the new dedicated parameter type
POSTLivechatBusinessHoursSaveParamsinstead of the mismatchedILivechatBusinessHour. This resolves the type mismatch issue mentioned in the past review comment.apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts (1)
106-106: LGTM!The method signature change from
ILivechatBusinessHourtoPartial<ILivechatBusinessHour>correctly supports the new REST endpoint requirements while maintaining backward compatibility. This aligns with the new endpoint accepting partial business hour data.apps/meteor/app/livechat/server/business-hour/AbstractBusinessHour.ts (1)
32-32: LGTM!The interface change from
ILivechatBusinessHourtoPartial<ILivechatBusinessHour>correctly supports partial updates while maintaining return type consistency. This change propagates properly through the inheritance hierarchy.apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts (2)
10-11: LGTM!The fix correctly extracts
departmentIdfrom the object array before joining, resolving the issue wherejoin(',')was being called on objects that would stringify to"[object Object]". The parameter cleanup (removing unusedid) also aligns with the new server-generated ID approach.
13-35: Endpoint path is correct — no change needed. The POST route is registered as API.v1.post('livechat/business-hours.save') in apps/meteor/app/livechat/imports/server/rest/businessHours.ts and is referenced by the frontend (useEndpoint('POST','/v1/livechat/business-hours.save')) and tests (api.post('/livechat/business-hours.save')).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts (1)
106-114: Ensure every caller sends the flattened ISaveLivechatBusinessHour (timezone as string; start/finish as strings)REST endpoint, UI and the v1 API callers already send the flattened shape, but DDP/Meteor test callers still forward full timezone objects — update those callers or normalize on the server before delegating to type.saveBusinessHour.
- apps/meteor/tests/data/livechat/businessHours.ts — saveBusinessHour() forwards whatever businessHour it receives to the 'livechat:saveBusinessHour' DDP method.
- apps/meteor/tests/end-to-end/api/livechat/19-business-hours.ts — calls (including the after() teardown) pass timezone objects (no timezoneName) in some places.
Either update those test helpers/calls to send timezone/timezoneName + workHours with string start/finish, or add server-side normalization before calling type.saveBusinessHour.
🧹 Nitpick comments (1)
apps/meteor/tests/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts (1)
192-205: Test doesn’t exercise “invalid type” pathYou stub
getBusinessHourTypeto return a type even whenbusinessHour.typeis 'invalid', so the “invalid type” branch is not covered.Update the stub in this test to
returns(undefined as any)so the function early-returns without logging, actually validating the scenario.
📜 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 (5)
apps/meteor/app/livechat/server/business-hour/AbstractBusinessHour.ts(2 hunks)apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts(3 hunks)apps/meteor/tests/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts(1 hunks)packages/core-typings/src/ILivechatBusinessHour.ts(1 hunks)packages/rest-typings/src/v1/omnichannel.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/app/livechat/server/business-hour/AbstractBusinessHour.ts
- packages/rest-typings/src/v1/omnichannel.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
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/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts
🧬 Code graph analysis (1)
apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts (2)
apps/meteor/app/livechat/server/methods/saveBusinessHour.ts (1)
businessHourData(16-23)packages/core-typings/src/ILivechatBusinessHour.ts (1)
ISaveLivechatBusinessHour(44-65)
⏰ 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 (3)
apps/meteor/tests/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts (1)
161-165: LGTM: updated payload shape in fixtureThe test data matches the new flattened payload (timezone string + day-aware workHours).
apps/meteor/app/livechat/server/business-hour/BusinessHourManager.ts (1)
1-1: LGTM: import updatesImporting
ISaveLivechatBusinessHourhere is consistent with the new save contract.packages/core-typings/src/ILivechatBusinessHour.ts (1)
44-65: LGTM — departmentsToApplyBusinessHour is a comma-separated stringVerified: REST typing and code treat it as a string (packages/rest-typings/src/v1/omnichannel.ts:3137); client builds the value with .join(',') (apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx) and server code parses it with .split(',') (apps/meteor/ee/app/livechat-enterprise/server/business-hour/Multiple.ts, Custom.ts). No change required.
apps/meteor/tests/unit/app/livechat/server/business-hour/BusinessHourManager.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
3143-3144: Optional: export a named params type for consistencyMany endpoints expose a
...Paramsalias; consider exporting one for ergonomics.Example:
export type POSTLivechatBusinessHoursSaveParams = ISaveLivechatBusinessHour; // (keep ajv.compile generic as this alias if preferred)
📜 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)
packages/rest-typings/src/v1/omnichannel.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
packages/core-typings/src/ILivechatBusinessHour.ts (1)
ISaveLivechatBusinessHour(44-65)
⏰ 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)
packages/rest-typings/src/v1/omnichannel.ts (2)
3086-3088: Good: TS/Schema alignment fortypeenumEnum narrowed to 'default' | 'custom' in schema, matching TS expectations.
3145-3154: Success response schema OK
{ success: true }compiled asvoidmatches existing conventions here.
| type: 'object', | ||
| properties: { | ||
| time: { type: 'string' }, | ||
| }, | ||
| }, | ||
| finish: { | ||
| type: 'object', | ||
| properties: { | ||
| time: { type: 'string' }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Require time in daysTime.start/finish and lock shape
start/finish objects don't require time and allow extraneous keys. This permits invalid payloads to pass AJV while TS expects { time: string }.
Apply this diff:
start: {
type: 'object',
properties: {
time: { type: 'string' },
},
+ required: ['time'],
+ additionalProperties: false,
},
finish: {
type: 'object',
properties: {
time: { type: 'string' },
},
+ required: ['time'],
+ additionalProperties: false,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type: 'object', | |
| properties: { | |
| time: { type: 'string' }, | |
| }, | |
| }, | |
| finish: { | |
| type: 'object', | |
| properties: { | |
| time: { type: 'string' }, | |
| }, | |
| }, | |
| type: 'object', | |
| properties: { | |
| time: { type: 'string' }, | |
| }, | |
| required: ['time'], | |
| additionalProperties: false, | |
| }, | |
| finish: { | |
| type: 'object', | |
| properties: { | |
| time: { type: 'string' }, | |
| }, | |
| required: ['time'], | |
| additionalProperties: false, | |
| }, |
🤖 Prompt for AI Agents
In packages/rest-typings/src/v1/omnichannel.ts around lines 3103 to 3113, the
schema for daysTime.start and daysTime.finish currently defines time as an
optional property and allows extraneous keys; update both objects to require the
time property by adding required: ['time'] and prevent extra properties by
adding additionalProperties: false so the AJV validation matches the TypeScript
type { time: string } and rejects payloads with missing or extra fields.
lucas-a-pelegrino
changed the title
livechat:saveBusinessHour with new endpoint replacing it
Proposed changes (including videos or screenshots)
This PR adds a deprecation warning for
livechat:saveBusinessHourmeteor method, as well as it adds an endpoint (livechat/business-hours.save) to replace it.Issue(s)
CTZ-63
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Deprecations
Improvements
Tests
Chores