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 Group issues after object modification #3949

Closed
mzbroch opened this issue Jun 19, 2023 · 3 comments · Fixed by #4366
Closed

Dynamic Group issues after object modification #3949

mzbroch opened this issue Jun 19, 2023 · 3 comments · Fixed by #4366
Assignees
Labels
type: bug Something isn't working as expected

Comments

@mzbroch
Copy link
Contributor

mzbroch commented Jun 19, 2023

Environment

  • Nautobot version (Docker tag too if applicable): 1.5.16
  • Python version:
  • Database platform, version:
  • Middleware(s):

Steps to Reproduce

  1. Create two Platform objects:
- name: juniper_junos
  slug: juniper_junos
- name: cisco_ios
  slug: cisco_ios
  1. Create dynamic group for Device object , in a Platform field select two created platforms.
Screenshot 2023-06-19 at 19 32 06
  1. Modify Platform slug and name: change cisco_ios to cisco-ios (dash instead of underscore)

  2. Attempt to modify dynamic group

  3. After modifying Platform, cisco-ios is now not visible in a form.

Screenshot 2023-06-19 at 19 33 23

Expected Behavior

Clicking "Update" button results in modifying the dynamic group.

Observed Behavior

Clicking "Update" button and saving dynamic group results with an error:

Screenshot 2023-06-19 at 19 34 12
@bryanculver bryanculver added the type: bug Something isn't working as expected label Jun 20, 2023
@lampwins
Copy link
Member

@glennmatthews
Copy link
Contributor

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/django/forms/models.py", line 413, in _post_clean
    self.instance.full_clean(exclude=exclude, validate_unique=False)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/base.py", line 1251, in full_clean
    raise ValidationError(errors)

During handling of the above exception ({'platform': ['Select a valid choice. cisco-ios is not one of the available choices.']}), another exception occurred:
  File "/usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.10/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/views/generic/base.py", line 70, in view
    return self.dispatch(request, *args, **kwargs)
  File "/source/nautobot/core/views/generic.py", line 419, in dispatch
    return super().dispatch(request, *args, **kwargs)
  File "/source/nautobot/utilities/views.py", line 114, in dispatch
    return super().dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/views/generic/base.py", line 98, in dispatch
    return handler(request, *args, **kwargs)
  File "/source/nautobot/extras/views.py", line 616, in post
    if form.is_valid():
  File "/usr/local/lib/python3.10/site-packages/django/forms/forms.py", line 175, in is_valid
    return self.is_bound and not self.errors
  File "/usr/local/lib/python3.10/site-packages/django/forms/forms.py", line 170, in errors
    self.full_clean()
  File "/usr/local/lib/python3.10/site-packages/django/forms/forms.py", line 374, in full_clean
    self._post_clean()
  File "/usr/local/lib/python3.10/site-packages/django/forms/models.py", line 415, in _post_clean
    self._update_errors(e)
  File "/usr/local/lib/python3.10/site-packages/django/forms/models.py", line 389, in _update_errors
    self.add_error(None, errors)
  File "/usr/local/lib/python3.10/site-packages/django/forms/forms.py", line 343, in add_error
    raise ValueError(

Exception Type: ValueError at /extras/dynamic-groups/test/edit/
Exception Value: 'DynamicGroupForm' has no field named 'platform'.

@glennmatthews glennmatthews self-assigned this Aug 31, 2023
@glennmatthews
Copy link
Contributor

glennmatthews commented Aug 31, 2023

For context, DynamicGroup updates via the UI are done through multiple forms rather than a single unified form class - DynamicGroupForm only understands/validates/updates the base class attribute (name, content_type, etc.) while the filter is updated via a form dynamically constructed by instance.generate_filter_form(). The issue here is that because the underlying data changed, the existing DynamicGroup instance's filter data is no longer valid (in this specific case it references a slug that no longer exists, but the same issue would happen for e.g. a UUID-based filter that referenced a now-deleted object), and so when the DynamicGroupForm.is_valid() is called, calling the instance.full_clean() fails before the generated "filter_form" has a chance to update the instance to a new (valid) filter value.

Most likely the required fix here will be to change the form definitions and/or code flow in DynamicGroupEditView.post() so that there's only a single form class/object that handles the updating/validation of the entire DynamicGroup instance data in a single pass. This will be pretty invasive code-wise but I think it's necessary.

Changing the model clean/clean_fields/full_clean logic turned out to be a cleaner way to accomplish this specific fix.

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
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants