Skip to content

Conversation

@rinna11
Copy link

@rinna11 rinna11 commented Oct 20, 2025

Fixes: #20422

This allows AggregateFilter and PrefixFilter to filter by their family similar with IPAddressFilter.
I've tested the following queries manually:

aggregate_v6: aggregate_list(filters: {family: FAMILY_6}) {
    prefix
}
prefixes_v4: prefix_list(filters: {family: FAMILY_4}) {
    prefix
    role {
      name
    }
    vrf {
      name
    }
    vlan {
      vid
    }
}

@jnovinger jnovinger requested review from a team and jnovinger and removed request for a team October 20, 2025 02:27
Copy link
Member

@jnovinger jnovinger left a comment

Choose a reason for hiding this comment

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

Thanks for the work, @rinna11. The approach looks good and correctly follows the existing pattern.

Please add tests covering the family filter for both AggregateFilter and PrefixFilter, with both FAMILY_4 and FAMILY_6 values to verify the filter returns the correct address family results.

You can add custom test methods to the AggregateTest and PrefixTest classes in ipam/tests/test_api.py. These inherit from APIViewTestCase which provides self.client, self.user, and other helpers. Please mark these tests with @tag('regression'), which you can see elsewhere in the same file. For examples, see the templated tests that are defined in netbox/tests/test_graphql.py.

@rinna11
Copy link
Author

rinna11 commented Oct 23, 2025

Hi @jnovinger , thank you for the suggestion!
I had a quick look at ipam/tests/test_api.py, and it seems there aren’t any existing GraphQL filter tests for AggregateTest and PrefixTest there. I did find relevant filtering tests in ipam/tests/test_filtersets.py, and those already include conditions for filtering by family. These tests are passing both in the current codebase and with this PR.
Also, when the family field was enabled for IPAddressFilter in #19621 , adding specific filter tests wasn’t required at that time either.

Given that, I would like to kindly ask if we could proceed with merging this PR without adding new tests. Please let me know if you're still up for it after all that—happy to adjust!
Thanks again for your time and review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GraphQL StrFilterLookup filter on prefix field causes unexpected error with IPNetworkField

2 participants