Need to wire up submission validations

[jacobsfletch]

Currently you can create form submissions with arbitrary data. This means that you're front-end is relied upon entirely to validate your data. We need to set up the form-submissions collection to do this natively. See the TODO noted in code here: https://github.com/payloadcms/plugin-form-builder/blob/main/src/collections/FormSubmissions/index.ts#L58

Discord: https://discord.com/channels/967097582721572934/1136001656253861888

[jacobsfletch]

The implementation here should be done through normal field-level validations (if possible) and not through a custom beforeValidate hook like one might expect. We should investigate if we can copy over the same validations that normally take place on the fields of the form collection itself (the ones that are cloned to form-submissions).

[IAmNatch]

The field level validation does appear possible, however it definitely seems less efficient.

The field level validation function provides the following data:

value: <whatever value this field supports>
options: {
   ...fieldConfig,
   jsonError, 
   data, 
   siblingData,
   id,
   operation,
   user,   
   payload
}

Since the formSubmission collection is totally separate from the form collection, none of the relevant data will be provided by the existing params. The same is true for the beforeValidate hook.

As well, we could not simply reference the other models collection config, as the fields are dynamic blocks.

With this in mind, I believe either approach (field level or collection level) would need to utilize the provided payload instance to fetch the related form model. This approach should be pretty straightforward for me to implement, my main concern is that having each field refetching the related form would be less performant.

I know payload does have some level of caching- do you think this would mitigate the concern, or it would be best to do a single query on the collection level, and then validate each of its fields as necessary?

[IAmNatch]

I had some time today so I did some experimenting and put up a WIP PR #61, implementing the field level validation, as you suggested.

The good:

• From my preliminary testing, the extra queries did not have any significant impact on response time. I'm guessing this is due to payloads caching mechanism, but I'd still like to test a bit more on a form with at least 100 inputs to be sure.
• Everything worked as expected, and I was also able to add in validation for formData fields that do not exist.

Less good:

• Unfortunately, as mentioned in the PR, this approach has one major limitation, which is that although we can validate that required fields are not null or empty, we cannot validate that they are present. If we don't send a certain field, there is no field for us to validate against, and therefore we can't send a validation error for this.

I think this points toward needing a collection level validation instead of field level for this one, but I'll wait to hear your feedback before proceeding in that direction. If we go that way, I can also resolve payloadcms/payload#4471 in the same PR.

Thanks for your time!

[IAmNatch]

@jacobsfletch, I've updated the PR to address some of the cases mentioned above. Looking forward to hearing your thoughts!

[IAmNatch]

@jacobsfletch, now that 2.0 is out, any chance you’ll have some time to review this? I think it meets the outlined requirements pretty well!

[jacobsfletch]

Hey @IAmNatch thank you for your diligence on this. Right now we're actively working toward importing this repo into the Payload monorepo, and maintaining it there instead. This will provide us with a robust testing suite that we can use to significantly improve stability and also improve the release frequency of this plugin. At some point we'll need to re-open this PR against the new home for it, but I will follow up when that happens over the next couple of weeks. Then we can invest much more heavily in this feature. How does that sound?

[IAmNatch]

Hey @jacobsfletch, thanks for the update, this makes a ton of sense. I'm glad my extra message wasn't too much of a pain! Re: the way, I'm installing off my fork, so it's no problem at all.

In the interim, if there's any opportunity for community contributors to help out with that migration (or anything else), I'd be happy to lend some time. This was my first foray into contributing to payload, and as someone who's been dreaming of something like payload for a long while now, I'm looking forward to the next opportunity.

[jacobsfletch]

This plugin is now being maintained in the Packages Directory of the Payload Monorepo. This repo will soon be archived and all open issues including this one will be closed. This issue has already been added to this open discussion, though, so that it will not get lost. Please refer to that discussion for more details and to continue the conversation.