Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Groups with nested object filters like Location don't work in child groups #2693

Closed
jathanism opened this issue Oct 27, 2022 · 0 comments · Fixed by #2695
Closed
Assignees
Labels
type: bug Something isn't working as expected
Milestone

Comments

@jathanism
Copy link
Contributor

jathanism commented Oct 27, 2022

Environment

  • Nautobot version (Docker tag too if applicable): 1.4.7
  • Python version: 3.10
  • Database platform, version: n/a
  • Middleware(s): n/a

Steps to Reproduce

This is a follow-up to #2665, which fixed the ability for Dynamic Groups to filter properly on nested object types like Location and Region by augmenting TreeNodeMultipleChoiceFilter.filter() to iterate the tree descendants (e.g. Location A --> Location B).

Create Locations

  1. Create a LocationType called My Region and content-type dcim | device
  2. Create a LocationType called My Site with parent set to LocationType("MyRegion") and content-type dcim | device

image

  1. Create a new Location with LocationType of My Region called Location A with site AMS01 (which is part of the demo dataset)
  2. Create a new Location with LocationType of My Region --> MySite called Location B with parent Location A

image

Create Devices

  1. Create a new Device named new-device1 with site AMS01 and location set to Location A
  2. Create a new Device named new-device2 with site AMS01 and location set to Location B

image

Create Dynamic Groups

  1. Create a DynamicGroup with a filter for a location = "Location A" named devices-location-a

image

  1. Create a DynamicGroup with an empty filter as a parent group named location-parent
  2. Edit location-parent and set the first child group to devices-location-a with operator Include (OR).

image

Expected Behavior

  • Navigate to Dynamic Groups list view
  • Observe that devices-location-a has 2 members
  • Observe that location-parent also has 2 members

Observed Behavior

  • Observe that devices-location-a has 2 members
  • Observe that location-parent only has 1 member

What is happening in the workaround from #2665 is:

  • DynamicGroup.get_queryset() only uses the filterset to identify membership
  • DynamicGroup.get_group_queryset() instead relies on aggregated generated Q() objects derived from all child groups
  • DynamicGroup.members was augmented to return get_queryset() if there are no child groups as a workaround to make filtering on tree objects like Location work, as evident in devices-location-2 having 2 members from Location A and Location B (which is a child of Location A)
  • Nested Dynamic Groups with children still fallback to the old behavior
  • The root cause is in the difference between forward lookup using filtersets (e.g. {"location": ["location-a"]}. This will filter as expected because the TreeNodeMultipleChoiceFilter is doing recursive descent for all child locations...
  • Compared to the reverse lookup, which is using generated Q() objects, (e.g. location__slug='location-a') which you can see IS NOT recursively descending. In the case of the location-parent, observe that the correct generated Q object should be (location__slug='location-a' OR location__slug='location-b').

TL;DR

  • Simple dynamic groups (no children) use forward filtering based on filtersets
  • Advanced dynamic groups (with children) use reverse filtering based on Q objects
  • TreeNodeMultipleChoiceFilter needs to be augmented to correctly generate a nested Q object for any descendants using the generate_query() pattern (See: SearchFilter.generate_query()
  • DynamicGroup.generate_query_for_filter() needs to be augmented to look for generate_query() methods on Filter instances when it is building nested Q objects for advanced dynamic group. It currently type checks if the filter is an instance of django_filters.MultipleChoiceFilter and calls get_filter_predicate() for each value, but this isn't sufficient for tree node filters.
@jathanism jathanism added the type: bug Something isn't working as expected label Oct 27, 2022
@jathanism jathanism added this to the v1.5.0 milestone Oct 27, 2022
@jathanism jathanism self-assigned this Oct 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working as expected
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant