Sulu: Allow to display custom validation and error messages in Form

[antonstsk]

Problem description

I'm extending the admin area with custom entities and having issues setting unique fields. The unique check works but it's completely hidden for the user.
On the screenshoots I'm trying to save item with already used title.

Proposed solution

Provide a possibility of using custom error messages or at least marking wrong fields.

[alexander-schranz]

Thank you for creating this proposal 👍 .

We definitely should add a way to add an error message from the backend. I think we should implement something like the rfc7807. This is also supported by the FOSRestBundle in newer version (2.8 / 3.0). The json format is very flexible as the rfc allows to add custom fields to the response json.
The rfc has an own mimetype application/problem+json but this is all supported by FOSRest we just would need to implement the JS part that the message is displayed. And decorate the Normalizer (which we currently also do for providing the exception code in the fos rest legacy format).

[antonstsk]

I wish I can help... If someone that knows Sulu feels like can help on planning the job needed to have that functionality please be my guest! I can definitely invest some time on this!

[alexander-schranz]

@antonstsk

Wouldn't it be great to create a project in the sulu repository for people like me that don't know much about Sulu but still can work with Symfony + React and can help on so necessary functionality 😅

Nobody directly works in the sulu repository also every core developer has there own sulu fork. So you click in this repository on the right top on Fork then clone your fork, there you create a new branch and then you push it into your fork and create PR to the main sulu repository. Or what kind of project are you searching for?

I wish I can help... If someone that knows Sulu feels like can help on planning the job needed to have that functionality please be my guest! I can definitely invest some time on this!

I think in this case the best one which is most familiar how this could be implemented in the JS is @danrot.

[alexander-schranz]

Some additional information about the RFC7807 how it looks like and how we could use its data:

Example Full RFC7807 response as json:

{
    "type": "about:blank",
    "title": "Title",
    "detail": "More Detail",
    "status": 400,
    "instance": "/admin/api/contacts/1",
    "any-additional-property": "some-value"
}

All properties are optional so we don't need provide them.
I would use the information the following way.

type: The type is an url to a problem type page. In our case when set he would be able to click on the error and the page would be open this error page usage could be linking to a trouble shooting page in the documentation. When not set or when is about:blank there it is not able to click on it.

title: Should be the detail which should be displayed. I think we should truncate too long titles.

detail: Detail can contain some additional information. There should be some show detail button which expand the error to show also this information and the full title if truncated. In the first implementation we could maybe just display response.title + ': ' + response.detail or something.

status: As all fields are optional I would avoid this duplicated information as it represent the http status code and I'm really not a big fan having the http status code as part of the http content.

instance: Not sure if I would return this information as its not important for the end user.

any-additional-property: In our case maybe additional property could be skin of the error if we have different error skins (error, warning) or maybe a error icon or something. In some cases we have an exception code so we should also display that and return this as code.
In future it would also be nice in the Form view to add error messages directly to a field so some additional property like the following would maybe be great there which need to be handled by the FormRenderer:

{
    "title": "Invalid Form Data submitted",
    "fieldErrors": [{
        "path": "/blocks/0/password",
        "title": "Password need to contain number",
    }, {
        "path": "/blocks/0/password",
        "title": "Password need to contain a special character",
    }, {
        "path": "/blocks/0/username",
        "title": "Username can not contain spaces",
    }],
}

but that's definitely another issue and we should first implement some basic error handling.

/cc @sulu/core-team what do you think about the rfc7807?

[danrot]

Until now we have always used a simple code property on the response. See e.g. how the warning when a form has been changed in the mean time is shown. I would keep it like this for now, I think the only thing we would have to change, is how the error message is chosen. Currently we show a generic one, but we would have to select it somehow based on the passed error code.

Not sure how to handle this though... We could build a translation string dynamically in the JS code:

this.errors.push(translate('sulu_admin.form_save_server_error-' + error.code));

Then all the backend developer would have to do, is to return a code property in the error response, and define the correct translations.

The alternative would be to define a mapping somewhere (symfony configuration?) between error code and translation key. Would have the advantage that it is easier to follow, because if you search for a translation string like "sulu_admin.form_save_server_error-2305" you won't find any occurences, and might lead to people thinking they can delete it.

I am generally in favour of reusing existing standards, but I think in that case it would be a bit of an overkill. Would maybe do it if start from scratch, but right now it would be quite a big BC break, and I am not sure how we would handle different translations. Until now the REST API does not return messages like this localized.

[alexander-schranz]

@danrot going on code I would really avoid. We should show clear translated messages in the API and not translated it on the client. I would really avoid using code mapping think its not a great DX and UX if we have only just magic code responses.

[nnatter]

I think i would prevent using numerical codes too, because its hard to figure out whats going wrong from the API response alone then. The firebase SDK which i am currently using for a project returns error keys (eg. auth/invalid-email, auth/user-not-found) (they still call this codes). If we dont want the REST API to return these values localized, that might be an alternative.

Additionally, as @alexander-schranz already wrote, we should not only think about how to display a general error, but also about how to mark specific fields as invalid from the backend when tackling this. At least the solution for displaying a general error should be extendable to allow for this in the future.

[antonstsk]

Don't know about implementation yet but for sure from the UX point of view the best solution would be having the incorrect field marked and with the specific message below. The top flash message "There was an error when trying to save the from" can remain the same.

About the translations wouldn't it be better to show the message that comes from the backend?
That way we can use default messages included with @Assert annotations or specify the translation key.

Don't know how Sulu works under the hood but with the way that Symfony handles the form validation seems pretty clean to me.

[danrot]

@nnatter @alexander-schranz While I totally agree with what you said, I don't think that we should implement any of this now for BC reasons. I think that's something we could change when working on Sulu 3.0 (would also make more sense from a timing perspective, as we have other priorities right now).

@antonstsk The initial idea was to use JsonSchema for stuff like that, which has the advantage that the validation can already happen at the frontend immediately after the user has typed something. In addition this schema could also be used for server side validation (not implemented yet). But I get that some stuff (mostly when something has to be compared with other existing data) won't work that way. Therefore we probably have to think about how to return server-side errors tied to a specific field. I think we could go for the fieldErrors approach @alexander-schranz has suggested above, and add this as an additional field in the current error response.

[nnatter]

Personally, I really dont think we should postpone this to the next major version. In my opinion this is a critical feature for an application like Sulu. I have not investigated this, but I feel like the places that use numerical codes to indicate errors in the API are limited. So I am confident that its possible to find a solution that feels adequate without breaking too much 🙂

[danrot]

I didn't say that I would postpone this in general, what I meant is that we should probably stick with the error codes for now, since we are already using that in our REST API. IMO the REST API is one of the most important things we should keep backwards compatability, since there is the backend and frontend application being involved. I get that this might be not optimal, but it is not something I'd like to change within a minor release.

[danrot]

Don't know about implementation yet but for sure from the UX point of view the best solution would be having the incorrect field marked and with the specific message below. The top flash message "There was an error when trying to save the from" can remain the same.

About the translations wouldn't it be better to show the message that comes from the backend?
That way we can use default messages included with @Assert annotations or specify the translation key.

Don't know how Sulu works under the hood but with the way that Symfony handles the form validation seems pretty clean to me.

Read over that idea too quickly: Leaving the top message the same, and make it possible to add the error to a single field by using something like the fieldError to error responses @alexander-schranz mentioned above might work without introducing any BC break, since we would only add a new field to the response... Still would not change the return type of the error code for now.

[nnatter]

Yeah, I agree that we should not break BC in the API. I just searched through the codebase and it looks like i underestimated the number of numerical error codes that are already in use. I am not sure if all of them occur in the REST API, but the following Exception classes set the code property which will be included in the JSON response if they are thrown during an API request:

KeywordNotUniqueException
CacheNotFoundException
CollectionNotFoundException
CollectionTypeNotFoundException
FilenameAlreadyExistsException
FileNotFoundException
FileVersionNotFoundException
FormatNotFoundException
FormatOptionsMissingParameterException
GhostScriptNotFoundException
ImageProxyInvalidFormatOptionsException
ImageProxyMediaIdNotFoundException
ImageProxyMediaNotFoundException
ImageProxyUrlNotFoundException
InvalidFileException
InvalidFileTypeException
InvalidMediaTypeException
InvalidMimeTypeForPreviewException
MaxFileSizeExceededException
MediaNotFoundException
MediaTypeNotFoundException
OriginalFileNotFoundException
UploadFileNotSetException
ProviderNotFoundException
RouteDefaultsProviderNotFoundException
TemplateNotFoundException
TokenNotFoundException
TwigException
UnexpectedException
WebspaceLocalizationNotFoundException
WebspaceNotFoundException
RoleKeyAlreadyExistsException
RoleNameAlreadyExistsException
UserNotInSystemException
EmailNotUniqueException
EmailTemplateException
InvalidTokenException
MissingPasswordException
NoTokenFoundException
TokenAlreadyRequestedException
TokenEmailsLimitReachedException
UsernameNotUniqueException
ResourceLocatorAlreadyExistsException
MissingDomainPartException
RouteNotRemovableException
TitleAlreadyExistsException
InvalidHashException
UniqueConstraintViolationException

[nnatter]

There were a few changes regarding validation in the recent weeks, therefore I want to add a small update here. We have added a new PropertyMetadataMapperRegistry in #5640 that is used to generate a specific JSON schema for a given form property. The idea is to generate a JSON schema as specific as possible for a given form configuration. This JSON schema then can be used for data validation in the frontend (already done) and also in the backend (not done yet).

For example, #5672 will add a TextPropertyMetadataMapper that generates a JSON schema that validates if the min_length and max_length param of a text_line is fulfilled. This allows to use and validate a property like this in the form configuration:

<property name="text_line" type="text_line">
    <meta>
        <title lang="en">Text Line</title>
        <info_text lang="en">There are no commas allowed in this field</info_text>
    </meta>

    <params>
        <param name="pattern" value="^[^,]*$"/>
        <param name="min_length" value="2"/>
        <param name="max_length" value="3"/>
    </params>
</property>

If the content-manager does not fulfill the pattern, min_length and max_length in the administration interface, a respective error will be displayed.

That said, we know that this does not solve every usecase. We still need to find a solution for allowing things like:

• displaying a custom error message returned by the server above the form
• displaying a custom error message returned by the server for a specific field
• validating entity-wide unique constraints such as keys (eg: category key)
• validating constraints relative to another property (eg: start- and end-date as described in #5193)
• validating data based on some specific domain logic

As stated above, the new services allow to generate a JSON schema that could be used for data validation on the server in the future. When implementing a solution for these cases listed above, this solution should be compatible with such a JSON schema validation.
At least, this means that we need to define a format (or use an existing like suggested in https://github.com/sulu/sulu/issues/5312#issuecomment-639090000) for returning validation errors that can be used in a custom controller, but also be generated when using JSON schema validation.