Some suggestions for the Validation and Blueprint classes

[bovisp]

As it stands now, when one is using the form plugin, and novalidate and formnovalidate have both been turned off to disable native browser HTML5 form validation, when a user submits a form, validation is done on the server using the Grav Validation and Blueprint classes.

As it stands now, the first error in the form validation will stop the validation process and the an error message is returned. What I propose is a process whereby the whole form is validated in Grav, any errors are dumped into an array where the value in each key would contain the field attributes as well as the error message.

In some instances, there may be multiple errors for the same field. An example would be required email inputs, which may potentially require two error messages:

• one for incorrect formatting
• one for not filling out that input

Because of this, the user should have the option of creating multiple error messages in their form frontmatter such as message.rule and message.required and so on. There should be generic fallback translations strings for missing a required field (eg FORM.REQUIRED_FAIL and, currently "Missing reqired field: " followed by field.name) and invalid input (eg FORM.INVALID_FAIL and, currently "Invalid input in: " followed by field.name), which the user could override within their /user/languages/xx.yaml files. Within the array of error messages returned from the server (let's call this form.messages), each error message type should be identified so that correct message output will be displayed: invalid or missing input.

Additionally, I would suggest that the generic error message, currently "Validation Failed: " be moved to the frontmatter so that it can be customized for every form and make translation easier. This could be assigned to form.errormsg, for example. The fallback would be stored in /system/languages/xx.yaml asFORM.VALIDATION_FAIL which the user could override either in their frontmatter or their /user/languages/xx.yaml files.

When the form page reloads with an error in the current configuration, we have, at the top of /user/plugins/form/templates/forms/default/form.html.twig:

{% if form.message %}<div class="alert">{{ form.message }}</div>{% endif %}

In the new configuration, we might have something like:

{% if form.messages %}
    <div class="error">
        <p class="error-form-message">{{ form.errormsg ? form.errormsg : 'FORM.VALIDATION_FAIL'|t }}</p>
        <ul class="error-list">
            {% for error in form.messages %}
                <li class="error-list-item">
                    {% if error.id %}
                        {% if error.type == "required" %}
                            <a href="#{{ error.id }}">{{ error.message.required ? error.message.required : `FORM.REQUIRED_FAIL`|t }}</a>
                        {% elseif error.type =="rule" %}
                            <a href="#{{ error.id }}">{{ error.message.rule ? error.message.rule : `FORM.INVALID_FAIL`|t }}</a>
                        {% endif %}
                    {% else %}
                        {% if error.type == "required" %}
                           {{ error.message.required ? error.message.required : `FORM.REQUIRED_FAIL`|t }}
                        {% elseif error.type =="rule" %}
                            {{ error.message.rule ? error.message.rule : `FORM.INVALID_FAIL`|t }}
                        {% endif %}
                    {% endif %}
                </li>
            {% endfor %}
        </ul>
    </div>
{% endif %}

where:

• form.messages is the array of form messages returned from the server.
• form.errormsg is the generic form error message supplied in the form frontmatter with a default fallback if it is absent.
• error.type is the type of error, rule or required. This is so that the appropriate error message can be displayed if multiple validations are placed on the same field.
• error.message.rule and error.message.required are the custom error messages supplied within the frontmatter of the form with a default fallback if they are absent.
• the conditional testing for error.id would allow the individual who filled out the form to jump to the input where the error occured. This is particularly useful for longer forms.

I welcome your thoughts!

[rhukster]

This is a great set of suggestions. I think we've not put much effort into the server-side validation because all our testing was done using HTML5 browsers and that was catching the errors client-side.

The server side validation definitely needs some improvements and your thoughts will be helpful with that.

[bovisp]

@rhukster thanks for your feedback. My reasoning behind the proposal was to ensure that manual form creation is a flexible as possible. This allows developers and designers to craft custom feedback for any context and greatly eases translations by moving everything to the front matter.

Unfortunately, there are still organizations that have to support non-HTML5 browsers such as IE8-9 (well IE9 is sort of HTML5) so we should make sure that the feedback from the server is as meaningful as possible.

Additionally, there are still quite a few die hards out there who feel more comfortable with having their forms validated server-side whether it's an HTML5 browser or not.

I would be happy to help you test any solution that you come up with in the future. Keep up the great work Andy!

[Sommerregen]

I agree with you @bovisp . Having fields only client-side validated opens the door for spoofing/XSS the servers system. That's why I implemented server-side validation using Grav built-in functionality for Forms.

Your ideas @bovisp are closely those already implemented in the https://github.com/Respect/Validation library. We may learn from the best and find a way making it useful for Grav, too.

[mahagr]

I have done the beginnings for this, but a lot more work is needed for Validation class to fully support this. The main issue is that the class has no support for error messages, except for the custom message you can set in yaml.

[bovisp]

@mahagr @rhukster . Thank you so much for taking up this issue. I'm afraid that I can do little to help regarding the Validation class, but I would be happy to help in the testing of any new functionality, if need be.

Does Respect/Validation support error messages in their classes? If so, would we be able to "borrow" their functionality?

[mahagr]

I did take a deep look into Respect/Validation, but unfortunately it doesn't really fit into our use case (one reason being that error messages cannot be translated from Grav). What it does, though, is that it has a single error message per field, which has some pretty complicated logic to create the error message. So basically it would take some time to recreate similar behaviour.

[bovisp]

I'm creating a form plugin that handles:

• validation
• sanitizes data
• filters a page collection using a form
• exception messages by validation type
• ability to add exception messages by validation type for any language
• ability to add custom exception messages by validation type in the page yaml
• a few other things

I'm making use of Respect/Validation for the form validation only. I'm bypassing their Exception templating system in favour of something custom so that I can get over the language issue. @mahagr, I concur that their logic for error messages is too complicated to try to get to work with Grav, thus my custom, rather inelegant (yet functional) solution. I use auraphp/Aura.Session for handling CSRF token generation, which makes use of their complex RandVal class. I use daylerees/sanitizer for its nifty way of handling form sanitation.

I've been at this for two weeks now and I'd say I'll have something close to stable in the next two weeks. I have been developing this for our internal Grav website at work. It's my first plugin, but Grav core has made this a very enjoyable experience.

You can find the code here: https://github.com/bovisp/grav-plugin-interstellar-form

[mahagr]

Will need to bookmark that one; I'm really interested to see what you have created. Validation is a thing that I'd really want to improve at some point, but something that takes a lot of time to do properly.

[bovisp]

@mahagr. Thanks. Do keep in mind that my plugin does not use anything like their templating system. It's really just as simple as all exception messages for a rule for a single language being contained within an individual method. Therefore if you have five languages that you need to support, you are going to need five exception methods for every rule you want to support. Is this sustainable, I don't know.

Also keep in mind that this also my first object oriented PHP project of any kind. My feeling is that lots of people look to the Grav core dev team to solve their problems without trying to solve it themselves. I've decided to take the time to learn PHP properly and solve it myself! I have used several of your team's plugins as a starting point though

What I do believe it that there should be a central plugin for form creation, sanitation and validation. This way it is much more clear how the form process runs from start to finish.

In addition, I have pushed all form element creation to a class rather than a set of partial templates. I wanted to abstract as much of that away as I possibly could. Additionally, my plugin only wraps form elements in an

tag so as to give designers maximum freedom regarding how they are styled. I will give designers the ability to add form element and wrapping element classes in their page front matter.

I'm taking a break from coding to start on documentation. I'll post an update here every week to advise on progress so stay tuned! Keep up the great work and I can't wait to buy Admin Pro!

[mahagr]

I don't like the exception part, even without having really looked into the code. There has to be a better way to do it via translations.

There is a plugin for forms and all of the forms in Grav use that plugin, including admin.

[bovisp]

@mahagr I'm not very happy with my current approach either regarding exceptions. Is there a way to pass variables into Grav translations? That might help.

[bovisp]

@mahagr . OK, I've had a look at how the translate() method works on the language class. Let's say we have a length rule and in the front matter and we have stipulated that the length of an simple text input can only be between 2 and 15 characters. If the input does not satisfy this rule, we can have a translation argument in a languages.yaml file:

en:
  PLUGIN_TEST:
    LENGTH_BETWEEN: "The length of your response must be between %d and %d characters"

Then in my Length.php rule class, if the rule does not validate for a form element, the class would first check to see if a custom message for that rule exists in the front matter, if so, return that otherwise, return something like:

return (Grav::instance()['language'])->translate([PLUGIN_TEST.LENGTH_BETWEEN, $minValue, $maxValue], null, true);

which would result in:

The length of your response must be between 2 and 15 characters

The advantage here is that we can give the user multiple errors per form field so that know exactly what went wrong with their input rather than saying your input is invalid

How does that sound?

[Sommerregen]

@bovisp Sorry to jump in here. But I really think, that everything can be done right now with a few changes in the Forms plugin. I previously had something in mind, but ran out of time. The current state of the art is, that there is an onFormValidationError event and you can set validation patterns and messages (see https://learn.getgrav.org/forms/blueprints/fields-available). What I don't know exactly, is whether the validation message has multi-language support.

The thing to improve is basically the server-side validation part. Right now, the validation stops at the first invalid field. But when looking at Grav core there is no reason to stop the validation after that. Second, there is no way to hook into the validation process. What my idea was, was to allow validation form actions (like the process actions) and a hook which can be used by other plugins to add custom validation procedures (a captcha plugin for example). This should make the whole process easier and IMO allow everything you are looking for.

[bovisp]

@Sommerregen this would be the most preferred route here. My issue with Grav form validation is that, as you stated, the process bails out at the first error. I would prefer a full validation take place with the errors collected in an array. Additionally, Grav should have the ability to produce multiple error messages per form field. If the field is required, needs a certain number of characters entered and needs the value to be numeric, there should be an error message produced for each one of those validation failures. This is what Laravel and Respect\Validation do.

For the user, filling out forms is a chore. If they make a mistake, it's important to help them along as much as possible to resolve their issues and advance to a successful form completion.

[bovisp]

Hi everyone. Just checking in regarding this issue. Is there anything that I can do to help? If not, should I close this issue?

[mahagr]

Validation is something that needs to be improved upon. I wouldn't close this issue because of the current implementation is just a quickly hacked together version, which I meant to improve long time ago.

Unfortunately, the validation libraries out there do not really fit into Grav, as they are usually meant to validate either HTTP POST or abstract data, not a combination of them both. In addition to validation, we also need a way to convert string values from the response into the real types we are using, rendering some of the better validation methods invalid. Not to even mention nested data...

[joebordes]

Have you seen this validation library? https://github.com/vlucas/valitron

[mahagr]

I have it starred. I'm not sure why I didn't pick it -- I think that the reason was because of it didn't seem to support nested arrays in its validation.