chore: deprecate conditions param in livechat autocomplete model methods - #41766
chore: deprecate conditions param in livechat autocomplete model methods#41766jonasflorencio wants to merge 3 commits into
conditions param in livechat autocomplete model methods#41766Conversation
|
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 |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (5)
|
| 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.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| custom_name: string; | ||
| } | ||
| > { | ||
| if (Object.keys(conditions).length > 0) { |
There was a problem hiding this comment.
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:
Also, instead of using
logger.warn, use the utils from deprecationWarningLogger.ts
There was a problem hiding this comment.
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Deprecates the
conditionsparameter in thefindByNameRegexWithExceptionsAndConditionsmethod ofLivechatDepartmentandLivechatVisitorsmodels. 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
selector.conditionsoption is used in department and visitor autocomplete requests.