UN-3375 [FIX] Support multiple nullable FK paths in OrganizationFilterBackend#1889
Merged
chandrasekharan-zipstack merged 4 commits intomainfrom Apr 2, 2026
Merged
Conversation
…rBackend Add org_filter_paths ViewSet attribute for models with multiple nullable FK paths to Organization (e.g. Notification → Pipeline or API → Workflow → Organization). When set, the filter uses OR across all paths instead of BFS which only finds one path and misses records linked via the other. Apply org_filter_paths to NotificationViewSet to fix 404 on notification PUT/DELETE operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Summary by CodeRabbit
WalkthroughAdded organization filter path configuration support to the notification viewset and updated the organization filter backend to use explicit filter paths when provided. The backend now reads a configurable list of ORM traversal paths from viewsets to apply organization filtering instead of relying solely on automatic path discovery. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/notification_v2/views.py (1)
16-19: Use an immutable class attribute fororg_filter_paths.Configuration attributes should use tuples instead of lists to prevent accidental in-place mutations. Although no mutations are currently found in the codebase, using an immutable structure is a safer pattern for static configuration.
Proposed diff
- org_filter_paths = [ + org_filter_paths = ( "pipeline__workflow__organization", "api__workflow__organization", - ] + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/notification_v2/views.py` around lines 16 - 19, Replace the mutable list org_filter_paths with an immutable tuple and make it a class-level attribute where it is defined; specifically change org_filter_paths = ["pipeline__workflow__organization", "api__workflow__organization"] to org_filter_paths = ("pipeline__workflow__organization", "api__workflow__organization") (defined as a class attribute on the view class or module-level constant) to prevent accidental in-place mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/utils/filters/organization_filter.py`:
- Around line 62-70: The explicit-path filtering using explicit_paths =
getattr(view, ORG_FILTER_PATHS, None) can raise
django.core.exceptions.FieldError and currently lacks input validation; update
the block around explicit_paths to first validate that explicit_paths is a list
or tuple and that each element is a non-empty string, then build the Q() as
before but call queryset.filter(q) inside a try/except that catches FieldError
(and any subclasses), logs the problem, and returns a fail-closed result (e.g.,
queryset.none()) on error; reference the symbols explicit_paths,
ORG_FILTER_PATHS, q, and queryset.filter when making the changes.
---
Nitpick comments:
In `@backend/notification_v2/views.py`:
- Around line 16-19: Replace the mutable list org_filter_paths with an immutable
tuple and make it a class-level attribute where it is defined; specifically
change org_filter_paths = ["pipeline__workflow__organization",
"api__workflow__organization"] to org_filter_paths =
("pipeline__workflow__organization", "api__workflow__organization") (defined as
a class attribute on the view class or module-level constant) to prevent
accidental in-place mutations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a0f4236-3832-4c87-86ba-f9d193ab521f
📒 Files selected for processing (2)
backend/notification_v2/views.pybackend/utils/filters/organization_filter.py
Contributor
|
| Filename | Overview |
|---|---|
| backend/utils/filters/organization_filter.py | Adds SKIP_ORG_FILTER/ORG_FILTER_PATHS constants and an opt-in OR-filter branch for multi-path models; existing BFS fallback is untouched and the new branch is guarded correctly. |
| backend/notification_v2/views.py | Adds org_filter_paths with the two correct nullable FK traversal paths; get_queryset narrowing by pipeline/api UUID interacts safely with the downstream org filter. |
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/UN-3375-FIX..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
chandrasekharan-zipstack
approved these changes
Apr 2, 2026
athul-rs
approved these changes
Apr 2, 2026
|
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
chandrasekharan-zipstack
deleted the
fix/UN-3375-FIX_org_filter_multiple_fk_paths
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
org_filter_pathsViewSet attribute toOrganizationFilterBackendfor models with multiple nullable FK paths to Organizationorg_filter_pathstoNotificationViewSetto fix 404 on PUT/DELETEWhy
Notificationmodel has two nullable FKs to Organization:pipeline→workflow→organdapi→workflow→org. The BFS inOrganizationFilterBackendpicks one path — records linked through the other nullable FK get filtered out, returning 404.How
When
org_filter_pathsis set on a ViewSet, the filter builds an OR query across all paths instead of running BFS. This handles models where the FK path to Organization depends on which nullable FK is populated.Also added
SKIP_ORG_FILTERandORG_FILTER_PATHSconstants to avoid typos ingetattrcalls.Can this PR break any existing features?
No.
org_filter_pathsis opt-in — ViewSets without it use the existing BFS path discovery unchanged. The constants are used only inside the filter backend'sgetattrcalls.Database Migrations
None
Env Config
None
Related Issues or PRs
Notes on Testing
Checklist