Skip to content

Conversation

@lucas-a-pelegrino
Copy link
Contributor

@lucas-a-pelegrino lucas-a-pelegrino commented Oct 22, 2025

Proposed changes (including videos or screenshots)

This PR aims at adding a deprecation warning for livechat:saveTag, it also adds a new endpoint replacing the deprecated livechat method; livechat/tags.save

Issue(s)

CORE-1411

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • New Features

    • Added REST endpoint POST /v1/livechat/tags.save for creating/updating livechat tags with a structured payload and returns the saved tag.
  • Deprecations

    • Deprecated livechat:saveTag method; migrate to POST /v1/livechat/tags.save (scheduled removal in v8.0.0).
  • Public API

    • Added new request/response types and validation for the tags.save endpoint in the REST typings.

@lucas-a-pelegrino lucas-a-pelegrino added this to the 7.13.0 milestone Oct 22, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 22, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.14.0, but it targets 7.13.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2025

🦋 Changeset detected

Latest commit: e2da75e

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

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/rest-typings Patch
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/http-router Patch
@rocket.chat/models 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/stream-hub-service Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/media-calls Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/core-typings Patch
@rocket.chat/apps Patch
@rocket.chat/freeswitch Patch
@rocket.chat/model-typings Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker 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 Oct 22, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Migrates livechat tag saving from a Meteor RPC method (livechat:saveTag) to a REST endpoint (POST /v1/livechat/tags.save), adds typed request/response schemas, logs a deprecation warning for the old method, updates client usage and tests, and separates the delete route.

Changes

Cohort / File(s) Summary
REST API Endpoint & Types
apps/meteor/ee/app/livechat-enterprise/server/api/tags.ts, packages/rest-typings/src/v1/omnichannel.ts
Adds POST /v1/livechat/tags.save with POSTLivechatTagsSaveParams and POSTLivechatTagsSaveSuccessResponse schemas; wires auth/permissions/license; separates livechat/tags.delete route; expands error handling and response mapping; augments rest-typings Endpoints.
Client Migration
apps/meteor/client/omnichannel/tags/TagEdit.tsx
Replaces useMethod('livechat:saveTag') with useEndpoint('POST','/v1/livechat/tags.save') and adjusts payload from positional params to { _id, tagData, tagDepartments }.
Method Deprecation Notice
apps/meteor/ee/app/livechat-enterprise/server/methods/saveTag.ts
Adds methodDeprecationLogger.method('livechat:saveTag', '8.0.0', 'POST /v1/livechat/tags.save') at method start.
Tests & E2E Helpers
apps/meteor/tests/data/livechat/tags.ts, apps/meteor/tests/e2e/utils/omnichannel/tags.ts
Switches test payloads to POST /v1/livechat/tags.save shape; simplifies response handling; changes CreateTagParams.departments from { departmentId: string }[] to string[].
Changeset
.changeset/seven-otters-turn.md
Adds changeset documenting patch bumps for packages and deprecation notice for livechat:saveTag.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client App
    participant REST as REST Endpoint
    participant Service as LivechatEnterprise

    rect rgb(220, 255, 220)
    Note over Client,REST: New flow (POST /v1/livechat/tags.save)
    Client->>REST: POST /v1/livechat/tags.save\n{ _id, tagData, tagDepartments }
    REST->>Service: LivechatEnterprise.saveTag(_id, tagData, tagDepartments)
    Service-->>REST: result
    REST-->>Client: 200 OK { tag data }
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas to focus on:
    • apps/meteor/ee/app/livechat-enterprise/server/api/tags.ts — error handling branches and response mapping.
    • packages/rest-typings/src/v1/omnichannel.ts — AJV schemas and exported types correctness.
    • apps/meteor/client/omnichannel/tags/TagEdit.tsx — payload mapping and UI side effects (toasts, query invalidation).
    • Test updates (apps/meteor/tests/...) — ensure test payloads/types align with new shapes.

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • aleksandernsilva
  • KevLehman

Poem

🐇 I hopped from RPC to REST today,
New endpoints guide my carrot-way,
Tags saved with POST, old call says “bye”,
I twitch my nose and watch tests fly! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive Unable to fully assess compliance as the linked issue CORE-1411 lacks detailed requirements or coding objectives. Provide detailed requirements and coding objectives from issue CORE-1411 to validate that all changes meet the expected implementation.
✅ 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 accurately summarizes the main changes: deprecation warning for livechat:saveTag and new REST endpoint to replace it.
Out of Scope Changes check ✅ Passed All changes are in-scope: deprecation warning added to saveTag method, new REST endpoint created, API typings defined, and tests updated to use new endpoint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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 Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.81%. Comparing base (dc51116) to head (e2da75e).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #37281      +/-   ##
===========================================
+ Coverage    68.78%   68.81%   +0.02%     
===========================================
  Files         3363     3360       -3     
  Lines       114202   114142      -60     
  Branches     20617    20604      -13     
===========================================
- Hits         78556    78543      -13     
+ Misses       33551    33511      -40     
+ Partials      2095     2088       -7     
Flag Coverage Δ
e2e-api 43.23% <ø> (+1.02%) ⬆️

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 October 24, 2025 21:11
@lucas-a-pelegrino lucas-a-pelegrino requested review from a team as code owners October 24, 2025 21:11
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: 2

📜 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 917005c and e552717.

📒 Files selected for processing (7)
  • .changeset/seven-otters-turn.md (1 hunks)
  • apps/meteor/client/omnichannel/tags/TagEdit.tsx (3 hunks)
  • apps/meteor/ee/app/livechat-enterprise/server/api/tags.ts (2 hunks)
  • apps/meteor/ee/app/livechat-enterprise/server/methods/saveTag.ts (2 hunks)
  • apps/meteor/tests/data/livechat/tags.ts (1 hunks)
  • apps/meteor/tests/e2e/utils/omnichannel/tags.ts (1 hunks)
  • packages/rest-typings/src/v1/omnichannel.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation

Files:

  • apps/meteor/tests/e2e/utils/omnichannel/tags.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently

Files:

  • apps/meteor/tests/e2e/utils/omnichannel/tags.ts
🧬 Code graph analysis (4)
apps/meteor/tests/data/livechat/tags.ts (1)
apps/meteor/tests/data/api-data.ts (2)
  • request (10-10)
  • credentials (39-42)
apps/meteor/client/omnichannel/tags/TagEdit.tsx (1)
apps/meteor/ee/app/livechat-enterprise/server/lib/LivechatEnterprise.ts (1)
  • saveTag (141-143)
apps/meteor/ee/app/livechat-enterprise/server/api/tags.ts (3)
packages/rest-typings/src/v1/omnichannel.ts (4)
  • POSTLivechatTagsSaveSuccessResponse (586-586)
  • isPOSTLivechatTagsSaveParams (555-555)
  • POSTLivechatTagsRemoveSuccessResponse (616-616)
  • isPOSTLivechatTagsRemoveParams (603-603)
apps/meteor/ee/app/livechat-enterprise/server/lib/LivechatEnterprise.ts (1)
  • LivechatEnterprise (13-183)
apps/meteor/ee/app/livechat-enterprise/server/methods/removeTag.ts (1)
  • id (17-26)
apps/meteor/tests/e2e/utils/omnichannel/tags.ts (2)
apps/meteor/tests/data/livechat/tags.ts (1)
  • removeTag (28-48)
apps/meteor/ee/app/livechat-enterprise/server/methods/removeTag.ts (1)
  • id (17-26)

dougfabris
dougfabris previously approved these changes Oct 24, 2025
Copy link
Member

@dougfabris dougfabris left a 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!

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 e552717 and 2ac8925.

📒 Files selected for processing (1)
  • apps/meteor/ee/app/livechat-enterprise/server/methods/saveTag.ts (2 hunks)
🧰 Additional context used
🧠 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/saveTag.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/saveTag.ts
⏰ 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). (1)
  • GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/ee/app/livechat-enterprise/server/methods/saveTag.ts (2)

7-7: LGTM!

The import is correct and necessary for the deprecation warning functionality.


19-19: Previous issue resolved.

The deprecation logger now correctly references 'livechat:saveTag'. The method name, version, and replacement endpoint are all accurate.

lucas-a-pelegrino and others added 2 commits November 6, 2025 11:48
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2025

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +12MiB
rocketchat 359MiB 347MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB -1.9KiB
queue-worker-service 132MiB 132MiB +117B
ddp-streamer-service 126MiB 126MiB -206B
account-service 113MiB 113MiB +902B
stream-hub-service 111MiB 111MiB +261B
authorization-service 111MiB 111MiB +1.2KiB
presence-service 111MiB 111MiB -1.2KiB

📊 Historical Trend

---
config:
  theme: "dark"
  xyChart:
    width: 900
    height: 400
---
xychart
  title "Image Size Evolution by Service (Last 30 Days + This PR)"
  x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 14:05", "12/04 15:54 (PR)"]
  y-axis "Size (GB)" 0 --> 0.5
  line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
  line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Loading

Statistics (last 13 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.2GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37281
  • Baseline: develop
  • Timestamp: 2025-12-04 15:54:54 UTC
  • Historical data points: 13

Updated: Thu, 04 Dec 2025 15:54:54 GMT

@scuciatto scuciatto modified the milestones: 7.13.0, 7.14.0 Nov 25, 2025
@lucas-a-pelegrino lucas-a-pelegrino marked this pull request as draft December 1, 2025 20:03
@lucas-a-pelegrino lucas-a-pelegrino marked this pull request as ready for review December 2, 2025 14:06
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

Frontend looks good!

@lucas-a-pelegrino lucas-a-pelegrino added stat: ready to merge PR tested and approved waiting for merge stat: QA assured Means it has been tested and approved by a company insider labels Dec 3, 2025
@kodiakhq kodiakhq bot merged commit dc67590 into develop Dec 4, 2025
86 of 88 checks passed
@kodiakhq kodiakhq bot deleted the chore/v7/CORE-1411 branch December 4, 2025 18:00
@dougfabris dougfabris modified the milestones: 7.14.0, 8.0.0 Jan 19, 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.

7 participants