chore: Adds deprecation warning on removeCannedResponse
#37051
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: f71fbf4 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 |
WalkthroughMoves canned-response deletion from a body-based Meteor method to a RESTful DELETE /v1/canned-responses/:_id endpoint; refactors client to use a new useRemoveCannedResponse(id) hook, adds RemoveCannedResponseButton, wires onDelete props through edit components, logs deprecation for the old method, and updates E2E tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Client UI (Edit/Table)
participant Hook as useRemoveCannedResponse(id)
participant API as DELETE /v1/canned-responses/:_id
participant S as Server Handler
participant Store as Client Cache / Router
U->>UI: Click "Delete"
UI->>Hook: onDelete()
Note right of Hook #DDEBF7: confirm modal shown, then proceed
Hook->>API: DELETE /v1/canned-responses/:_id
API->>S: validate auth & permissions
S-->>API: 200 success
API-->>Hook: success
Hook->>Store: invalidate getCannedResponses, show toast, navigate list
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
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (7)
🔇 Additional comments (1)
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❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37051 +/- ##
===========================================
+ Coverage 67.31% 67.35% +0.03%
===========================================
Files 3337 3327 -10
Lines 113294 113224 -70
Branches 20562 20574 +12
===========================================
- Hits 76266 76262 -4
+ Misses 34422 34355 -67
- 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: 2
🧹 Nitpick comments (4)
apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
21-26: Also invalidate the detail query to prevent stale cachesWhen deleting from the edit view, invalidate the per-id cache too.
await removeCannedResponse(); queryClient.invalidateQueries({ queryKey: ['getCannedResponses'], }); + queryClient.invalidateQueries({ + queryKey: ['getCannedResponseById', id], + }); router.navigate('/omnichannel/canned-responses'); dispatchToastMessage({ type: 'success', message: t('Canned_Response_Removed') });apps/meteor/tests/end-to-end/api/livechat/15-canned-responses.ts (1)
255-258: Rename test to reflect path-based 404The title references body param; now it’s 404 due to missing path param.
-it('should fail if _id is not on the request', async () => { +it('should 404 when DELETE /canned-responses has no :_id path parameter', async () => { await updatePermission('remove-canned-responses', ['livechat-agent', 'livechat-monitor', 'livechat-manager', 'admin']); return request.delete(api('canned-responses')).set(credentials).expect(404); });apps/meteor/client/omnichannel/cannedResponses/RemoveCannedResponseButton.tsx (1)
15-23: Add aria-label for accessibilityProvide an explicit accessible name for the icon button.
- <IconButton + <IconButton icon='trash' small + aria-label={t('Remove')} title={t('Remove')} onClick={(e) => { e.stopPropagation(); handleDelete(); }} />apps/meteor/client/omnichannel/cannedResponses/CannedResponseEdit.tsx (1)
81-86: Guard Delete button rendering when onDelete is not providedPrevents a runtime error if a caller omits onDelete.
- {cannedResponseData?._id && ( + {cannedResponseData?._id && onDelete && ( <ButtonGroup> <Button danger onClick={onDelete}> {t('Delete')} </Button> </ButtonGroup> )}
📜 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 (10)
.changeset/late-papayas-swim.md(1 hunks)apps/meteor/client/omnichannel/cannedResponses/CannedResponseEdit.tsx(3 hunks)apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithData.tsx(2 hunks)apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithDepartmentData.tsx(2 hunks)apps/meteor/client/omnichannel/cannedResponses/CannedResponsesTable.tsx(2 hunks)apps/meteor/client/omnichannel/cannedResponses/RemoveCannedResponseButton.tsx(1 hunks)apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx(1 hunks)apps/meteor/ee/app/api-enterprise/server/canned-responses.ts(5 hunks)apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts(2 hunks)apps/meteor/tests/end-to-end/api/livechat/15-canned-responses.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
apps/meteor/client/omnichannel/cannedResponses/RemoveCannedResponseButton.tsx (1)
apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
useRemoveCannedResponse(8-45)
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEdit.tsx (1)
packages/livechat/src/components/Button/index.tsx (1)
Button(34-99)
apps/meteor/tests/end-to-end/api/livechat/15-canned-responses.ts (2)
apps/meteor/tests/data/api-data.ts (2)
request(10-10)credentials(39-42)apps/meteor/tests/data/livechat/canned-responses.ts (1)
createCannedResponse(7-25)
apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithData.tsx (2)
packages/ui-contexts/src/index.ts (1)
useEndpoint(32-32)apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
useRemoveCannedResponse(8-45)
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithDepartmentData.tsx (1)
apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
useRemoveCannedResponse(8-45)
apps/meteor/ee/app/api-enterprise/server/canned-responses.ts (1)
packages/rest-typings/src/v1/omnichannel.ts (2)
isPOSTCannedResponsesProps(2883-2883)isCannedResponsesProps(998-998)
🔇 Additional comments (7)
.changeset/late-papayas-swim.md (1)
1-5: Changelog entry looks good.Patch bump and deprecation note are captured clearly.
apps/meteor/client/omnichannel/cannedResponses/useRemoveCannedResponse.tsx (1)
15-15: LGTM: using DELETE /v1/canned-responses/:_idEndpoint binding with path params is correct.
apps/meteor/client/omnichannel/cannedResponses/CannedResponsesTable.tsx (1)
171-171: LGTM: centralized delete via RemoveCannedResponseButtonGood consolidation and event propagation stop handled in the button.
apps/meteor/tests/end-to-end/api/livechat/15-canned-responses.ts (2)
253-253: LGTM: unauthorized delete now targets path-based endpoint403 expectation remains correct.
262-265: LGTM: switched to DELETE /canned-responses/:_idMatches new API contract and keeps response assertions intact.
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithData.tsx (1)
39-39: LGTM: delegate delete via onDeleteWires the new hook through props cleanly.
apps/meteor/ee/app/api-enterprise/server/canned-responses.ts (1)
112-135: LGTM!Splitting the permissions per verb and moving the delete handler onto the id-scoped route aligns with the updated endpoint contract without introducing regressions.
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithDepartmentData.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/omnichannel/cannedResponses/CannedResponseEditWithDepartmentData.tsx
Outdated
Show resolved
Hide resolved
| ); | ||
| return API.v1.success(); | ||
| }, | ||
| async delete() { |
There was a problem hiding this comment.
you cannot remove this, it would be a breaking change.. what you could do is also deprecate this one in favor of the new one you created.. but I'd recommend you just leave this one here and use it even though it doesn't follow any standards :/
Proposed changes (including videos or screenshots)
This PR adds a deprecation warning for
removeCannedResponsemeteor method, as well as it updatesuseRemoveCannedResponseto use corresponding endpoint.Issue(s)
CTZ-72
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Deprecation
Chores