Skip to content

Conversation

@lucas-a-pelegrino
Copy link
Contributor

@lucas-a-pelegrino lucas-a-pelegrino commented Sep 18, 2025

Proposed changes (including videos or screenshots)

This PR adds a deprecation warning for livechat:saveDepartment meteor method.

Issue(s)

CTZ-65

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Chores
    • Added a deprecation warning for the legacy Livechat “save department” operation. A server-side log is emitted when it’s used. No functional changes for current users.
  • Documentation
    • Updated release notes to reflect the deprecation of the legacy Livechat department save flow and guide migration to supported endpoints.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Sep 18, 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 Sep 18, 2025

🦋 Changeset detected

Latest commit: 561e963

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

This PR includes changesets to release 39 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@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 Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/web-ui-registration Patch
@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/stream-hub-service Patch
@rocket.chat/license 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 Patch
@rocket.chat/ui-voip Patch
@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 Sep 18, 2025

Walkthrough

Introduces internal deprecation logging for the Meteor method livechat:saveDepartment using methodDeprecationLogger, emitting a notice for version 8.0.0 and REST path /v1/livechat/department. Adds a corresponding changeset entry marking a patch release for @rocket.chat/meteor. No public signatures or error handling are modified.

Changes

Cohort / File(s) Summary
Livechat deprecation logging
apps/meteor/app/livechat/server/methods/saveDepartment.ts
Inserted methodDeprecationLogger call at method start for 'livechat:saveDepartment' (version '8.0.0', path '/v1/livechat/department'); no other logic or signatures changed.
Changeset metadata
.changeset/rare-plants-shake.md
Added changeset marking a patch bump for @rocket.chat/meteor and documenting the deprecation warning for livechat:saveDepartment.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant M as Meteor Method<br/>livechat:saveDepartment
  participant L as methodDeprecationLogger
  participant S as Livechat Service/DB

  C->>M: Call saveDepartment(payload)
  rect rgba(200,220,255,0.25)
    note over M,L: New deprecation notice
    M->>L: log('livechat:saveDepartment', '8.0.0', '/v1/livechat/department')
    L-->>M: acknowledged
  end
  M->>M: Get userId, check permissions, validate input
  M->>S: Create/Update department
  S-->>M: Result/Errors
  M-->>C: Response
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • aleksandernsilva
  • sampaiodiego

Poem

A bunny taps the server logs—tap tap!
“Deprecated,” it whispers, map by map.
SaveDepartment hops to eight-point-oh,
Leaves a note before the data flow.
Patch-sized carrots, neatly stored,
Warren’s tidy, warnings scored. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ❓ Inconclusive The PR references CTZ-65 but the linked issue content provided here contains no acceptance criteria or implementation requirements, so I cannot fully verify that all issue-level requirements are met; the actual code changes (adding an internal deprecation log call and a changeset entry) do align with the stated intent to deprecate livechat:saveDepartment. Please attach or link the CTZ-65 acceptance criteria and confirm which items this PR satisfies, and add any required tests, documentation, or external notices if the issue requires them; if the issue only requested a deprecation log and changeset, state that explicitly in the PR description.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: Adds deprecation warning on livechat:saveDepartment" is concise, specific, and accurately describes the primary change (adding a deprecation warning for the Meteor method); it is a single clear sentence and contains no extraneous noise.
Out of Scope Changes Check ✅ Passed Based on the provided file summaries, the changes are limited to a new changeset entry and an internal deprecation logging call added to the saveDepartment method with no signature or unrelated file modifications, so there is no evidence of out-of-scope changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/v7/CTZ-65

📜 Recent 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 b1cefb0 and 6a6219f.

📒 Files selected for processing (2)
  • .changeset/rare-plants-shake.md (1 hunks)
  • apps/meteor/app/livechat/server/methods/saveDepartment.ts (2 hunks)
🔇 Additional comments (3)
.changeset/rare-plants-shake.md (1)

1-5: Changeset looks good; scope and summary are appropriate for a patch.

Accurately documents the deprecation warning without implying an API break. No further edits needed.

apps/meteor/app/livechat/server/methods/saveDepartment.ts (2)

41-41: Deprecation log placed before auth — confirm intent

Logging currently runs before the hasPermissionAsync guard and will record unauthorized attempts (which may add noise). If you only want to warn legitimate callers, move the deprecation log after the permission check; otherwise keep as-is. The deprecation path '/v1/livechat/department' matches existing conventions.

Proposed alternative (move after the guard):

async 'livechat:saveDepartment'(_id, departmentData, departmentAgents, departmentUnit) {
-    methodDeprecationLogger.method('livechat:saveDepartment', '8.0.0', '/v1/livechat/department');
     const uid = Meteor.userId();
     if (!uid || !(await hasPermissionAsync(uid, 'manage-livechat-departments'))) {
       throw new Meteor.Error('error-not-allowed', 'Not allowed', {
         method: 'livechat:saveDepartment',
       });
     }
+    methodDeprecationLogger.method('livechat:saveDepartment', '8.0.0', '/v1/livechat/department');
     return saveDepartment(uid, _id, departmentData, { upsert: departmentAgents }, departmentUnit);
}

6-6: Import of methodDeprecationLogger — canonical import confirmed

apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts is the single implementation; saveDepartment.ts's import ('../../../lib/server/lib/deprecationWarningLogger') resolves to it and methodDeprecationLogger.method is referenced across the repo — no alternate re-exports detected in the provided search output.


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

@lucas-a-pelegrino lucas-a-pelegrino added this to the 7.11.0 milestone Sep 18, 2025
@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.26%. Comparing base (65de26f) to head (561e963).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #36981      +/-   ##
===========================================
- Coverage    67.32%   66.26%   -1.07%     
===========================================
  Files         3339     3393      +54     
  Lines       113205   115456    +2251     
  Branches     20532    21157     +625     
===========================================
+ Hits         76219    76503     +284     
- Misses       34380    36342    +1962     
- Partials      2606     2611       +5     
Flag Coverage Δ
e2e 56.92% <ø> (-0.02%) ⬇️
unit 71.23% <ø> (+<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.

@lucas-a-pelegrino lucas-a-pelegrino marked this pull request as ready for review September 18, 2025 17:05
@lucas-a-pelegrino lucas-a-pelegrino requested a review from a team as a code owner September 18, 2025 17:05
@MartinSchoeler MartinSchoeler added the stat: QA assured Means it has been tested and approved by a company insider label Sep 19, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Sep 19, 2025
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