Opened 2 years ago

Closed 2 years ago

#21066 closed Cleanup/optimization (wontfix)

ModelAdmin._create_formsets can generate formsets with a 'None-1' prefix, which is ugly

Reported by: kamni Owned by:
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In cases where a programmer overrides BaseFormSet.get_default_prefix, it is possible to return a 'None' value due to programmer oversight. The solution currently in place is to add 'or not prefix' to prevent errors (https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1565-L1565). However this results in formsets with a prefix like 'None-1', which looks unfinished and is uninformative when looking at the HTML.

A better choice would be to modify line 1563 to read 'prefix = FormSet.get_default_prefix() or "form"', or another default that might indicate to the developer that their code was failing, without generating ugly html.

Change History (5)

comment:1 Changed 2 years ago by kamni

  • Component changed from Forms to contrib.admin
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by polmuz

  • Owner changed from nobody to polmuz
  • Status changed from new to assigned

comment:3 Changed 2 years ago by timo

You don't think ugly HTML indicates that something is amiss? Adding the fallback seems like it would make the problem more transparent. If this is an issue, I think we should fail loudly if get_default_prefix() doesn't return a value.

comment:4 Changed 2 years ago by polmuz

  • Owner polmuz deleted
  • Status changed from assigned to new

comment:5 Changed 2 years ago by EvilDMP

  • Resolution set to wontfix
  • Status changed from new to closed

Following discussion with Amymeric Augustin and Tim Graham, there are probably more serious issues with admin inline formsets, and the risks/benefits of interfering with an already delicate part of the system suggest it's better left alone for now.

Note: See TracTickets for help on using tickets.
Back to Top