Fixes #20776: Add changelog message to bulk rename process

[jnovinger]

Fixes: #20776

Adds changelog message support to the bulk rename view, bringing it in line with bulk edit, bulk delete, and bulk import.

Changes

• BulkRenameForm now inherits from ChangelogMessageMixin, adding an optional changelog message field
• BulkRenameView sets obj._changelog_message before save (both MPTT and non-MPTT paths)
• Conditional field removal in BulkRenameView.__init__ for models that don't support change logging
• New test test_bulk_rename_objects_with_changelog_message in the standard view test mixin

No template changes were needed: render_form.html already handles meta_fields rendering.

Form relocation

BulkRenameForm was moved from utilities/forms/forms.py to netbox/forms/bulk_rename.py. This was necessary because utilities/forms/forms.py loads early in Django's boot chain (via extras/models → utilities/fields → utilities/forms). Adding ChangelogMessageMixin as a parent there creates a circular import since the mixin's module imports extras.models, which hasn't finished loading at that point. The new location in netbox/forms/ loads after the app registry is populated, so the import works cleanly.

[jnovinger]

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

[github-actions]

Claude finished @jnovinger's task in 4m 19s —— View job

---

Code Review

• Read changed files
• Analyze for potential issues
• Post review findings

---

Issues of Concern

1. BulkRenameView ignores the form class attribute — VMInterfaceBulkRenameForm / VirtualDiskBulkRenameForm are dead code

BulkRenameView.__init__ unconditionally creates a fresh _Form inheriting from BulkRenameForm and assigns it to self.form, overriding any class-level form attribute set by subclasses:

# bulk_views.py
class _Form(BulkRenameForm):          # always BulkRenameForm, never self.form
    pk = ModelMultipleChoiceField(
        queryset=self.queryset,
        widget=MultipleHiddenInput()
    )
self.form = _Form                     # overwrites VMInterfaceBulkRenameView.form

This means:

• VMInterfaceBulkRenameView.form = forms.VMInterfaceBulkRenameForm (views.py:669) is never used.
• VirtualDiskBulkRenameView.form = forms.VirtualDiskBulkRenameForm (views.py:742) is never used.
• VMInterfaceBulkRenameForm and VirtualDiskBulkRenameForm in bulk_edit.py:300, 338 exist only as dead code — including their pk queryset declarations.

This is a pre-existing issue, but the PR's form relocation makes it more visible and adds a new import in virtualization/forms/bulk_edit.py that exists solely to support these unused classes. Either the dead classes should be removed, or BulkRenameView.__init__ should be updated to use type(self.form.__name__, (self.form,), {'pk': ...}) as the base so subclasses can actually extend the form. Fix this →

---

2. base_fields.pop() mutates the class-level dict

# bulk_views.py:872
if not issubclass(self.queryset.model, ChangeLoggingMixin):
    self.form.base_fields.pop('changelog_message', None)

base_fields is a class-level dict. Even though _Form is created fresh per request (making this safe in practice), directly mutating it is fragile and departs from the pattern used elsewhere. The idiomatic Django approach is to conditionally define or override fields at class creation time rather than removing them afterwards — e.g., constructing _Form differently based on whether the model supports change logging. Fix this →

---

3. New test does not skip for models without change logging support

test_bulk_rename_objects_with_changelog_message (testing/views.py:1053) unconditionally queries ObjectChange and asserts records exist. For any model mixed into BulkRenameObjectsViewTestCase that doesn't support ChangeLoggingMixin, the changelog_message field would be removed from the form (no ObjectChange created), and the assertIsNotNone(oc) assertion at line 1082 would fail. Consider adding a guard:

if not issubclass(self.model, ChangeLoggingMixin):
    self.skipTest("Model does not support change logging")

All current uses appear to be ChangeLoggedModel subclasses, but this would protect against future regressions. Fix this →

[jnovinger]

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

1. BulkRenameView ignores the form class attribute — VMInterfaceBulkRenameForm / VirtualDiskBulkRenameForm are dead code

Out of scope, will file a follow up housekeeping issue for this.

2. base_fields.pop() mutates the class-level dict

Added a comment indicating why this is safe.

3. New test does not skip for models without change logging support

Added.

[github-actions]

Claude finished @jnovinger's task in 3m 24s —— View job

---

Code Review

• Read changed files
• Verify fixes for previously raised issues
• Post review findings

---

Both addressed issues check out. One minor residual concern below.

ChangelogMessageMixin.__init__ still populates meta_fields after base_fields.pop()

When changelog_message is removed from base_fields for a non-ChangeLoggingMixin model, ChangelogMessageMixin.__init__ still runs at form instantiation time and appends 'changelog_message' to the instance's meta_fields list. The template then evaluates:

{# render_form.html #}
{% if form.meta_fields %}        {# True — list is ['changelog_message'] #}
  <div class="bg-primary-subtle...">
    {% if form.changelog_message %}  {# Resolves to the class-level CharField, which is truthy #}
      {% render_field form.changelog_message %}  {# Passed a CharField, not a BoundField #}

Django's template engine falls back to attribute lookup when form["changelog_message"] raises KeyError, and _Form.changelog_message is still present as a class attribute (Django's DeclarativeFieldsMetaclass copies fields to base_fields but leaves the original class attribute in place). The CharField instance is truthy, so render_field would be invoked with it rather than a BoundField.

Practical impact: none — every NetBox model that registers a bulk-rename view inherits from ChangeLoggingMixin / NetBoxModel, so this branch is currently dead code. The concern is only relevant if a plugin author subclasses BulkRenameView for a model that deliberately opts out of change logging.

If you want to close off the theoretical case, the minimal fix would be to also remove 'changelog_message' from meta_fields after popping the field, or to conditionally construct _Form without inheriting from ChangelogMessageMixin at all for those models. But given the zero current impact, deferring to a follow-up is entirely reasonable.

---

No other issues found. The two previously raised items (explanatory comment on base_fields.pop(), skipTest guard) are implemented correctly.

[pheus]

Thanks for explaining the circular import! That helped a lot while reviewing.

That said, I don’t think we should move BulkRenameForm out of utilities.forms in a patch release. That changes an existing import path that plugins/custom code may already rely on, e.g. from utilities.forms import BulkRenameForm.

It also changes the current separation between generic utility forms in utilities and NetBox model-specific forms in netbox. I’m not opposed to moving or reorganizing these forms, but I think that should wait for a release where breaking import-path changes are allowed.

The cleanest patch-release-safe alternative I can think of is to keep utilities.forms.BulkRenameForm as the generic form and add a NetBox-specific wrapper, similar to the existing bulk edit pattern:

class NetBoxModelBulkRenameForm(ChangelogMessageMixin, BulkRenameForm):
    pass

I’d also suggest making BulkRenameView.form an explicit extension point, while still injecting the pk field dynamically from the view. The pk field depends on the scoped queryset, so I think it should remain view-owned rather than becoming part of the generic form.

For example:

class BulkRenameView(GetReturnURLMixin, BaseMultiObjectView):
    """
    An extendable view for renaming objects in bulk.

    Attributes:
        field_name: The name of the object attribute for which the value is being updated (defaults to "name")
        form: Optional base form class used for the rename operation
    """
    field_name = 'name'
    template_name = 'generic/bulk_rename.html'
    filterset = None
    form = None

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

        base_form = self.form
        if base_form is None:
            base_form = (
                NetBoxModelBulkRenameForm
                if issubclass(self.queryset.model, ChangeLoggingMixin)
                else BulkRenameForm
            )

        class _Form(base_form):
            pk = ModelMultipleChoiceField(
                queryset=self.queryset,
                widget=MultipleHiddenInput()
            )

        self.form = _Form

This keeps utilities.forms.BulkRenameForm generic and backwards-compatible, avoids importing ChangelogMessageMixin into utilities.forms, and removes the need to mutate base_fields to strip changelog_message for models that do not support change logging.

One thing to watch: VMInterfaceBulkRenameView and VirtualDiskBulkRenameView already set form, but BulkRenameView currently overwrites it, so those form classes have effectively been unused. If we make form supported now, those two should either be updated to inherit from the new NetBox-specific bulk rename form, or removed/cleaned up separately if they are not needed.