Next release - #1693
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds ChangesNew operator support
Sequence Diagram(s)No sequence diagram generated; the changes are operator additions to existing evaluation/query logic without new multi-component interaction flows. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/models/device_instance.py`:
- Around line 134-142: The `not_equals` and `not_contains` branches in the
filter builder are excluding NULL field values because `field != ?` and `field
NOT LIKE ?` do not match NULLs. Update the logic in `device_instance.py` so the
query treats NULL as a valid non-match for these operators, using the existing
clause-building flow around `operator`, `clauses`, and `params`. Keep the
behavior of `contains` unchanged, and make sure the negation semantics for
`not_equals` and `not_contains` include rows where the target field is NULL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b91e9f21-bc2d-4721-afee-40382c5fe454
📒 Files selected for processing (5)
docs/WORKFLOW_EXAMPLES.mdfront/workflowsCore.phpserver/models/device_instance.pyserver/workflows/conditions.pytest/backend/test_workflows.py
| elif operator == "not_equals": | ||
| clauses.append(f"{field} != ?") | ||
| params.append(value) | ||
| elif operator == "contains": | ||
| clauses.append(f"{field} LIKE ?") | ||
| params.append(f"%{value}%") | ||
| elif operator == "not_contains": | ||
| clauses.append(f"{field} NOT LIKE ?") | ||
| params.append(f"%{value}%") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
NULL rows silently excluded from not_equals/not_contains results.
In SQL, field != ? and field NOT LIKE ? evaluate to NULL (not TRUE) when field is NULL, so devices with a NULL value for the compared field are dropped from the result set even though "NULL is not equal to X" is intuitively true. This diverges from the negation semantics implied by the operator names.
🛠️ Proposed fix
elif operator == "not_equals":
- clauses.append(f"{field} != ?")
+ clauses.append(f"({field} != ? OR {field} IS NULL)")
params.append(value)
elif operator == "contains":
clauses.append(f"{field} LIKE ?")
params.append(f"%{value}%")
elif operator == "not_contains":
- clauses.append(f"{field} NOT LIKE ?")
+ clauses.append(f"({field} NOT LIKE ? OR {field} IS NULL)")
params.append(f"%{value}%")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif operator == "not_equals": | |
| clauses.append(f"{field} != ?") | |
| params.append(value) | |
| elif operator == "contains": | |
| clauses.append(f"{field} LIKE ?") | |
| params.append(f"%{value}%") | |
| elif operator == "not_contains": | |
| clauses.append(f"{field} NOT LIKE ?") | |
| params.append(f"%{value}%") | |
| elif operator == "not_equals": | |
| clauses.append(f"({field} != ? OR {field} IS NULL)") | |
| params.append(value) | |
| elif operator == "contains": | |
| clauses.append(f"{field} LIKE ?") | |
| params.append(f"%{value}%") | |
| elif operator == "not_contains": | |
| clauses.append(f"({field} NOT LIKE ? OR {field} IS NULL)") | |
| params.append(f"%{value}%") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/models/device_instance.py` around lines 134 - 142, The `not_equals`
and `not_contains` branches in the filter builder are excluding NULL field
values because `field != ?` and `field NOT LIKE ?` do not match NULLs. Update
the logic in `device_instance.py` so the query treats NULL as a valid non-match
for these operators, using the existing clause-building flow around `operator`,
`clauses`, and `params`. Keep the behavior of `contains` unchanged, and make
sure the negation semantics for `not_equals` and `not_contains` include rows
where the target field is NULL.
There was a problem hiding this comment.
@jokob-sk, noted — this request is directed at @copilot, so I'll step back and let them implement the fix. Feel free to tag me if you'd like me to review the change once it's applied, or if you'd prefer I implement it instead.
🧠 Learnings used
Learnt from: jokob-sk
Repo: netalertx/NetAlertX PR: 1621
File: server/auth/ldap_provider.py:97-97
Timestamp: 2026-04-27T00:51:56.385Z
Learning: In the NetAlertX server-side Python code, treat logging text inside `mylog(...)` as developer/debug-only: keep it in English and do not add those strings to `front/php/templates/language/en_us.json`. By contrast, any string that is user-facing (e.g., error messages returned from API responses that are rendered in the login page/front-end) must be added as a translation key to `front/php/templates/language/en_us.json` so it can be translated via Weblate.
Learnt from: jokob-sk
Repo: netalertx/NetAlertX PR: 1621
File: server/auth/ldap_provider.py:165-165
Timestamp: 2026-04-27T00:52:02.534Z
Learning: In the NetAlertX codebase, do not use inline/dynamic imports in Python (e.g., `import ...` inside functions/methods). Require all imports to be at module top level so they are auditable and missing dependencies fail at import/load time rather than later at runtime. If an optional dependency must be handled, use a top-level `try/except ImportError` block (e.g., for `ldap3`) instead of a deferred inline import.
|
@coderabbitai re check |
|
✅ Action performedReview finished.
|
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation