verbb / formie

The most user-friendly forms plugin for Craft CMS.
Other
94 stars 72 forks source link

FileUpload fails required validation check #1130

Open roland-d opened 1 year ago

roland-d commented 1 year ago

Describe the bug

We have a Formie form that has 2 FileUpload fields that are required. When we POST this form via GraphQL to Craft the submission is received and Craft starts the process to validate the data. During this validation when it gets to the FileUpload field, the data is removed from the posted submission and replaced with an empty AssetQuery. After that the field is checked if it is empty or not, this confirms it is empty because the AssetQuery is empty.

In the file /vendor/craftcms/cms/src/fields/Assets.php line 397 in class normalizeValue() the uploaded data is put in $this->_uploadedDataFiles and then returns an empty craft\element\db\AssetQuery. After that the validation starts in the file vendor/yiisoft/yii2/validators/RequiredValidator.php on line 73 and it fails the check on line 79 because it finds an empty AssetQuery and this fails the required check for the FileUpload field, so the error message is added.

The actual error is {"Resume":["Resume cannot be blank."],"CoverLetter":["Cover Letter cannot be blank."]}

I am not sure what needs to be done but I would think that the AssetQuery should be populated with at least the fileData.

Steps to reproduce

  1. Create a form in Formie
  2. Add a FileUpload field and set it to required
  3. Send a graphql mutation query, for example http://site.com/graphql?query=mutation+xxx+{save_Apply_form_Submission(CoverLetter:[{"fileData": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAQUA...","filename": "testing.png"}])}
  4. Check the response saying the field cannot be empty

Form settings

Craft CMS version

Craft Pro 4.2.5.2

Plugin version

2.0.12

Multi-site?

No

Additional context

I reported this issue first with Craft because I think it has to do with Craft core but they asked to post it here as well. You can find the bug report here: https://github.com/craftcms/cms/issues/12111

Perhaps you can shed some light on this.

Thank you.

engram-design commented 1 year ago

So for GQL, this normalizeValue() function should do the pre-handling of the asset from the GQL mutation.

But - this only seems to be an issue when the file upload field is marked as required. If un-required, things are uploaded correctly, and of course no validation errors.

You're totally correct, that the native Craft Asset field handling grabs the values we're sending it (in normalizeValue) and stores them in the _uploadedDataFiles private variable, but it doesn't add anything to the AssetQuery (unless you're passing in an array of IDs, which we aren't here - we're passing in raw data).

As such, yes - when you hit validation and runs isValueEmpty(), $this->_getUploadedFiles($element) returns an empty array, because $this->_uploadedDataFiles in null. This is despite in the normalizeValue() function, $this->_uploadedDataFiles is populated correctly.

I'll do some more digging. For now, the short-term resolution would be to mark the File Upload field as un-required.

roland-d commented 1 year ago

As you suggested, we have made the File Upload now un-required and do the check in Next.js.