starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-12 05:20:31 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Ensure all custom GraphQL filter methods apply prefix in Q(...) for nested filters #11706

New Issue
Closed
opened 2025-12-29 21:48:51 +01:00 by adam · 2 comments
adam commented 2025-12-29 21:48:51 +01:00
Owner
Copy Link

Originally created by @pheus on GitHub (Oct 9, 2025).

Originally assigned to: @pheus on GitHub.

NetBox Edition

NetBox Community

NetBox Version

v4.4.2

Python Version

3.12

Steps to Reproduce

  1. Run a nested GraphQL query that relies on a custom filter method returning a raw Q(...) without applying the Strawberry‑Django prefix (illustrative example from #20466): 
    query {
  device_list(filters: { primary_ip4: { assigned: true } }) {
    id
    primary_ip4 { address }
  }
}
  2. Observe that the generated lookup key is unqualified (e.g., assigned_object_id__isnull) and is applied to the outer model rather than the nested relation, producing an error like “Cannot resolve keyword ‘assigned_object_id’ into field”.

Note: #20466 proposes/fixes one concrete instance in IPAddressFilter.assigned by using return Q(**{f"{prefix}assigned_object_id__isnull": not value}). This issue tracks auditing other custom filter methods for the same class of bug.

Expected Behavior

All custom GraphQL filter methods that return Q qualify their lookups using the provided prefix, so nested filters target the correct related model and do not error.

Observed Behavior

At least one confirmed case (#20466) shows a custom filter method constructing Q(...) without prefix, causing nested filters to mis-target the outer model and fail with “Cannot resolve keyword …”. Other custom filter methods that build raw Q(...) may be similarly affected.

Proposed Fix / Scope

  • Audit all @strawberry_django.filter_field() methods across GraphQL filters and update any return Q(...) to include prefix in the lookup keys. Example pattern: 
    @strawberry_django.filter_field()
def <name>(self, value: T, prefix: str) -> Q:
    return Q(**{f"{prefix}<field_path>": value})
  • Where a method returns (queryset, Q), ensure any annotated keys or aliases also incorporate prefix.

References

Originally created by @pheus on GitHub (Oct 9, 2025). Originally assigned to: @pheus on GitHub. ### NetBox Edition NetBox Community ### NetBox Version v4.4.2 ### Python Version 3.12 ### Steps to Reproduce 1. Run a nested GraphQL query that relies on a custom filter method returning a raw `Q(...)` without applying the Strawberry‑Django `prefix` (illustrative example from #20466): ```graphql query { device_list(filters: { primary_ip4: { assigned: true } }) { id primary_ip4 { address } } } ``` 2. Observe that the generated lookup key is unqualified (e.g., `assigned_object_id__isnull`) and is applied to the *outer* model rather than the nested relation, producing an error like “Cannot resolve keyword ‘assigned_object_id’ into field”. **Note**: #20466 proposes/fixes one concrete instance in `IPAddressFilter.assigned` by using `return Q(**{f"{prefix}assigned_object_id__isnull": not value})`. This issue tracks auditing **other** custom filter methods for the same class of bug. ### Expected Behavior All custom GraphQL filter methods that return `Q` qualify their lookups using the provided `prefix`, so nested filters target the correct related model and do not error. ### Observed Behavior At least one confirmed case (#20466) shows a custom filter method constructing `Q(...)` without `prefix`, causing nested filters to mis-target the outer model and fail with “Cannot resolve keyword …”. Other custom filter methods that build raw `Q(...)` may be similarly affected. --- #### Proposed Fix / Scope - **Audit** all `@strawberry_django.filter_field()` methods across GraphQL filters and update any `return Q(...)` to include `prefix` in the lookup keys. Example pattern: ```python @strawberry_django.filter_field() def <name>(self, value: T, prefix: str) -> Q: return Q(**{f"{prefix}<field_path>": value}) ``` - Where a method returns `(queryset, Q)`, ensure any annotated keys or aliases also incorporate `prefix`. #### References - Example bug & reproduction: #20466 - Strawberry‑Django docs show `prefix` is **required** for nested filters and demonstrate qualified `Q(**{f"{prefix}…": …})` lookups. - https://strawberry.rocks/docs/django/guide/filters#custom-filter-methods - https://strawberry.rocks/docs/django/guide/filters#resolver-arguments
adam added the type: bugstatus: acceptednetboxseverity: low labels 2025-12-29 21:48:51 +01:00
adam closed this issue 2025-12-29 21:48:51 +01:00
adam commented 2025-12-29 21:48:52 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 9, 2025):

I’d like to propose the following solution to address the nested GraphQL filtering issues (scoped to ipam.graphql.filters):

Proposed changes

  • Make custom filter methods consistently prefix‑aware when building Q(**{f"{prefix}…": …}) (e.g., IPAddressFilter.assigned) so nested filters target the correct model.

  • Add input safeguards in contains/parent helpers (try/except) to skip invalid networks without failing the entire filter.

  • Breaking (GraphQL) - AggregateFilter: Aggregate.prefix is a concrete field, not a relation. Replace the nested PrefixFilter with:

    • prefix: FilterLookup[str] | None for equality/string lookups, and
    • a field‑level contains(...) applying prefix__net_contains.

    This changes the input shape for aggregates:

    • Before: aggregate_list(filters: { prefix: { contains: "10.0.0.0/8" } })
    • After: containment → aggregate_list(filters: { contains: ["10.0.0.0/8"] }); equality → aggregate_list(filters: { prefix: { exact: "10.0.0.0/8" } })

Scope of this bug report

  • Only ipam.graphql.filters appear affected by the prefix/nesting issue in this pass.

If this direction looks good, I’m happy to open a PR.

@pheus commented on GitHub (Oct 9, 2025): I’d like to **propose** the following solution to address the nested GraphQL filtering issues (scoped to `ipam.graphql.filters`): **Proposed changes** - Make custom filter methods consistently **prefix‑aware** when building `Q(**{f"{prefix}…": …})` (e.g., `IPAddressFilter.assigned`) so nested filters target the correct model. - Add **input safeguards** in `contains`/`parent` helpers (`try/except`) to **skip invalid networks** without failing the entire filter. - **Breaking (GraphQL) - `AggregateFilter`:** `Aggregate.prefix` is a **concrete field**, not a relation. Replace the nested `PrefixFilter` with: - `prefix: FilterLookup[str] | None` for equality/string lookups, and - a field‑level `contains(...)` applying `prefix__net_contains`. This changes the input shape for aggregates: - **Before:** `aggregate_list(filters: { prefix: { contains: "10.0.0.0/8" } })` - **After:** containment → `aggregate_list(filters: { contains: ["10.0.0.0/8"] })`; equality → `aggregate_list(filters: { prefix: { exact: "10.0.0.0/8" } })` **Scope of this bug report** - Only `ipam.graphql.filters` appear affected by the prefix/nesting issue in this pass. If this direction looks good, I’m happy to open a PR.
adam commented 2025-12-29 21:48:52 +01:00
Author
Owner
Copy Link

@jnovinger commented on GitHub (Oct 10, 2025):

Thanks, @pheus , assigned to you.

@jnovinger commented on GitHub (Oct 10, 2025): Thanks, @pheus , assigned to you.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label netbox severity: low status: accepted type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#11706
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?