django-modelcluster icon indicating copy to clipboard operation
django-modelcluster copied to clipboard

Validation on models with deferred related model does no fully work

Open unexceptable opened this issue 5 years ago • 3 comments

Attempting to validate a Form Page in Wagtail that has FormFields associated with it via a ParentalKey doesn't allow us to validate that certain formfields make sense. In the clean function of the Page, any call to my related object queryset doesn't return any of the about to be created form fields, only ones that already exist.

The requirement: We have some email related fields that rely on certain form fields potentially existing, so if one of those fields is set, we validate that the formfield exists.

The issue: Until the page is actually saved, those form fields don't exist, so validating against form fields that are about to be saved in page.clean doesn't work. We first have to create the form fields, then add to that field, then save again.

Steps to reproduce:

Using a form page based on the below code, go to create a new page.

On that new page set a form field with the name email and populate reply_to_from_field with the same. Save draft. Despite the form field existing reply_to_from_field will raise a validation error.

If you unset reply_to_from_field, then save draft, and then fill reply_to_from_field, and save again, it will now work.

Expected result

Some way to get at the deferred objects when validating the Page model, so we can check if the models about to be created will actually fulfil the requirements of the Page.

Our code

class SafeCaptchaFormBuilder(WagtailCaptchaFormBuilder):

    @property
    def formfields(self):
        fields = super(SafeCaptchaFormBuilder, self).formfields
        if not settings.RECAPTCHA_PUBLIC_KEY:
            fields.pop(self.CAPTCHA_FIELD_NAME)
        return fields


class FormField(AbstractFormField):
    page = ParentalKey('FormPage', related_name='form_fields')


class FormPage(BasePage, WagtailCaptchaEmailForm):
    intro = RichTextField()
    thank_you_text = RichTextField()
    reply_to_from_field = models.CharField(
        max_length=255, blank=True,
        help_text=(
            "Label of the form field to get reply-to from. "
            "Supercedes From Address.")
    )

    content_panels = [
        FormSubmissionsPanel(),
        FieldPanel('title', classname="full title"),
        FieldPanel('intro', classname="full"),
        InlinePanel('form_fields', label="Form fields"),
        FieldPanel('thank_you_text', classname="full"),
        MultiFieldPanel([
            FieldPanel('to_address'),
            FieldRowPanel([
                FieldPanel('reply_to_from_field', classname="col6"),
                FieldPanel('from_address', classname="col6"),
            ]),
            FieldPanel('subject'),
        ], "Email"),
    ]

    def __init__(self, *args, **kwargs):
        super(FormPage, self).__init__(*args, **kwargs)
        self.form_builder = SafeCaptchaFormBuilder

    def clean(self):
        super(FormPage, self).clean()

        if self.reply_to_from_field:
            reply_field = str(slugify(self.reply_to_from_field))
            found = False
            is_email = False
            for field in self.form_fields.all():
                if field.clean_name == reply_field:
                    found = True
                    if field.field_type == "email":
                        is_email = True

            if not found:
                raise ValidationError(
                    {'reply_to_from_field': (
                        'Form field with this label must exist.')}
                )
            if not is_email:
                raise ValidationError(
                    {'reply_to_from_field': (
                        'Form field with this label must be an email field.')}
                )

    def send_mail(self, form):
        addresses = [x.strip() for x in self.to_address.split(',')]
        content = []
        reply_field = str(slugify(self.reply_to_from_field))
        from_address = None
        for field in form:
            value = field.value()
            if isinstance(value, list):
                value = ', '.join(value)
            content.append('{}: {}'.format(field.label, value))
            if str(slugify(field.label)) == reply_field:
                from_address = value
        content = '\n'.join(content)
        if from_address:
            send_mail(self.subject, content, addresses, from_address,)
        else:
            send_mail(self.subject, content, addresses, self.from_address,)

unexceptable avatar May 14 '19 02:05 unexceptable

Any updates on this? I just ran into the same issue when I wanted to ensure that a Wagtail form builder form includes at least an email field and a first name field (for sending confirmation mails).

robbert-vdh avatar Sep 21 '20 12:09 robbert-vdh

I've also identified this same issue, with a Wagtail application which has a Page that includes some Orderables which have a ParentalKey relation back to the Page model. The Page model's clean() method is called multiple times and sometimes the Orderables are there and sometimes they aren't, depending on the exact manner in which the page is being saved, published or previewed. That makes validation difficult. As I see it, we either need a very specific clean-like method on the parent model once the relations between the models have been set up from the incoming form - or ensure those relations are set up before the form calls any downstream is_valid() methods.

My use case is not actually validation, but calculating some hidden fields in both the Page and the Orderable based on data in the other (ultimately to make the database queries much simpler). In my case, I can make those field calculations in the model's save() method, though that method isn't called during a page preview, which is my biggest problem. I'd prefer if there were an opportunity to do this much earlier in the page processing.

dkirkham avatar May 09 '21 11:05 dkirkham

I have this issue too. I was tryin to check the number of related pages (Orderables with a ParentalKey to the page) in a pages clean() method. It seems to be returning the previously saved value, not the submitted one. This may be a Wagtail issue 🤔

tbrlpld avatar May 01 '23 21:05 tbrlpld