#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 django-updates+unsubscr...@googlegroups.com. To post to this group, send email to django-updates@googlegroups.com. 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.