Skip to content

Conversation

@juliajforesti
Copy link
Contributor

@juliajforesti juliajforesti commented Nov 7, 2025

ARCH-1872

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Breaking Changes
    • Removed removeCannedResponse from the public API. Applications using this method will require updates for compatibility with this major version.

@juliajforesti juliajforesti added this to the 8.0.0 milestone Nov 7, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 7, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 7, 2025

🦋 Changeset detected

Latest commit: 0d010aa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 40 packages
Name Type
@rocket.chat/meteor Major
@rocket.chat/core-typings Major
@rocket.chat/rest-typings Major
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

This PR completes the deprecation cycle by removing the public DDP-method exposure and ServerMethods augmentation for removeCannedResponse. The internal function remains available for internal use; only the remote invocation wrapper and deprecation logger are eliminated.

Changes

Cohort / File(s) Change Summary
Deprecation removal
apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts
Removed Meteor.methods DDP wrapper, ServerMethods interface augmentation for @rocket.chat/ddp-client, and deprecation logger imports. Core function logic preserved.
Changeset documentation
.changeset/smooth-birds-crash.md
Documents major version bump for @rocket.chat/meteor package and removal of deprecated removeCannedResponse from public API.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that the DDP method removal is complete and no external references remain
  • Confirm the internal removeCannedResponse function is still available for internal callers

Possibly related PRs

  • RocketChat/Rocket.Chat#37051: Added deprecation warning on the Meteor method and client hook changes; this PR removes the deprecated exposure entirely.
  • RocketChat/Rocket.Chat#37392: Removes other deprecated server-exposed Meteor methods and their ServerMethods augmentations using the same removal pattern.

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • dougfabris
  • tassoevan
  • sampaiodiego

Poem

🐰 Deprecated methods fade away,
Public APIs removed today,
Internal logic stays so bright,
Breaking changes done just right!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removal of the deprecated removeCannedResponse method, matching the changeset contents.
Linked Issues check ✅ Passed The linked issue ARCH-1872 references removing the deprecated removeCannedResponse method. The PR successfully removes the DDP method exposure and public API interface while maintaining the internal function.
Out of Scope Changes check ✅ Passed All changes are directly related to the objective of removing the deprecated removeCannedResponse method from the public API, with no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/remove-removeCannedResponse

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.50%. Comparing base (d1adf9e) to head (0d010aa).
⚠️ Report is 4 commits behind head on release-8.0.0.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           release-8.0.0   #37446      +/-   ##
=================================================
- Coverage          70.50%   70.50%   -0.01%     
=================================================
  Files               2870     2870              
  Lines             102098   102096       -2     
  Branches           17589    17587       -2     
=================================================
- Hits               71984    71982       -2     
- Misses             28356    28357       +1     
+ Partials            1758     1757       -1     
Flag Coverage Δ
unit 72.27% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@juliajforesti juliajforesti marked this pull request as ready for review November 7, 2025 20:39
@juliajforesti juliajforesti requested a review from a team as a code owner November 7, 2025 20:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a8bcf20 and 0d010aa.

📒 Files selected for processing (2)
  • .changeset/smooth-birds-crash.md (1 hunks)
  • apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts
🔇 Additional comments (1)
.changeset/smooth-birds-crash.md (1)

1-5: Changeset metadata structure and versioning are correct.

The major version bump is semantically appropriate for removing a public API method, which is a breaking change. The description clearly communicates the deprecation removal.

Comment on lines +1 to +5
---
'@rocket.chat/meteor': major
---

Removes deprecated `removeCannedResponse` method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify actual implementation changes to confirm deprecation removal was completed.

This changeset file appears valid, but I can only review the metadata. To ensure the PR objectives are fully met, please confirm:

  1. The DDP-method exposure (Meteor.methods wrapper) for removeCannedResponse was actually removed.
  2. The public ServerMethods augmentation in the ddp-client module was removed.
  3. The internal removeCannedResponse function implementation remains intact.
  4. No orphaned imports or deprecation logger references remain.
  5. No references to the removed public API remain in other files.

The related TypeScript files mentioned in the summary (e.g., apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts) were not provided in this review, so I cannot directly verify these changes.


🏁 Script executed:

#!/bin/bash
# Verify that removeCannedResponse was properly removed from public API

# Check 1: Search for any remaining Meteor.methods references to removeCannedResponse
echo "=== Checking for remaining Meteor.methods references ==="
rg -nA 3 "removeCannedResponse" apps/meteor/ee/app/canned-responses/ --type ts

# Check 2: Search for ServerMethods augmentation references
echo "=== Checking for ServerMethods augmentation ==="
rg -nA 2 "ServerMethods.*removeCannedResponse|removeCannedResponse.*ServerMethods" --type ts

# Check 3: Verify internal function still exists
echo "=== Verifying internal removeCannedResponse function exists ==="
rg -nB 2 -A 5 "export.*function.*removeCannedResponse|export async function removeCannedResponse" apps/meteor/ee/app/canned-responses/ --type ts

Length of output: 1639


🏁 Script executed:

#!/bin/bash
# More comprehensive verification of removeCannedResponse removal

# Check 1: Search for Meteor.methods registration across entire codebase
echo "=== Checking for Meteor.methods registration ==="
rg -n "Meteor\.methods\s*\(" --type ts --type js -A 15 | grep -C 5 -i "removeCannedResponse" || echo "No Meteor.methods with removeCannedResponse found"

# Check 2: Search for all imports of removeCannedResponse
echo ""
echo "=== Checking for imports of removeCannedResponse ==="
rg -n "import.*removeCannedResponse|from.*removeCannedResponse" --type ts --type js

# Check 3: Search in ddp-client for ServerMethods augmentation
echo ""
echo "=== Checking ddp-client for ServerMethods type ==="
fd -e ts -e tsx -path "*ddp-client*" | head -20

# Check 4: Look for ServerMethods interface/type definition
echo ""
echo "=== Searching for ServerMethods interface definition ==="
rg -n "interface ServerMethods|type ServerMethods" --type ts -A 20 | head -50

# Check 5: Check if removeCannedResponse is registered anywhere else
echo ""
echo "=== Broader search for removeCannedResponse registration patterns ==="
rg -n "removeCannedResponse.*:" --type ts --type js -C 2 | head -40

Length of output: 6282


🏁 Script executed:

#!/bin/bash
# Verify actual usage and REST API removal

# Check 1: See how removeCannedResponse is used in api-enterprise
echo "=== Usage in api-enterprise/server/canned-responses.ts ==="
cat apps/meteor/ee/app/api-enterprise/server/canned-responses.ts | head -100

# Check 2: Search for REST route handlers for canned-responses
echo ""
echo "=== Searching for REST routes for canned-responses ==="
rg -n "DELETE.*canned-responses|/v1/canned-responses" --type ts -B 3 -A 3

# Check 3: Check if there's a deprecation wrapper or if the method was actually removed from public use
echo ""
echo "=== Checking for any deprecation decorators or comments ==="
rg -n "deprecated|Deprecated|DEPRECATED" apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts -B 2 -A 2

# Check 4: Verify imports in removeCannedResponse.ts file
echo ""
echo "=== Complete removeCannedResponse.ts file ==="
cat apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts

Length of output: 17085


🏁 Script executed:

#!/bin/bash
# Find where removeCannedResponse is actually called in route handlers

echo "=== Full DELETE route handler implementation for canned-responses/:_id ==="
sed -n '166,200p' apps/meteor/ee/app/api-enterprise/server/canned-responses.ts

echo ""
echo "=== Search for removeCannedResponse calls in the file ==="
grep -n "removeCannedResponse" apps/meteor/ee/app/api-enterprise/server/canned-responses.ts

echo ""
echo "=== Full route handler for DELETE canned-responses (deprecated) ==="
sed -n '106,160p' apps/meteor/ee/app/api-enterprise/server/canned-responses.ts

Length of output: 2501


Changeset message is incorrect—removeCannedResponse function was not removed and remains actively used.

The changeset claims "Removes deprecated removeCannedResponse method," but verification shows:

  • The removeCannedResponse function still exists in apps/meteor/ee/app/canned-responses/server/methods/removeCannedResponse.ts and remains exported
  • The function is actively called in two REST route handlers in apps/meteor/ee/app/api-enterprise/server/canned-responses.ts:
    • Line 159: DELETE /v1/canned-responses/:_id
    • Line 188: DELETE /v1/canned-responses (deprecated)
  • The function has no deprecation decorator or logger (unlike saveCannedResponse)
  • No Meteor.methods registration ever existed for this function

Either update the changeset message to reflect the actual changes, or complete the removal by:

  1. Replacing both route handlers to use an alternative implementation
  2. Removing the import and function file
  3. Adding deprecation logging if maintaining backward compatibility

@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Nov 7, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 7, 2025
@ggazzo ggazzo merged commit 750af59 into release-8.0.0 Nov 7, 2025
54 checks passed
@ggazzo ggazzo deleted the chore/remove-removeCannedResponse branch November 7, 2025 20:48
gaolin1 pushed a commit to gaolin1/medsense.webchat that referenced this pull request Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants