Opened 5 years ago

Last modified 5 years ago

#12780 new New feature

Provide a hook for compound form/formset validation in ModelAdmin

Reported by: mrts Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: shaun@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Sometimes it is necessary to validate inline formset data based on the main form data and vice-versa.

See e.g. http://groups.google.com/group/django-users/browse_thread/thread/259c1679ddaaceb8/6ad3698e5451d3fe for a use case.

My use case is somewhat similar: an object can be published only if its associated items meet certain criteria. Thus, to validate the "is published" flag in the form, access to formsets is required.

For advanced users a simple change is sufficient: substitute if all_valid(formsets) and form_validated: with if self.formsets_are_valid(formsets, form, form_validated) and form_validated: in admin/options.py, where def formset_are_valid(self, formsets, ...): return all_valid(formsets). The latter can then be overriden for arbitrary further manipulations. Will upload a patch eventually.

However, this means that people have to mess with either Form or FormSet internals directly (_non_form_errors etc) to expose the errors.

This is yet another proof of a conceptual problem, quoting myself from http://groups.google.com/group/django-developers/browse_thread/thread/f9aae709a7fda689/fb16a17ab6a31931 :

"Inline formsets usually represent
composition, i.e. the inline objects are inseparable from the
main entity. The relation is stronger than a ForeignKey (from the
main entity to another entity), yet the latter is displayed as a
field of the form and can be easily manipulated -- but not the inlines.
"

So, this is yet another reason for introducing FormsetFields in the long run -- Form.clean() would have access to formsets both in the admin and ordinary forms.

Attachments (1)

ticket12780.patch (1.7 KB) - added by mrts 5 years ago.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by mrts

comment:1 Changed 5 years ago by mrts

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

See http://gist.github.com/296411 for example usage.

comment:2 Changed 5 years ago by mrts

  • Has patch set

comment:3 follow-up: Changed 5 years ago by russellm

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

Patch needs tests.

Also - regarding the prototype for formsets_are_valid():

  • For consistency with other parts of Django (e.g., inlineformset_factory), I'd say the argument order should be request, new_object, form, formset
  • Is there a reason that form_validated is required as an argument? I can understand the "provide everything just in case" approach, but given that if form_validated=False, it doesn't matter what you return from formsets_are_valid, it doesn't make much sense to me to pass it in as an argument. Am I missing something?

comment:4 in reply to: ↑ 3 Changed 5 years ago by mrts

Replying to russellm:

Patch needs tests.

Will be done.

  • For consistency with other parts of Django (e.g., inlineformset_factory), I'd say the argument order should be request, new_object, form, formset

Agreed, will be done.

  • Is there a reason that form_validated is required as an argument? I can understand the "provide everything just in case" approach, but given that if form_validated=False, it doesn't matter what you return from formsets_are_valid, it doesn't make much sense to me to pass it in as an argument. Am I missing something?

One may decide to avoid any formset processing if the form is not valid.

The example at http://gist.github.com/296411 is not my use case so I can't comment on the actual requirements. but e.g. it may be useful to check if form_is_valid before validating base language/is approved. Feel free to disagree though.

comment:5 Changed 5 years ago by mrts

I'll have to abandon driving this further, sorry.

comment:6 Changed 5 years ago by shauncutts

  • Cc shaun@… added

I think there is another use case that can be addressed in this: I wanted to show an additional formset based on objects that are not directly related to a given object (they are "grandchildren"). The inline formset interface wasn't flexible enough for this, so I am displaying them in the {% after_related_objects %} block in the template, adding the additional formset to the extra_context by intercepting change_view. This works fine in this case. However, there is nowhere to trigger validation for the additional formset I have added. An overall ModelAdmin hook that allows extra validation would be great. (To address my use case, I wouldn't have called it "formsets_are_valid", but that's ok... :))

Note: a workaround that I have been able to use is to override ModelAdmin.get_form() and tack the additional validation onto the validation for the base form. However, this is obviously a hack.

Thanks!

comment:7 Changed 4 years ago by lukeplant

  • Type set to New feature

comment:8 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:9 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:10 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

Note: See TracTickets for help on using tickets.
Back to Top