Address PR review feedback
- Trim _get_rename_fields docstring and required=False comment to single lines - Pass field_name as a parameter to _rename_objects instead of extracting it from form.cleaned_data inside the method (single source of truth) - Fix test skip condition to use _meta.fields instead of _meta.get_fields() to match the implementation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
f986903f6e
commit
054e36ae1b
|
|
@ -884,14 +884,7 @@ class BulkRenameView(GetReturnURLMixin, BaseMultiObjectView):
|
|||
filterset = None
|
||||
|
||||
def _get_rename_fields(self):
|
||||
"""
|
||||
Return a list of (value, label) choices for the rename field selector.
|
||||
|
||||
The selector is shown only when the view targets the default 'name' field and
|
||||
the model also has a 'label' field (device/module components and their templates).
|
||||
Views that hardcode a non-default field_name (e.g. Cable → 'label') return an
|
||||
empty list so no selector is rendered.
|
||||
"""
|
||||
"""Return (value, label) choices for the field selector, or [] if only one field applies."""
|
||||
if self.field_name != 'name':
|
||||
return []
|
||||
model_field_names = {f.name for f in self.queryset.model._meta.fields}
|
||||
|
|
@ -922,10 +915,7 @@ class BulkRenameView(GetReturnURLMixin, BaseMultiObjectView):
|
|||
choices=rename_fields,
|
||||
label=_('Field'),
|
||||
initial='name',
|
||||
# Django's ChoiceField.validate() skips the valid_value() check when the
|
||||
# value is empty, so required=False lets existing callers (tests, scripts)
|
||||
# omit the field entirely; the view falls back to self.field_name.
|
||||
required=False,
|
||||
required=False, # empty value falls back to self.field_name in post()
|
||||
)
|
||||
|
||||
self.form = type('_Form', (base_form,), form_attrs)
|
||||
|
|
@ -933,9 +923,10 @@ class BulkRenameView(GetReturnURLMixin, BaseMultiObjectView):
|
|||
def get_required_permission(self):
|
||||
return get_permission_for_model(self.queryset.model, 'change')
|
||||
|
||||
def _rename_objects(self, form, selected_objects):
|
||||
def _rename_objects(self, form, selected_objects, field_name=None):
|
||||
renamed_pks = []
|
||||
field_name = form.cleaned_data.get('field_name') or self.field_name
|
||||
if field_name is None:
|
||||
field_name = self.field_name
|
||||
|
||||
for obj in selected_objects:
|
||||
# Take a snapshot of change-logged models
|
||||
|
|
@ -975,7 +966,7 @@ class BulkRenameView(GetReturnURLMixin, BaseMultiObjectView):
|
|||
field_name = form.cleaned_data.get('field_name') or self.field_name
|
||||
try:
|
||||
with transaction.atomic(using=router.db_for_write(self.queryset.model)):
|
||||
renamed_pks = self._rename_objects(form, selected_objects)
|
||||
renamed_pks = self._rename_objects(form, selected_objects, field_name)
|
||||
|
||||
if '_apply' in request.POST:
|
||||
# For MPTT models, delay tree updates until all saves are complete
|
||||
|
|
|
|||
|
|
@ -1120,7 +1120,7 @@ class ViewTestCases:
|
|||
@override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
|
||||
def test_bulk_rename_label_field(self):
|
||||
"""When field_name='label' is submitted, labels (not names) are updated."""
|
||||
if 'label' not in {f.name for f in self.model._meta.get_fields()}:
|
||||
if 'label' not in {f.name for f in self.model._meta.fields}:
|
||||
self.skipTest("Model does not have a label field")
|
||||
|
||||
objects = self._get_queryset().all()[:3]
|
||||
|
|
|
|||
Loading…
Reference in New Issue