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

Fixes #11460 - Fix unterminated cable exception when editing cable #15813

Merged

Conversation

DanSheps
Copy link
Member

Fixes: #11460 - Fix unterminated cable exception when editing cable

  • Fixes the exception when a single cable is not present
  • Fixes the exception when both cables aren't present (but does not allow user to proceed)

@DanSheps DanSheps changed the base branch from develop to feature April 23, 2024 05:53
@DanSheps
Copy link
Member Author

DanSheps commented Apr 23, 2024

There are other options here for this, right now, this is the path of least resistance without a re-work of the form rendering logic to only partially render specific fields.

Another caveat, for some reason <button> does not send in the param for a hx-get so it requires the get url to be specified, so there isn't an easy way to account for both type being missing

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

Nice work @DanSheps. However, as this is an open bug in v3.7 we need to fix it in the develop branch (which shouldn't require much change).

If you haven't already, look at how we handle type selection for remote data sources dynamically; we probably want to follow the same pattern here.

netbox/dcim/views.py Outdated Show resolved Hide resolved
netbox/dcim/views.py Outdated Show resolved Hide resolved
netbox/dcim/views.py Outdated Show resolved Hide resolved
netbox/dcim/views.py Outdated Show resolved Hide resolved
@DanSheps DanSheps changed the base branch from feature to develop April 23, 2024 16:50
@DanSheps DanSheps changed the base branch from develop to feature April 23, 2024 16:50
@DanSheps DanSheps force-pushed the 11460-fix_unterminated_cable_throws_exception branch from 22f5825 to 85db007 Compare April 23, 2024 17:34
@DanSheps DanSheps changed the base branch from feature to develop April 23, 2024 19:11
@DanSheps DanSheps requested a review from jeremystretch April 23, 2024 20:20
@DanSheps
Copy link
Member Author

DanSheps commented Apr 23, 2024

Updated to develop but I also switched around the rending a bit. Instead of using the hacky method I was using and pulling down the full page as an HTMX and then selecting I did the following:

  1. Modify netbox/views/generic/object_views.py to have an htmx_template_name attribute for the ObjectEditView.
  2. Defaulted that to the regular htmx form (htmx/form.html)
  3. Overrode the htmx_template_name in the CableEditView
  4. Broke the form (dcim/cable_edit.html) out into an new file in a new "htmx" folder (form is at dcim/htmx/cable_edit.html)
  5. Changed the htmx request around slightly

I was thinking about this last night, I like it more because it is extendable and I think easier, just wasn't sure which way to go.

@arthanson arthanson self-requested a review April 29, 2024 16:28
@arthanson
Copy link
Collaborator

@DanSheps the edit form comes up, but if you just select save without any changes an exception is thrown:

 File "/Users/ahanson/dev/work/netbox/netbox/utilities/forms/mixins.py", line 55, in is_valid
    is_valid = super().is_valid()
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.10/site-packages/django/forms/forms.py", line 201, in is_valid
    return self.is_bound and not self.errors
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.10/site-packages/django/forms/forms.py", line 196, in errors
    self.full_clean()
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.10/site-packages/django/forms/forms.py", line 434, in full_clean
    self._clean_form()
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.10/site-packages/django/forms/forms.py", line 455, in _clean_form
    cleaned_data = self.clean()
  File "/Users/ahanson/dev/work/netbox/netbox/dcim/forms/connections.py", line 103, in clean
    self.instance.a_terminations = self.cleaned_data['a_terminations']
KeyError: 'a_terminations'

@DanSheps
Copy link
Member Author

Thanks Arthur, didn't think to check the next step. Will try and figure out something for that.

@arthanson
Copy link
Collaborator

@jeremystretch looks like your changes fixed the issue I had previously

@arthanson
Copy link
Collaborator

arthanson commented May 1, 2024

One weirdness - go to edit a cable and click the delete 'x' next to the Type in the "B Side", the form auto-refreshes and any Type, device and interface are reset. You can see it better by first deleting interface, then when you delete Type interface and Type will be reset back.

@jeremystretch
Copy link
Member

This implementation isn't quite perfect, but it does at least resolve #11460 without (AFAICT) breaking any existing functionality. I'm going to merge this as-is to resolve the immediate issues, and we can iterate on improvements separately from there.

@jeremystretch jeremystretch merged commit c08784d into develop May 1, 2024
8 checks passed
@jeremystretch jeremystretch deleted the 11460-fix_unterminated_cable_throws_exception branch May 1, 2024 18:24
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2024
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.

Editing an unterminated cable throws exception
3 participants