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

3.6.4 Regression rendering hidden input for checkbox breaks forms.BooleanField in Filters #16293

Closed
ibuclaw opened this issue May 24, 2024 · 5 comments

Comments

@ibuclaw
Copy link
Contributor

ibuclaw commented May 24, 2024

Deployment Type

Self-hosted

NetBox Version

v4.0.3

Python Version

3.11

Steps to Reproduce

  1. Modify sources to add a forms.BooleanField parameter to a Filters form, e.g:
--- a/netbox/dcim/forms/filtersets.py
+++ b/netbox/dcim/forms/filtersets.py
@@ -162,11 +162,16 @@ class SiteFilterForm(TenancyFilterForm, ContactModelFilterForm, NetBoxModelFilte
     model = Site
     fieldsets = (
         FieldSet('q', 'filter_id', 'tag'),
+        FieldSet('boolean_field', name=_('Debug')),
         FieldSet('status', 'region_id', 'group_id', 'asn_id', name=_('Attributes')),
         FieldSet('tenant_group_id', 'tenant_id', name=_('Tenant')),
         FieldSet('contact', 'contact_role', 'contact_group', name=_('Contacts')),
     )
     selector_fields = ('filter_id', 'q', 'region_id', 'group_id')
+    boolean_field = forms.BooleanField(
+        label=_('Test field'),
+        required=False
+    )
     status = forms.MultipleChoiceField(
         label=_('Status'),
         choices=SiteStatusChoices,
  1. Restart NetBox and visit the page (in this case /dcim/sites/ -> Filters
  2. Inspect generated html of the field.
<div class="form-check mb-0">
    <input type="hidden" name="boolean_field" value="">
    <input type="checkbox" name="boolean_field" id="id_boolean_field" class="form-check-input">
    <label for="id_boolean_field" class="form-check-label">Test field</label>
</div>

Expected Behavior

NetBox 3.7 rendered this as something like:

<div class="form-check">
    <input type="checkbox" name="boolean_field" class="form-check-input" placeholder="Test field" id="id_boolean_field">
    <label for="id_boolean_field" class="form-check-label">Test field</label>
</div>

Observed Behavior

There is a hidden input with the same name that did not exist in 3.7.x.

As far as I can tell, this means BooleanField's are ignored due to this GET event listener deleting all form data for it, because the "hidden" input has an empty value.

const formData: FormData = event.formData;
for (const [name, value] of Array.from(formData.entries())) {
if (value === '') formData.delete(name);
}

@ibuclaw ibuclaw added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels May 24, 2024
@ibuclaw
Copy link
Contributor Author

ibuclaw commented May 24, 2024

I suspect it was introduced by a46255d

So I can't say why I'm not seeing this in 3.7.x then - maybe plug-ins pick up the upstream Django template, and not the Netbox vendored version, and now this is no longer the case in 4.x.

@ibuclaw
Copy link
Contributor Author

ibuclaw commented May 25, 2024

Ah-ha! I don't see it in plug-ins as they are fudging the UI because of this bug.

netbox-community/netbox-topology-views#408

@ibuclaw ibuclaw changed the title 4.0 Regression rendering input type checkbox from forms.BooleanField in Filters 3.6.4 Regression rendering input type checkbox from forms.BooleanField in Filters May 25, 2024
@ibuclaw ibuclaw changed the title 3.6.4 Regression rendering input type checkbox from forms.BooleanField in Filters 3.6.4 Regression rendering hidden input for checkbox breaks forms.BooleanField in Filters May 25, 2024
@jeremystretch
Copy link
Member

Arbitrarily modifying core code does not constitute a valid bug report. Please modify your post above to describe the specific behavior in NetBox you believe is unintentional.

@jeremystretch jeremystretch added status: revisions needed This issue requires additional information to be actionable and removed status: needs triage This issue is awaiting triage by a maintainer labels May 28, 2024
@ibuclaw
Copy link
Contributor Author

ibuclaw commented May 28, 2024

It presents - in a bite-size example - code that already exists in plugins that "NetBox Cloud and NetBox Enterprise include commercial support for..." are doing, and NetBox is breaking them by injecting bad html for this field type.

There already one hacky condition in the template because some other part of NetBox got broke by it.

{% if widget.name != '_selected_action' %}<input type="hidden" name="{{ widget.name }}" value="">{% endif %}

{% if widget.name != '_selected_action' %}

If this was instead something along the lines of

{% if htmx_enabled %}

Then that would resolve the issue - if I correctly understand the original intention for introducing this hidden input field.

@netbox-community netbox-community deleted a comment from ibuclaw Jun 3, 2024
@jeremystretch
Copy link
Member

@ibuclaw the code you reference was added intentionally and serves a purpose. If you would like to propose an alternative solution, please submit a feature request citing the detailed proposal and justification for the change.

I'm closing this issue as it does not identify unexpected, reproducible behavior in NetBox itself.

@jeremystretch jeremystretch closed this as not planned Won't fix, can't repro, duplicate, stale Jun 3, 2024
@jeremystretch jeremystretch removed their assignment Jun 3, 2024
@jeremystretch jeremystretch removed type: bug A confirmed report of unexpected behavior in the application status: revisions needed This issue requires additional information to be actionable labels Jun 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 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 a pull request may close this issue.

2 participants