#19255: BaseGenericInlineFormSet runs validation methods before linking form
instances to their related object
-------------------------------------+-------------------------------------
Reporter: bouke | Owner:
| arielpontes
Type: Bug | Status: assigned
Component: | Version: 1.4
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by arielpontes):
= Update =
Consider these models:
{{{
class Child(models.Model):
content_type = models.ForeignKey(ContentType)
object_id = models.PositiveIntegerField()
object = GenericForeignKey()
def clean(self):
assert self.object_id
class Parent(models.Model):
parent_number = models.IntegerField()
}}}
And these admins:
{{{
class ChildInline(GenericStackedInline):
model = Child
class ParentAdmin(admin.ModelAdmin):
inlines = (ChildInline,)
admin.site.register(Parent, ParentAdmin)
}}}
== The easier part of the problem (#25488)==
I started investigating this problem as described in #25488 and I came up
with the following solution:
1. Moving the linking from `save_new` to `_construct_form`. It solves the
problem described in that ticket, which reported a case in which there was
a FormSet which was always initialized with a ParentModel instance.
This solution, however, breaks ANY attempt of saving a child inline in a
"create parent" admin form. This happens because the FormSet is not
initialized with a Parent instance in the admin, after all the parent
doesn't exist yet (it's being created in the same form submission), which
brings me to the revised solution:
2. Doing the linking both in `save_new` AND `_construct_form`. This way,
if a form is instantiated with an `instance` parameter, the `clean` will
be able to access its instance's related object. It will only fail in the
very particular case described above. It duplicates the linking but is
still an improvement over the current behavior. It fixes the problem in
#25488 without breaking anything (I ran the tests and they passed).
== The harder part of the problem ==
Consider the following case:
* you have a Child model with a GenericRelation,
* validation in the Child's `clean` method that relies on the existence of
the related object,
* a Parent model,
* a ChildInline,
* a ParentAdmin with a ChildInline,
* and you want to create a new Parent record while at the same time saving
a new Child record (in the same form submission)
Currently, this is pretty much impossible. Any solution I can think of
entails some of the following serious compromises:
3. We give up on the atomicity of the admin actions and save the parent
record before validating the inlines. If there are errors, we show some
(warning?) message like:
{{{
Your parent was created successfully but saving some of the children
failed. Please correct the mistakes and re-submit the form to add them.
}}}
4. We enforce the transactionality of this operation in the application
layer, creating a parent record and deleting it if validation of the
inlines fail. I have no idea how acceptable this is. Intuitively I'm not a
big fan.
5. We by default don't show inlines (inheriting from BaseInlineFormSet) in
admin "new record" forms at all. It only appears once the parent is saved,
in "edit record" forms.
== Conclusion ==
Since I can't think of alternatives, in principle I'm going for the second
solution. I'm thinking perhaps an improvement would be catching the
`django.db.models.fields.related.RelatedObjectDoesNotExist` Exception in
`BaseInlineFormSet.full_clean` and adding a more user-friendly message to
the formset errors. Something along the lines of:
{{{
>>> formset.non_form_errors()
['Please make sure a parent record exists before trying to save a child.']
}}}
It might give the impression that this is never possible though, while in
fact it's only not possible when there's a custom clean method trying to
access the related object (the error would only be displayed in this
situation).
Another idea would be throwing a more explanatory Django error, perhaps
something like:
{{{
django.db.models.fields.related.RelatedObjectDoesNotExist: Cannot validate
child's related object on parent creation
}}}
In any case, this solution wouldn't break any current code. The error
would appear if there's a custom clean, otherwise the parent will be
linked in `save_new` as usual.
--
Ticket URL: <https://code.djangoproject.com/ticket/19255#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/063.907c7c8197d884f099b538899250768b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
