-
Notifications
You must be signed in to change notification settings - Fork 13.1k
chore: adds deprecation warning for livechat:removeBusinessHour and new endpoint to replace it
#37775
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
…e/CORE-1413 # Conflicts: # apps/meteor/app/livechat/imports/server/rest/businessHours.ts # packages/rest-typings/src/v1/omnichannel.ts
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: f276a21 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 |
WalkthroughThis pull request introduces a new REST API endpoint ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
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 #37775 +/- ##
========================================
Coverage 67.70% 67.71%
========================================
Files 3452 3452
Lines 113975 113975
Branches 20940 20940
========================================
+ Hits 77168 77173 +5
- Misses 34678 34679 +1
+ Partials 2129 2123 -6
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: 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 (6)
.changeset/weak-frogs-relax.md(1 hunks)apps/meteor/app/livechat/imports/server/rest/businessHours.ts(2 hunks)apps/meteor/client/views/omnichannel/businessHours/useRemoveBusinessHour.tsx(1 hunks)apps/meteor/ee/app/livechat-enterprise/server/methods/removeBusinessHour.ts(2 hunks)apps/meteor/tests/data/livechat/businessHours.ts(2 hunks)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:
apps/meteor/ee/app/livechat-enterprise/server/methods/removeBusinessHour.tsapps/meteor/tests/data/livechat/businessHours.tspackages/rest-typings/src/v1/omnichannel.tsapps/meteor/client/views/omnichannel/businessHours/useRemoveBusinessHour.tsxapps/meteor/app/livechat/imports/server/rest/businessHours.ts
🧠 Learnings (2)
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/ee/app/livechat-enterprise/server/methods/removeBusinessHour.ts
📚 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/ee/app/livechat-enterprise/server/methods/removeBusinessHour.ts.changeset/weak-frogs-relax.md
🧬 Code graph analysis (3)
apps/meteor/tests/data/livechat/businessHours.ts (1)
apps/meteor/tests/data/api-data.ts (2)
request(10-10)credentials(39-42)
apps/meteor/client/views/omnichannel/businessHours/useRemoveBusinessHour.tsx (2)
packages/ui-contexts/src/index.ts (3)
useTranslation(81-81)useSetModal(71-71)useToastMessageDispatch(78-78)packages/mock-providers/src/MockedAppRootBuilder.tsx (1)
queryClient(270-280)
apps/meteor/app/livechat/imports/server/rest/businessHours.ts (1)
packages/rest-typings/src/v1/omnichannel.ts (4)
POSTLivechatBusinessHoursSaveSuccessResponse(3412-3412)isPOSTLivechatBusinessHoursSaveParams(3396-3398)POSTLivechatBusinessHoursRemoveSuccessResponse(3449-3449)isPOSTLivechatBusinessHoursRemoveParams(3433-3435)
🔇 Additional comments (13)
.changeset/weak-frogs-relax.md (1)
1-6: LGTM!The changeset accurately documents the deprecation of the old method and the introduction of the new REST endpoint. The patch version bump is appropriate for this type of backward-compatible change.
apps/meteor/ee/app/livechat-enterprise/server/methods/removeBusinessHour.ts (2)
5-5: LGTM!The deprecation logger import is correctly added to support the deprecation warning.
16-17: LGTM!The deprecation warning is correctly placed at the beginning of the method execution, before any business logic. This ensures all callers of the deprecated method receive the warning. The version target (8.0.0) and replacement endpoint path are clearly specified.
apps/meteor/tests/data/livechat/businessHours.ts (2)
6-6: LGTM!The
methodCallimport is correctly removed as it's no longer needed after migrating to the REST endpoint.
134-140: LGTM!The test helper correctly migrates from the deprecated
methodCallapproach to the new REST endpoint. The payload structure{ _id, type }matches thePOSTLivechatBusinessHoursRemoveParamsschema defined in rest-typings.apps/meteor/client/views/omnichannel/businessHours/useRemoveBusinessHour.tsx (3)
3-3: LGTM!The import is correctly updated from
useMethodtouseEndpointto support the new REST-based approach.
11-11: LGTM!The hook correctly uses
useEndpointwith the POST method and the new endpoint path. This aligns with the server-side route definition.
17-17: LGTM!The call signature is correctly updated from positional arguments
(_id, type)to an object payload({ _id, type }), matching the REST endpoint's expected body schema.packages/rest-typings/src/v1/omnichannel.ts (2)
3414-3435: LGTM!The type definition and JSON schema for
POSTLivechatBusinessHoursRemoveParamsare well-structured and follow the existing patterns in the codebase. Both required fields (_idandtype) are correctly specified withadditionalProperties: falsefor strict validation.
3437-3449: LGTM!The success response schema follows the established pattern used by other endpoints in this file (e.g.,
POSTLivechatBusinessHoursSaveSuccessSchema). Therequired: ['success']constraint ensures the response is properly validated.apps/meteor/app/livechat/imports/server/rest/businessHours.ts (3)
5-6: LGTM!The imports for the new validation schema and success response type are correctly added.
31-50: LGTM!The save endpoint is correctly refactored to use the method chaining pattern with explicit response schemas, authentication requirement, and body validation. The existing functionality is preserved.
72-77: LGTM!The type extraction and module augmentation correctly extend the
Endpointsinterface with the new routes, ensuring type safety for clients using these endpoints.
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!
… new endpoint to replace it (#37775)
Proposed changes (including videos or screenshots)
This PR adds deprecation warning for
livechat:removeBusinessHourand new endpoint to replace it;livechat/business-hours.removeIssue(s)
CORE-1413
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.