Skip to content

Regression from APISelect option optimization #3831

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

Closed
hSaria opened this issue Jan 3, 2020 · 6 comments
Closed

Regression from APISelect option optimization #3831

hSaria opened this issue Jan 3, 2020 · 6 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@hSaria
Copy link
Contributor

hSaria commented Jan 3, 2020

Environment

  • Python version: 3.6.8
  • NetBox version: 2.6.10

Steps to Reproduce

  1. Go to IP addresses page (/ipam/ip-addresses/)
  2. Only the filter form, select a VRF

There are multiple places that this is happening at. This is one example.

Following #3812, it seems I messed up some APISelect-based fields and I'm still unsure what's affected and what's not (I'll put a comment with more info).

Expected Behavior

The list of VRFs is shown

Observed Behavior

Select is stuck at Searching...

@hSaria
Copy link
Contributor Author

hSaria commented Jan 3, 2020

This seems to be related to the null option not being present, or rather no options being present at all.

Firstly, if you make the following change, the VRF field and most others start working:

--- a/netbox/utilities/templates/widgets/select_api.html
+++ b/netbox/utilities/templates/widgets/select_api.html
@@ -1,5 +1,5 @@
 <select name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>{% for group_name, group_choices, group_index in widget.opt
-  <optgroup label="{{ group_name }}">{% endif %}{% for widget in group_choices %}{% if widget.attrs.selected %}
+  <optgroup label="{{ group_name }}">{% endif %}{% for widget in group_choices %}{% if widget.attrs.selected or widget.value == "null" %}
   {% include widget.template_name %}{% endif %}{% endfor %}{% if group_name %}
   </optgroup>{% endif %}{% endfor %}
 </select>

However, there is still at least one field that I found not working: the Rack group field in Racks (/dcim/racks/) and I have no idea why that one specifically is there.

I'm not 100% sure, but I think any static options (API will not return) will need to be put into the page (not filtered by the template). If it is just the null option that is always the exception, then the above diff will fix it, but if there instances where we mix multiple static options with an APISelect-based field, the patch won't work.

But again, the patch above still doesn't fix the Rack group field in Racks, and I can't figure out why.

@hSaria
Copy link
Contributor Author

hSaria commented Jan 3, 2020

I think I found out why rack groups wasn't working: the default option has no value, so this fixes it

--- a/netbox/utilities/templates/widgets/select_api.html
+++ b/netbox/utilities/templates/widgets/select_api.html
@@ -1,5 +1,5 @@
 <select name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>{% for group_name, group_choices, group_index in widget.optgroups %}{% if group_name %}
-  <optgroup label="{{ group_name }}">{% endif %}{% for widget in group_choices %}{% if widget.attrs.selected %}
+  <optgroup label="{{ group_name }}">{% endif %}{% for widget in group_choices %}{% if widget.attrs.selected or widget.value == "null" or not widget.value %}
   {% include widget.template_name %}{% endif %}{% endfor %}{% if group_name %}
   </optgroup>{% endif %}{% endfor %}
 </select>

I don't know if this is the best or most efficient solution considering I'm accounting for two exception manually

@hoalex
Copy link

hoalex commented Jan 3, 2020

This is also happening on the Devices page when trying to filter for a specific rack.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application labels Jan 3, 2020
@jeremystretch jeremystretch self-assigned this Jan 3, 2020
@jeremystretch
Copy link
Member

Going to stick with option.value == "null" for now, until we have time to do some thorough cleanup around the select widgets. RackFilter.group_id (and possibly other fields) need to be changed to FilterChoiceField to populate null values correctly.

@jeremystretch jeremystretch pinned this issue Jan 3, 2020
@jeremystretch jeremystretch unpinned this issue Jan 3, 2020
@hSaria
Copy link
Contributor Author

hSaria commented Jan 3, 2020

@jeremystretch thanks for fixing and sorry for the mess.

@jeremystretch
Copy link
Member

@hSaria No worries, I didn't catch it either.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

3 participants