Opened 4 years ago
Closed 4 years ago
#33447 closed New feature (wontfix)
min_num on formsets should not just be added to extra, again
| Reported by: | typonaut | Owned by: | nobody |
|---|---|---|---|
| Component: | Forms | Version: | 3.2 |
| Severity: | Normal | Keywords: | inlineformset_factory min_num extra |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | yes |
Description (last modified by )
I have read #22628 and #27679.
I can see that the documentation says that if you have min_num = 1 and extra = 1 then you will get two blank inline forms in a new form.
I am flagging this as a new feature, although I think it is really a bug.
When using forms.models.inlineformset_factory min_num and extra input parameters are available. Logically min_num is really a validation parameter where extra is a utility/UI parameter. I believe that given this logic, with a new form and min_num = 1 and extra = 1 then there should only be one inline form displayed, rather than two as currently happens.
I would propose to add an additional input parameter to control this bahaviour and account for any backward compatibility issues: min_num_minimise as a boolean.
forms.models.inlineformset_factory(…
extra=1,
min_num=1,
min_num_minimise=True
)
This would render one inline form on a new form, where:
forms.models.inlineformset_factory(…
extra=1,
min_num=1,
min_num_minimise=False,
)
Would maintain the current behaviour – with the default being min_num_minimise=False.
I believe that the current implementation is more than a little confusing from a UI perspective. While it is possible to code around this implementation and test for at least min_num forms, that is quite a lot of work and more prone to break with updates.
In addition, there is a bug in behaviour: if min_num is greater than zero, a form action that deletes all existing inline forms is valid, which should not be the case.
Change History (2)
comment:1 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 4 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
Hi.
I'm going to have to say wontfix here: Given the discussion on #27679 (and the linked PR) it needs a consensus for change.
No arguments for the breaking were put forward in the four or so years #27679 was left open, and I can't see there being an appetite for introducing the single use kwarg here.
Nonetheless, if you want to write up the proposal (with links to the relevant history) for the DevelopersMailingList, you can do that to see.
Thanks.