Fixed #14688 -- Made BaseInlineFormSet support explicit "auto_id" argument. #19417
Conversation
There was a problem hiding this comment.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
…nsistency (with "prefix" argument and ancestors).
There was a problem hiding this comment.
@zeth Looks good to me.
Unfortunately, BaseModelFormSet has a positional argument auto_id at position 3. Hence, I would like to suggest to not allow auto_id as a positional argument at a different position in BaseInlineFormSet. Instead let it go into the named arguments.
| save_as_new=False, | ||
| prefix=None, | ||
| queryset=None, | ||
| auto_id="id_%s", |
There was a problem hiding this comment.
| auto_id="id_%s", |
I suggest to let this go into kwargs.
| self.unique_fields = {self.fk.name} | ||
| super().__init__(data, files, prefix=prefix, queryset=qs, **kwargs) | ||
| super().__init__( | ||
| data, files, prefix=prefix, queryset=qs, auto_id=auto_id, **kwargs |
There was a problem hiding this comment.
| data, files, prefix=prefix, queryset=qs, auto_id=auto_id, **kwargs | |
| data, files, prefix=prefix, queryset=qs, **kwargs |
... then the auto_id will be passed on within the kwargs.
|
|
||
| # Using positional argument | ||
| inlineformset = PoemInlineFormSet( | ||
| None, None, poet, False, None, None, "id_custom_%s" |
There was a problem hiding this comment.
| None, None, poet, False, None, None, "id_custom_%s" | |
| None, None, poet, False, None, None, auto_id="id_custom_%s" |
In the test, we would need to pass the auto_id not as a named argument.
Made BaseInlineFormSet explicitly support
auto_id, for consistency with theprefixargument and superclasses.Trac ticket number
ticket-14688
Branch description
BaseInlineFormSet.init does not explicitly accept the
auto_idargument, unlike its ancestor classes, and thus inconsistent with its sibling argumentprefix.This is picking up word already done by Victor Andrée and Tobias Kunze, with a longer and more specific test. The ticket was closed by mistake and then the ticket was reopened.
This is one of two approaches, the other would be to make
prefixnot an explicit argument to matchauto_id, but that would be a breaking change.No idea if anyone still wants this or cares 14 years after first opening, but this should hopefully push the ticket forward at least.
Checklist
mainbranch.