Re: [Django] #12780: Provide a hook for compound form/formset validation in ModelAdmin
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Changes (by Ramez Issac):
* owner: nobody => Ramez Issac
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Changes (by Ramez Issac):
* needs_tests: 1 => 0
Comment:
PR [https://github.com/django/django/pull/16777 ]
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:12>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
* cc: Carlton Gibson, Natalia Bidart, Mariusz Felisiak (added)
Comment:
In order to properly review the PR, I've been investigating solutions for
this ticket, and as I mentioned in
[[https://github.com/django/django/pull/16777|PR #16777]], I think there
is currently enough flexibility in overriding the inline's formset so that
any custom validation involving both the parent form/instance and the
formset themselves can be done there. Following the example referenced in
the ticket description use case, in order to block `Clip` instances to be
approved without a matching `ClipDescription` in the same language, the
customization would look similar to this:
{{{
# models.py
from django.db import models
LANGS = ['de', 'en', 'es', 'fr', 'it']
class Clip(models.Model):
is_approved = models.BooleanField(default=False)
language = models.CharField(choices=[(i, i) for i in LANGS],
max_length=2)
class ClipDescription(models.Model):
clip = models.ForeignKey(Clip, on_delete=models.CASCADE)
text = models.TextField()
language = models.CharField(choices=[(i, i) for i in LANGS],
max_length=2)
}}}
{{{
# admin.py
from django import forms
from django.contrib import admin
from django.core.exceptions import ValidationError
from .models import Clip, ClipDescription
class InlineFormset(forms.models.BaseInlineFormSet):
def clean(self):
languages = {
f.cleaned_data['language']
for f in self.forms
if 'language' in f.cleaned_data
# This next line filters out inline objects that did exist
# but will be deleted if we let this form validate --
# obviously we don't want to count those if our goal is to
# enforce a condition of related objects.
and not f.cleaned_data.get('DELETE', False)
}
if (
self.instance.is_approved
and self.instance.language not in languages
):
raise ValidationError(
'A Clip cannot be approved without a description in the
same language'
)
class ClipDescriptionInline(admin.TabularInline):
model = ClipDescription
formset = InlineFormset
class ClipAdmin(admin.ModelAdmin):
inlines = [
ClipDescriptionInline,
]
admin.site.register(Clip, ClipAdmin)
}}}
In summary, I'm proposing to close this issue as invalid/wontfix, but I'd
live to know what others think first.
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:13>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Comment (by Carlton Gibson):
Hi Natalia -- good example.
The inline has access to the parent object, so that direction at least is
covered.
For any remaining cases not covered, I'd be inclined to put them in ''"the
admin is not your app"'' land — just write a custom view: the admin
doesn't have to do everything, no matter how niche.
My suspicion is that a hook here would get very little use. (It's been
inactive for 11 years after all.) Nonetheless every reader of the admin
site docs would have one more method to read about and understand, before
coming to the conclusion it wasn't relevant to them. As such, I'd lean (as
often) towards thinking it probably wouldn't pay its way.
Thus I'd agree that wontfix is the right conclusion.
+1
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:14>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Comment (by Mariusz Felisiak):
"wontfix" from me.
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:15>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
* status: assigned => closed
* resolution: => wontfix
Comment:
Hello
Thank you Natalia for the example, now I can see the reference way via the
instance, didn't know about it .
Now I also agree with the won't fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:16>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: closed
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Comment (by ldeluigi):
Replying to [comment:13 Natalia Bidart]:
What if you need to cross-validate two or more different inline formsets
together?
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:17>
Django
-------------------------------+---------------------------------------
Reporter: mrts | Owner: Ramez Issac
Type: New feature | Status: closed
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+---------------------------------------
Comment (by Natalia Bidart):
Replying to [comment:17 ldeluigi]:
>
> What if you need to cross-validate two or more different inline formsets
together?
Could you please provide a django test project with an example of your
scenario?
--
Ticket URL: <https://code.djangoproject.com/ticket/12780#comment:18>