Skip to content

chore: deprecate conditions param in livechat autocomplete model methods - #41766

Open
jonasflorencio wants to merge 3 commits into
developfrom
chore/deprecate-conditions-param-livechat-autocomplete
Open

chore: deprecate conditions param in livechat autocomplete model methods#41766
jonasflorencio wants to merge 3 commits into
developfrom
chore/deprecate-conditions-param-livechat-autocomplete

Conversation

@jonasflorencio

@jonasflorencio jonasflorencio commented Aug 12, 2026

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

Deprecates the conditions parameter in the findByNameRegexWithExceptionsAndConditions method of LivechatDepartment and LivechatVisitors models. A warning will now be logged whenever this parameter is used. It will be fully removed in version 9.0.0.

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Deprecations
    • Added notices when the deprecated selector.conditions option is used in department and visitor autocomplete requests.
    • Existing autocomplete behavior remains unchanged while this option is being phased out.

@jonasflorencio
jonasflorencio requested a review from a team as a code owner August 12, 2026 20:03
@dionisio-bot

dionisio-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 537042a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c4e1c7ef-7792-46fc-a85b-9f3ed4ad33df

📥 Commits

Reviewing files that changed from the base of the PR and between 378f3c6 and 537042a.

📒 Files selected for processing (2)
  • apps/meteor/server/api/v1/omnichannel/departments.ts
  • apps/meteor/server/api/v1/omnichannel/visitors.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/meteor/server/api/v1/omnichannel/departments.ts
  • apps/meteor/server/api/v1/omnichannel/visitors.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: CodeQL-Build
  • GitHub Check: Hacktron Security Check
  • GitHub Check: CodeQL-Build
⚠️ CI failures not shown inline (5)

GitHub Check: Dionisio QA: Some checks did not pass

Conclusion: failure

View job details

**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ❌ **QA assured** — This PR is missing the 'stat: QA assured' label
- ✅ **Mergeable**
- ❌ **Has milestone or project** — This PR is missing the required milestone or project
- ✅ **Valid PR title**
- ✅ **Correct target version**

GitHub Check: Dionisio QA: Some checks did not pass

Conclusion: failure

View job details

**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ❌ **QA assured** — This PR is missing the 'stat: QA assured' label
- ✅ **Mergeable**
- ❌ **Has milestone or project** — This PR is missing the required milestone or project
- ✅ **Valid PR title**
- ✅ **Correct target version**

GitHub Check: Dionisio QA: Some checks did not pass

Conclusion: failure

View job details

**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ❌ **QA assured** — This PR is missing the 'stat: QA assured' label
- ✅ **Mergeable**
- ❌ **Has milestone or project** — This PR is missing the required milestone or project
- ✅ **Valid PR title**
- ✅ **Correct target version**

GitHub Check: Dionisio QA: Some checks did not pass

Conclusion: failure

View job details

**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ❌ **QA assured** — This PR is missing the 'stat: QA assured' label
- ✅ **Mergeable**
- ❌ **Has milestone or project** — This PR is missing the required milestone or project
- ✅ **Valid PR title**
- ✅ **Correct target version**

GitHub Check: Dionisio QA: Some checks did not pass

Conclusion: failure

View job details

**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ❌ **QA assured** — This PR is missing the 'stat: QA assured' label
- ✅ **Mergeable**
- ❌ **Has milestone or project** — This PR is missing the required milestone or project
- ✅ **Valid PR title**
- ✅ **Correct target version**

Walkthrough

The department and visitor autocomplete endpoints now parse selectors once, log deprecated non-empty conditions, and pass the parsed selectors to their lookup methods.

Changes

Omnichannel autocomplete deprecation logging

Layer / File(s) Summary
Route selector deprecation logging
apps/meteor/server/api/v1/omnichannel/departments.ts, apps/meteor/server/api/v1/omnichannel/visitors.ts
Both autocomplete handlers reuse the parsed selector, log deprecation data when conditions is non-empty, and pass the parsed selector to the lookup method.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to 53704

This localized change deprecates the conditions parameter and logs a warning when it is used; no actionable merge-blocking risk remains after normal checks and review.

Suggested labels: type: chore

🚥 Pre-merge checks | ✅ 5
✅ 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 summarizes the main change: deprecating the conditions parameter in livechat autocomplete model methods.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/models/src/models/LivechatDepartment.ts Outdated
custom_name: string;
}
> {
if (Object.keys(conditions).length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The deprecation has to be done at REST endpoint level (I mean, in apps/meteor/server/api/v1/omnichannel/departments.ts and apps/meteor/server/api/v1/omnichannel/visitors.ts), not at model level, because the contract is being broke there. You can mirror this example:

apiDeprecationLogger.parameter(route, 'query', '9.0.0', response, messageGenerator);

Also, instead of using logger.warn, use the utils from deprecationWarningLogger.ts

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@jonasflorencio
jonasflorencio requested a review from a team as a code owner August 12, 2026 21:25

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/models/src/models/LivechatVisitors.ts">

<violation number="1">
P1: Custom agent: **Detect usage of deprecated HTTP endpoints or API helpers**

The PR claims to add deprecation warnings for the `conditions` parameter, but this change removes the existing warning (and its Logger instance) from `findByNameRegexWithExceptionsAndConditions`. Callers will no longer be warned that the parameter is deprecated and will be removed in 9.0.0, undermining the deprecation goal and increasing the risk of silent continued usage.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.04%. Comparing base (742009a) to head (537042a).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #41766   +/-   ##
========================================
  Coverage    69.04%   69.04%           
========================================
  Files         4224     4224           
  Lines       166089   166089           
  Branches     29557    29595   +38     
========================================
+ Hits        114669   114675    +6     
+ Misses       46261    46245   -16     
- Partials      5159     5169   +10     
Flag Coverage Δ
unit 70.94% <ø> (+<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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants