feat(ui): Scroll to the first invalid form field

A rejected form re-rendered at the top of the page with no sign of what
had failed. The first field carrying aria-invalid is now focused and
scrolled into view, for full-page submissions and HTMX-swapped forms
alike. Form method is matched by attribute, since a control named
"method" shadows the property.

Fixes #22949

netbox/netbox/tests/test_views.py

@@ -3,10 +3,13 @@ from contextlib import contextmanager
 from unittest.mock import patch
 
 from django.contrib.contenttypes.models import ContentType
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.http import HttpResponse
 from django.test import Client, TransactionTestCase, override_settings
 from django.urls import reverse
+from django.utils import timezone
 
+from core.models import DataFile, DataSource
 from dcim.choices import DeviceStatusChoices, InterfaceTypeChoices, SiteStatusChoices
 from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site, VirtualChassis
 from extras.events import enqueue_event
@@ -14,6 +17,7 @@ from extras.models import ImageAttachment
 from extras.validators import CustomValidator
 from ipam.choices import VLANStatusChoices
 from ipam.models import VLAN, VLANGroup
+from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, ImportMethodChoices
 from netbox.constants import EMPTY_TABLE_TEXT
 from netbox.search.backends import search_backend
 from users.models import User
@@ -29,6 +33,103 @@ class HomeViewTestCase(TestCase):
         self.assertHttpStatus(response, 200)
 
 
+class BulkImportViewTabsTestCase(TestCase):
+    """
+    Verify which bulk import tab is rendered active.
+    """
+
+    def test_get_activates_direct_import(self):
+        """An initial GET activates the Direct Import tab."""
+        self.add_permissions('dcim.add_site')
+
+        response = self.client.get(reverse('dcim:site_bulk_import'))
+
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        self.assertIn('<div class="tab-pane show active" id="import-form"', content)
+        self.assertIn('<div class="tab-pane show" id="upload-form"', content)
+
+    def test_rejected_upload_activates_its_tab(self):
+        """A rejected file upload activates the Upload File tab rather than the default."""
+        self.add_permissions('dcim.add_site')
+        upload_file = SimpleUploadedFile('sites.csv', b'no delimiters here', content_type='text/plain')
+
+        response = self.client.post(reverse('dcim:site_bulk_import'), {
+            'import_method': ImportMethodChoices.UPLOAD,
+            'upload_file': upload_file,
+            'format': ImportFormatChoices.AUTO,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        self.assertIn('<div class="tab-pane show active" id="upload-form"', content)
+        self.assertIn('<div class="tab-pane show" id="import-form"', content)
+
+    def test_rejected_json_upload_reports_on_the_upload_field(self):
+        """A malformed JSON upload reports its parse error inside the Upload File tab."""
+        self.add_permissions('dcim.add_site')
+        upload_file = SimpleUploadedFile('sites.json', b'{', content_type='application/json')
+
+        response = self.client.post(reverse('dcim:site_bulk_import'), {
+            'import_method': ImportMethodChoices.UPLOAD,
+            'upload_file': upload_file,
+            'format': ImportFormatChoices.AUTO,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        self.assertIn('<div class="tab-pane show active" id="upload-form"', content)
+        self.assertIn('id="id_upload_file_errors"', content)
+        self.assertIn('Invalid JSON data', content)
+
+    def test_rejected_json_data_file_reports_on_the_data_file_field(self):
+        """A malformed JSON data file reports its parse error inside the Data File tab."""
+        self.add_permissions('dcim.add_site')
+        data_source = DataSource.objects.create(
+            name='Data Source 1',
+            type='local',
+            source_url='file:///var/tmp/source1/'
+        )
+        data_file = DataFile.objects.create(
+            source=data_source,
+            path='sites.json',
+            last_updated=timezone.now(),
+            size=1,
+            hash='442da078f0111cbdf42f21903724f6597c692535f55bdfbbea758a1ae99ad9e1',
+            data=b'{'
+        )
+
+        response = self.client.post(reverse('dcim:site_bulk_import'), {
+            'import_method': ImportMethodChoices.DATA_FILE,
+            'data_source': data_source.pk,
+            'data_file': data_file.pk,
+            'format': ImportFormatChoices.AUTO,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        self.assertIn('<div class="tab-pane show active" id="datafile-form"', content)
+        self.assertIn('id="id_data_file_errors"', content)
+        self.assertIn('Invalid JSON data', content)
+
+    def test_unknown_import_method_falls_back_to_direct(self):
+        """An unrecognised import method leaves the Direct Import tab active."""
+        self.add_permissions('dcim.add_site')
+
+        response = self.client.post(reverse('dcim:site_bulk_import'), {
+            'import_method': 'bogus',
+            'format': ImportFormatChoices.AUTO,
+            'csv_delimiter': CSVDelimiterChoices.AUTO,
+        })
+
+        self.assertHttpStatus(response, 200)
+        content = response.content.decode()
+        self.assertIn('<div class="tab-pane show active" id="import-form"', content)
+
+
 class SearchViewTestCase(TestCase):
 
     @classmethod

netbox/netbox/views/generic/bulk_views.py

@@ -24,6 +24,7 @@ from core.models import ObjectType
 from core.signals import clear_events
 from extras.choices import CustomFieldUIEditableChoices
 from extras.models import CustomField, ExportTemplate
+from netbox.choices import ImportMethodChoices
 from netbox.forms.bulk_rename import NetBoxModelBulkRenameForm
 from netbox.models.features import ChangeLoggingMixin
 from netbox.object_actions import AddObject, BulkDelete, BulkEdit, BulkExport, BulkImport, BulkRename
@@ -724,6 +725,7 @@ class BulkImportView(GetReturnURLMixin, BaseMultiObjectView):
             'model': model,
             'form': form,
             'fields': self._get_form_fields(),
+            'import_method': ImportMethodChoices.DIRECT,
             'return_url': self.get_return_url(request),
             **self.get_extra_context(request),
         })
@@ -787,6 +789,8 @@ class BulkImportView(GetReturnURLMixin, BaseMultiObjectView):
             'model': model,
             'form': form,
             'fields': self._get_form_fields(),
+            # Return the user to the tab they submitted, so a rejected import shows its own error
+            'import_method': form.cleaned_data.get('import_method') or ImportMethodChoices.DIRECT,
             'return_url': self.get_return_url(request),
             **self.get_extra_context(request),
         })

netbox/project-static/src/forms/errors.ts

@@ -0,0 +1,69 @@
+import type TomSelect from 'tom-select';
+
+type TomSelectElement = HTMLSelectElement & { tomselect?: TomSelect };
+
+const INVALID_SELECTOR = '[aria-invalid="true"]';
+const CONTROL_SELECTOR = 'input:not([type="hidden"]), select, textarea';
+// Matched by attribute because a control named "method" shadows the property.
+const POST_FORM_SELECTOR = 'form[method="post" i]';
+
+/**
+ * Focus the control the server flagged. Tom Select hides the native <select> and overrides
+ * focus(), so its instance owns focus for enhanced fields.
+ */
+function focusControl(element: HTMLElement): void {
+  const instance = (element as TomSelectElement).tomselect;
+  if (instance) {
+    instance.focus();
+    return;
+  }
+
+  // A disabled control still gets its row revealed, it just cannot take focus.
+  if (element.matches(CONTROL_SELECTOR)) {
+    const control = element as FormControls;
+    if (!control.disabled) control.focus({ preventScroll: true });
+    return;
+  }
+
+  // Grouped widgets (radios, checkbox lists) carry the attribute on their wrapper.
+  for (const control of element.querySelectorAll<FormControls>(CONTROL_SELECTOR)) {
+    if (!control.disabled) {
+      control.focus({ preventScroll: true });
+      return;
+    }
+  }
+}
+
+/**
+ * Reveal the first field within `root` that the server marked invalid. Returns true once one
+ * has been revealed.
+ */
+export function focusFirstFormError(root: ParentNode): boolean {
+  for (const invalid of root.querySelectorAll<HTMLElement>(INVALID_SELECTOR)) {
+    const row = invalid.closest<HTMLElement>('.row') ?? invalid;
+
+    // A row in a collapsed tab pane or any display:none container has no box to reveal.
+    if (row.getClientRects().length === 0) continue;
+
+    focusControl(invalid);
+    // Scroll last so it wins over the browser's own scroll-on-focus.
+    row.scrollIntoView({ block: 'center', inline: 'nearest', behavior: 'instant' });
+    return true;
+  }
+
+  return false;
+}
+
+/**
+ * Reveal the first invalid field in a submitting form on the page. GET forms are skipped so a
+ * rejected list filter cannot take focus away from the page content.
+ */
+export function focusFirstPageFormError(): boolean {
+  for (const form of document.querySelectorAll<HTMLFormElement>(POST_FORM_SELECTOR)) {
+    if (focusFirstFormError(form)) {
+      return true;
+    }
+  }
+
+  return false;
+}

netbox/project-static/src/htmx.ts

@@ -6,6 +6,16 @@ import { initObjectSelector } from './objectSelector';
 import { initBootstrap } from './bs';
 import { initMessages } from './messages';
 import { initQuickAdd } from './quickAdd';
+import { focusFirstFormError } from './forms/errors';
+
+type HtmxSettleDetail = {
+  elt?: Element;
+  target?: Element;
+  requestConfig?: {
+    verb?: string;
+    triggeringEvent?: Event;
+  };
+};
 
 function initDepedencies(): void {
   initButtons();
@@ -18,10 +28,32 @@ function initDepedencies(): void {
   initMessages();
 }
 
+/**
+ * Reveal validation errors only for a swapped-in form submission, scoped to the swapped content.
+ * Dependent-field refreshes post on `change` and re-render a bound form, so gating on the verb
+ * alone would drag focus away from the field the user is editing.
+ */
+function revealSubmissionErrors(event: Event): void {
+  const { detail } = event as CustomEvent<HtmxSettleDetail>;
+  const trigger = detail?.requestConfig?.triggeringEvent?.type;
+  const verb = detail?.requestConfig?.verb;
+
+  if (trigger !== 'submit' || verb === 'get') return;
+
+  // An outerHTML swap detaches the original target, so fall back to the settled element.
+  const root = detail.target?.isConnected ? detail.target : detail.elt;
+  if (root?.isConnected) {
+    focusFirstFormError(root);
+  }
+}
+
 /**
  * Hook into HTMX's event system to reinitialize specific native event listeners when HTMX swaps
  * elements.
  */
 export function initHtmx(): void {
-  document.addEventListener('htmx:afterSettle', initDepedencies);
+  document.addEventListener('htmx:afterSettle', event => {
+    initDepedencies();
+    revealSubmissionErrors(event);
+  });
 }

netbox/project-static/src/netbox.ts

@@ -15,6 +15,10 @@ import { initHtmx } from './htmx';
 import { initSavedFilterSelect } from './forms/savedFiltersSelect';
 import { initHotkeys } from './hotkeys';
 import { initSSOForms } from './sso';
+import { focusFirstPageFormError } from './forms/errors';
+
+// Anything but post or dialog is a GET, and a control named "method" shadows the property.
+const GET_FORM_SELECTOR = 'form:not([method="post" i]):not([method="dialog" i])';
 
 function initDocument(): void {
   for (const init of [
@@ -41,22 +45,22 @@ function initDocument(): void {
 }
 
 function initWindow(): void {
-  const documentForms = document.forms;
-  for (const documentForm of documentForms) {
-    if (documentForm.method.toUpperCase() == 'GET') {
-      documentForm.addEventListener('formdata', function (event: FormDataEvent) {
-        const formData: FormData = event.formData;
-        for (const [name, value] of Array.from(formData.entries())) {
-          if (value === '') formData.delete(name);
-        }
-      });
-    }
+  for (const documentForm of document.querySelectorAll<HTMLFormElement>(GET_FORM_SELECTOR)) {
+    documentForm.addEventListener('formdata', function (event: FormDataEvent) {
+      const formData: FormData = event.formData;
+      for (const [name, value] of Array.from(formData.entries())) {
+        if (value === '') formData.delete(name);
+      }
+    });
   }
 
-  const contentContainer = document.querySelector<HTMLElement>('.content-container');
-  if (contentContainer !== null) {
-    // Focus the content container for accessible navigation.
-    contentContainer.focus();
+  // A rejected submission takes priority over the default landing focus.
+  if (!focusFirstPageFormError()) {
+    const contentContainer = document.querySelector<HTMLElement>('.content-container');
+    if (contentContainer !== null) {
+      // Focus the content container for accessible navigation.
+      contentContainer.focus();
+    }
   }
 }
 

netbox/templates/generic/bulk_import.html

@@ -13,6 +13,7 @@ Context:
   - model:       The model class being imported
   - form:        The bulk import form
   - fields:      A dictionary of form fields, to display import options (optional)
+  - import_method: Which of the three tabs is active ("direct", "upload" or "datafile")
   - return_url:  The URL to which the user is redirected after submitting the form
 {% endcomment %}
 
@@ -21,17 +22,17 @@ Context:
 {% block tabs %}
   <ul class="nav nav-tabs">
     <li class="nav-item" role="presentation">
-      <button class="nav-link active" id="import-form-tab" data-bs-toggle="tab" data-bs-target="#import-form" data-href="#tab_import-form" type="button" role="tab" aria-controls="import-form" aria-selected="true">
+      <button class="nav-link{% if import_method == 'direct' %} active{% endif %}" id="import-form-tab" data-bs-toggle="tab" data-bs-target="#import-form" data-href="#tab_import-form" type="button" role="tab" aria-controls="import-form" aria-selected="{% if import_method == 'direct' %}true{% else %}false{% endif %}">
         {% trans "Direct Import" %}
       </button>
     </li>
     <li class="nav-item" role="presentation">
-      <button class="nav-link" id="upload-form-tab" data-bs-toggle="tab" data-bs-target="#upload-form" data-href="#tab_upload-form" type="button" role="tab" aria-controls="upload-form" aria-selected="false">
+      <button class="nav-link{% if import_method == 'upload' %} active{% endif %}" id="upload-form-tab" data-bs-toggle="tab" data-bs-target="#upload-form" data-href="#tab_upload-form" type="button" role="tab" aria-controls="upload-form" aria-selected="{% if import_method == 'upload' %}true{% else %}false{% endif %}">
         {% trans "Upload File" %}
       </button>
     </li>
     <li class="nav-item" role="presentation">
-      <button class="nav-link" id="datafile-form-tab" data-bs-toggle="tab" data-bs-target="#datafile-form" data-href="#tab_datafile-form" type="button" role="tab" aria-controls="datafile-form" aria-selected="false">
+      <button class="nav-link{% if import_method == 'datafile' %} active{% endif %}" id="datafile-form-tab" data-bs-toggle="tab" data-bs-target="#datafile-form" data-href="#tab_datafile-form" type="button" role="tab" aria-controls="datafile-form" aria-selected="{% if import_method == 'datafile' %}true{% else %}false{% endif %}">
         {% trans "Data File" %}
       </button>
     </li>
@@ -41,7 +42,7 @@ Context:
 {% block content %}
 
   {# Data Import Form #}
-  <div class="tab-pane show active" id="import-form" role="tabpanel" aria-labelledby="import-form-tab">
+  <div class="tab-pane show{% if import_method == 'direct' %} active{% endif %}" id="import-form" role="tabpanel" aria-labelledby="import-form-tab">
     <div class="col col-md-12 col-lg-10 offset-lg-1">
       <form action="" method="post" enctype="multipart/form-data" class="form">
         {% csrf_token %}
@@ -73,7 +74,7 @@ Context:
   </div>
 
   {# File Upload Form #}
-  <div class="tab-pane show" id="upload-form" role="tabpanel" aria-labelledby="upload-form-tab">
+  <div class="tab-pane show{% if import_method == 'upload' %} active{% endif %}" id="upload-form" role="tabpanel" aria-labelledby="upload-form-tab">
     <div class="col col-md-12 col-lg-10 offset-lg-1">
       <form action="" method="post" enctype="multipart/form-data" class="form">
         {% csrf_token %}
@@ -105,7 +106,7 @@ Context:
   </div>
 
   {# DataFile Form #}
-  <div class="tab-pane show" id="datafile-form" role="tabpanel" aria-labelledby="datafile-form-tab">
+  <div class="tab-pane show{% if import_method == 'datafile' %} active{% endif %}" id="datafile-form" role="tabpanel" aria-labelledby="datafile-form-tab">
     <div class="col col-md-12 col-lg-10 offset-lg-1">
       <form action="" method="post" enctype="multipart/form-data" class="form">
         {% csrf_token %}

netbox/utilities/forms/bulk_import.py

@@ -52,10 +52,11 @@ class BulkImportForm(ChangelogMessageMixin, BackgroundJobMixin, SyncedDataMixin,
         if self.cleaned_data['data'] and import_method != ImportMethodChoices.DIRECT:
             raise forms.ValidationError(_("Form data must be empty when uploading/selecting a file."))
         if import_method == ImportMethodChoices.UPLOAD:
-            self.upload_file = 'upload_file'
+            self.data_field = 'upload_file'
             file = self.files.get('upload_file')
             data = file.read().decode('utf-8-sig')
         elif import_method == ImportMethodChoices.DATA_FILE:
+            self.data_field = 'data_file'
             data = self.cleaned_data['data_file'].data_as_string
         else:
             data = self.cleaned_data['data']

netbox/utilities/templatetags/form_helpers.py

@@ -157,8 +157,16 @@ def render_fieldset(form, fieldset):
                     'fields': [form[name] for name in tab['fields'] if name in form.fields]
                 } for tab in item.tabs
             ]
-            # If none of the tabs has been marked as active, activate the first one
-            if not any(tab['active'] for tab in tabs):
+            # A field error wins over initial data so a failed submission is not hidden in a tab
+            errored = next(
+                (tab for tab in tabs if any(field.errors for field in tab['fields'])),
+                None,
+            )
+            if errored is not None:
+                for tab in tabs:
+                    tab['active'] = tab is errored
+            elif not any(tab['active'] for tab in tabs):
+                # If none of the tabs has been marked as active, activate the first one
                 tabs[0]['active'] = True
             rows.append(
                 FieldsetRow('tabs', tabs)

netbox/utilities/tests/test_templatetags.py

@@ -9,7 +9,7 @@ from core.models import ObjectType
 from dcim.models import Site
 from extras.choices import CustomFieldTypeChoices
 from extras.models import CustomField, CustomFieldChoiceSet
-from utilities.forms.rendering import FieldSet, InlineFields
+from utilities.forms.rendering import FieldSet, InlineFields, TabbedGroups
 from utilities.templatetags.builtins.tags import badge, customfield_value, static_with_params
 from utilities.templatetags.form_helpers import any_required, render_field_with_aria, render_fieldset
 from utilities.templatetags.helpers import _humanize_capacity, humanize_speed
@@ -457,3 +457,62 @@ class RenderFieldsetInlineRequiredTestCase(TestCase):
         html = self._render(fieldset)
         # With no help text, the shared help-text row (col offset-3) must not be rendered
         self.assertNotIn('col offset-3', html)
+
+
+class RenderFieldsetTabsTestCase(TestCase):
+    """
+    Verify tab selection for a TabbedGroups row.
+    """
+
+    class TestForm(forms.Form):
+        first = forms.CharField(required=True)
+        second = forms.CharField(required=True)
+
+    fieldset = FieldSet(
+        TabbedGroups(
+            FieldSet('first', name='First'),
+            FieldSet('second', name='Second'),
+        ),
+    )
+
+    def _tabs(self, form):
+        return render_fieldset(form, self.fieldset)['rows'][0].items
+
+    def test_first_tab_active_without_initial_data(self):
+        """An unbound form activates the first tab."""
+        tabs = self._tabs(self.TestForm())
+        self.assertEqual([tab['active'] for tab in tabs], [True, False])
+
+    def test_initial_data_selects_its_tab(self):
+        """Initial data on a tab's leading field activates that tab."""
+        tabs = self._tabs(self.TestForm(initial={'second': 'x'}))
+        self.assertEqual([tab['active'] for tab in tabs], [False, True])
+
+    def test_errored_tab_is_activated(self):
+        """A field error overrides initial data and activates its own tab."""
+        form = self.TestForm({'first': 'x', 'second': ''}, initial={'first': 'x'})
+        self.assertFalse(form.is_valid())
+
+        tabs = self._tabs(form)
+
+        self.assertEqual([tab['active'] for tab in tabs], [False, True])
+
+    def test_first_errored_tab_wins(self):
+        """With both tabs errored, only the first errored tab is active."""
+        form = self.TestForm({'first': '', 'second': ''}, initial={'second': 'x'})
+        self.assertFalse(form.is_valid())
+
+        tabs = self._tabs(form)
+
+        self.assertEqual([tab['active'] for tab in tabs], [True, False])
+
+    def test_errored_pane_rendered_active(self):
+        """The errored tab's pane carries the active class."""
+        form = self.TestForm({'first': 'x', 'second': ''}, initial={'first': 'x'})
+        self.assertFalse(form.is_valid())
+
+        context = render_fieldset(form, self.fieldset)
+        html = render_to_string('form_helpers/render_fieldset.html', context)
+        pane_id = context['rows'][0].items[1]['id']
+
+        self.assertIn(f'class="tab-pane active" id="{pane_id}"', html)