Fixed #12780 -- Added compound form/formset validation in ModelAdmin

[RamezIssac]

Adds a hook called is_change_form_valid to ModelAdmin for compound form/formset validation. It has access to request, form, formsets and add parameters. Return a boolean

[shangxiao]

I've only briefed myself with the ticket & changes but just wanted to ask is a solution within forms not doable? 🤔

[RamezIssac]

I've only briefed myself with the ticket & changes but just wanted to ask is a solution within forms not doable? 🤔

Not really, this is to validate the whole change form ,
Forms and formset(s) in conjunction.
What we currently have is form alone, and formsets alone .

Should the hook by named 'is_whole_changeform_valid()' better ?

[nessita]

Hello @RamezIssac, could you rebase onto main and resolve conflicts? Thank you!

[nessita]

I've only briefed myself with the ticket & changes but just wanted to ask is a solution within forms not doable? thinking

Not really, this is to validate the whole change form , Forms and formset(s) in conjunction. What we currently have is form alone, and formsets alone .

Should the hook by named 'is_whole_changeform_valid()' better ?

I agree with @shangxiao, from the ticket description and the presented use cases, it seems that a solution available to all forms, outside the scope of the admin itself is quite useful.

To me, as a Djando user, I would expect the solution to be a method of a Form instance where I could optionally pass a list of extra parameters including the formsets that should be validated in conjunction. Have you, by any chance, consider this path in your PR journey? Was there a blocker that we could discuss?

Thank you!

[RamezIssac]

Hello , Branch rebased to solve conflicts

I've only briefed myself with the ticket & changes but just wanted to ask is a solution within forms not doable? > To me, as a Djando user, I would expect the solution to be a method of a Form instance where I could optionally pass a list of extra parameters including the formsets that should be validated in conjunction. Have you, by any chance, consider this path in your PR journey? Was there a blocker that we could discuss?

For the hook, it was always this hook this way. i mean, i needed it and added it in my projects under the name is_whole_change_form_valid

I don't see a Form solution would do it, or somewhere outside the admin would be a more appropriate location.. Can yo provide an example of where it should go ?

This hook is similar to save_related(request, form, formsets, change) but, it's for validation .

• a custom Admin form wont do it as it have no access to formsets
• a custom validation on formsets wont do it , dont have access to the form and other related formsets on the change form page.

Thank you !

[nessita]

Hello again @RamezIssac! I have been thinking about your latest response. I built a test project just like the one described in the original django-users post, and after some investigation, I found this stackoverflow post which made me realize that there is a way to validate both the form and the formsets by customizing the formset's clean method.

This is the example that I used:

# 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)

Given the above, I'm adding a comment to the ticket to get more feedback, but my initial proposal would be to close the ticket as invalid. What do you think?

[nessita]

Closing the PR as the original reporter agreed in the ticket. Thanks everyone!