Skip to content

Conversation

@renatoalmeidaoliveira
Copy link
Contributor

@renatoalmeidaoliveira renatoalmeidaoliveira commented Mar 19, 2025

Fixes: #18933 VLANGroupFilterSet site_group field doesn't match VLANGroupFilterForm sitegroup field

Fixes site_group field typo, it was defined as sitegroup in the FilterForm and as site_group in the FilterSet
Followed site_group pattern as it was already used in other places like:
ProviderFilterSet, CircuitTerminationFilterSet, LocationFilterSet and several others

@renatoalmeidaoliveira renatoalmeidaoliveira requested review from a team and jnovinger and removed request for a team March 19, 2025 00:36
@jnovinger
Copy link
Member

@renatoalmeidaoliveira , just to be clear, this fix should only enable filtering VLANGroups with a scope directly tied to a specific SiteGroup, correct?

That is, it should not also be filtering on site groups via a link between a VLAN group and a particular site that is part of a site group, right?

@renatoalmeidaoliveira
Copy link
Contributor Author

@jnovinger, in the current implementation the site_group is filtering only for VlanGroups that has some SiteGroup as scope.
In other models, that has a site attribut, filtering by site_group filters all models that has a site in some site_group.

@renatoalmeidaoliveira
Copy link
Contributor Author

But since you can assign scopes, and a SiteGroup is a valid scope for VlanGroup model, IMO, filtering for both sites, and SiteGroup may not be the best option, but please let me know if you prefer to change the FilterSet to include sites that belongs to that SiteGroup too.
I think that culd be possible by adding a Q with an OR in the queryset.

@jnovinger
Copy link
Member

Nope, not necessarily a preference. Just wanted to make sure I understood the scope of this change correctly! I didn't want to kick it back if it wasn't part of the scope. Thanks!

@jnovinger jnovinger merged commit d332a0c into netbox-community:main Mar 20, 2025
3 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VLANGroupFilterSet site_group field doesn't match VLANGroupFilterForm sitegroup field

2 participants