-
Notifications
You must be signed in to change notification settings - Fork 13k
chore: adds a deprecation warning for livechat:saveBusinessHour
#37690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 05700ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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 |
WalkthroughDeprecates the Meteor method Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant REST as POST /v1/livechat/business-hours.save
participant Method as Meteor method livechat:saveBusinessHour
participant Manager as businessHourManager
participant DB as Database
rect rgb(240,255,240)
Note over Client,DB: New preferred flow
Client->>REST: POST validated payload
REST->>REST: AJV validate request
REST->>Manager: saveBusinessHour(payload as ILivechatBusinessHour)
Manager->>DB: persist business hour
DB-->>Manager: OK
Manager-->>REST: return saved business hour
REST-->>Client: 200 + result
end
rect rgb(255,240,240)
Note over Client,Method: Legacy (deprecated) flow
Client->>Method: Call with payload
Method->>Method: Log deprecation (methodDeprecationLogger)
Method->>Manager: saveBusinessHour(payload)
Manager->>DB: persist business hour
DB-->>Manager: OK
Manager-->>Method: return saved business hour
Method-->>Client: return result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37690 +/- ##
========================================
Coverage 67.70% 67.70%
========================================
Files 3452 3452
Lines 113975 113975
Branches 20940 20940
========================================
+ Hits 77168 77171 +3
+ Misses 34678 34676 -2
+ Partials 2129 2128 -1
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
3214-3336: Business-hours save params/schemas look correct; consider exposing the new endpoint inOmnichannelEndpoints.
BusinessHourDaysTime,BusinessHourWorkHours, andPOSTLivechatBusinessHoursSaveParamsare consistent withPOSTLivechatBusinessHoursSaveParamsSchema(field names, required flags, and nested object shapes all align), and the success schema follows existing patterns, so no blocking issues from a typing/validation standpoint. One thing to double-check is that the newlivechat/business-hours.saveroute is actually represented in theOmnichannelEndpointsmap in this file (similar to the existing/v1/livechat/business-hourGET entry); otherwise, hooks likeuseEndpoint('POST', '/v1/livechat/business-hours.save')will not be fully type-safe even though these validators exist. Not commenting on the TODO itself per maintainers’ guidance. Based on learnings.
📜 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 (9)
.changeset/brown-llamas-worry.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(3 hunks)apps/meteor/tests/data/livechat/businessHours.ts(5 hunks)apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts(0 hunks)apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts(1 hunks)apps/meteor/tests/end-to-end/api/livechat/19-business-hours.ts(8 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)
**/*.{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/app/livechat/server/methods/saveBusinessHour.tsapps/meteor/tests/data/livechat/businessHours.tsapps/meteor/tests/e2e/utils/omnichannel/businessHours.tsapps/meteor/tests/end-to-end/api/livechat/19-business-hours.tspackages/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/businessHours.tsapps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/livechat/server/methods/saveBusinessHour.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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/data/livechat/businessHours.tsapps/meteor/tests/e2e/utils/omnichannel/businessHours.tsapps/meteor/tests/end-to-end/api/livechat/19-business-hours.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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.tsapps/meteor/tests/end-to-end/api/livechat/19-business-hours.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 : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.tsapps/meteor/tests/end-to-end/api/livechat/19-business-hours.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 : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.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/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/utils/omnichannel/businessHours.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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/19-business-hours.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/livechat/19-business-hours.ts
🧬 Code graph analysis (3)
apps/meteor/tests/end-to-end/api/livechat/19-business-hours.ts (1)
apps/meteor/tests/data/livechat/businessHours.ts (1)
saveBusinessHour(13-17)
apps/meteor/app/livechat/imports/server/rest/businessHours.ts (3)
packages/rest-typings/src/v1/omnichannel.ts (2)
POSTLivechatBusinessHoursSaveSuccessResponse(3336-3336)isPOSTLivechatBusinessHoursSaveParams(3320-3322)packages/rest-typings/src/v1/Ajv.ts (2)
validateBadRequestErrorResponse(47-47)validateUnauthorizedErrorResponse(70-70)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(73-77)
apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx (1)
apps/meteor/tests/data/livechat/businessHours.ts (1)
saveBusinessHour(13-17)
🔇 Additional comments (10)
apps/meteor/app/livechat/server/methods/saveBusinessHour.ts (1)
5-5: LGTM!The deprecation warning is correctly added at the method entry point, providing the method name, target deprecation version, and the replacement endpoint path. This follows proper deprecation patterns.
Also applies to: 17-17
.changeset/brown-llamas-worry.md (1)
1-6: LGTM!The changeset correctly identifies the affected packages (
@rocket.chat/meteorand@rocket.chat/rest-typings) with patch-level changes and provides a clear description of the deprecation and new endpoint.apps/meteor/client/views/omnichannel/businessHours/EditBusinessHours.tsx (1)
42-42: LGTM!The migration from the deprecated Meteor method to the new REST endpoint is correctly implemented. The
useEndpointhook provides proper type inference for the payload, eliminating the need foras anycasts.Also applies to: 58-72
apps/meteor/tests/e2e/utils/omnichannel/businessHours.ts (1)
13-35: Payload structure aligns with the new endpoint schema.The payload correctly includes both
daysTime(with nested time objects) andworkHours(with flat strings), matching the expectedPOSTLivechatBusinessHoursSaveParamsstructure.apps/meteor/tests/end-to-end/api/livechat/19-business-hours.ts (2)
52-58: LGTM!The test cleanup pattern correctly removes
_updatedAtandtsproperties before sending to the new endpoint, and the payload structure withtimezoneas a string andtimezoneNameis consistent across test cases.Also applies to: 91-104
182-210: LGTM!The custom business hour creation test correctly uses the new payload structure with
daysOpen,daysTime, andworkHoursfields, matching the new endpoint schema.apps/meteor/tests/data/livechat/businessHours.ts (3)
13-16: LGTM!The
saveBusinessHourfunction correctly uses the newPOSTLivechatBusinessHoursSaveParamstype and calls the new REST endpoint.
19-51: LGTM!The
createCustomBusinessHourfunction correctly constructs the payload with the new schema structure includingdaysOpen,daysTime, andworkHoursfields.
64-92: LGTM!The cleanup pattern to remove
_updatedAtandtsbefore constructing the payload is correctly applied, and the payload structure aligns with the new endpoint schema.apps/meteor/app/livechat/imports/server/rest/businessHours.ts (1)
50-55: LGTM!The type extraction using
ExtractRoutesFromAPIand module augmentation to extend theEndpointsinterface follows the established pattern in the codebase, ensuring the new endpoint is properly typed across the system.
dougfabris
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontend looks good to me!
c4433f6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 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(1 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:
packages/rest-typings/src/v1/omnichannel.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
🧬 Code graph analysis (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
packages/rest-typings/src/v1/Ajv.ts (1)
ajv(24-24)
⏰ 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
KevLehman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hoping CI passes first try ![]()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
3299-3301: Mark optional field as nullable in schema.The
timezoneproperty is optional in the TypeScript type definition (line 3238:timezone?: string) but the schema doesn't mark it asnullable: true. This creates a mismatch between the type system and runtime validation.Apply this diff to align the schema with the type definition:
timezone: { type: 'string', + nullable: true, },
📜 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(1 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:
packages/rest-typings/src/v1/omnichannel.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/omnichannel.ts
🧬 Code graph analysis (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
packages/rest-typings/src/v1/Ajv.ts (1)
ajv(24-24)
⏰ 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)
3229-3240: LGTM! Type definition is well-structured.The type correctly defines optional and required fields for the business hour save operation, supporting both the legacy
daysTimeformat and the newworkHoursformat during the transition period.
3326-3338: LGTM! Success response schema follows established patterns.The success response schema correctly defines a simple boolean success response and follows the same pattern used throughout this file for similar endpoints.
Proposed changes (including videos or screenshots)
This PR adds a deprecation warning for
livechat:saveBusinessHour, as well as a new endpoint (livechat/business-hours.save) to replace the legacy method.Issue(s)
CORE-1550
CORE-1551
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Deprecations
Tests
✏️ Tip: You can customize this high-level summary in your review settings.