Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation on models with deferred related model does no fully work #114

Open
unexceptable opened this issue May 14, 2019 · 3 comments
Open

Comments

@unexceptable
Copy link

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,)
@robbert-vdh
Copy link

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

@dkirkham
Copy link

dkirkham commented May 9, 2021

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.

@tbrlpld
Copy link

tbrlpld commented May 1, 2023

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 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants