Perform entity validation in bulk actions

What should the expected behavior be when validation fails? I think there are two general options..

1. If an entity fails validation, skip the entity and provide a message to that effect. Continue applying bulk action to other entities.
2. Fail the entire bulk action if any entity fails validation.

I assume option 1 is the easy first step. Is it worth giving 2 much thought? If you select 50 entities, and validation fails on half of them, isn't that kinda annoying? Maybe the reason they fail validation should be the larger consideration..

It would be difficult to fail applying the action to *all entities* since it would require some kind of "undo" logic.. Consider 10 entities, if the 5th one fails, but the previous 1-4 were OK (and already saved), the change would need to be reverted to properly "bail" the entire action.

Or, I suppose the action could go through and make the change but only save the entities as a last step if they all pass validation.

If an entity fails validation, skip the entity and provide a message to that effect.

I'm curious the best way to go about this. My first simple approach was to simply loop though all of the validation errors and add warning messages for each error but the issue is that the validation errors aren't meaningful by themselves, they really need the context of which field they are associated with. This is called the "property path" with constraint violations. In entity forms the property path is used to highlight the correct input element associated with the constraint violation.

For example, with this simple approach when assigning an invalid flag (as described in the issue summary) the validation message is simply: "The value you selected is not a valid choice." (see attached screenshot). With the property path of "flag.0" this message makes more sense, but I'm not sure how to represent this property path context in warning message. Can we do better than simply including "flag.0" ?

We should also consider that a custom constraint may span multiple fields, eg: "do not allow the 'needs review' flag if the asset is archived", in which case the property path may not be coupled to a single field. Also worth noting that constraints can be added to individual field properties eg: value or target_id on a more granular level.

One solution to this problem is to override the "default" constraints with messages that are more descriptive. I believe that the default constraints for a given field could be replaced altogether (with new constraint classes), or it seems that individual messages can be overwritten (this works for the flag action form, but I haven't tested more thoroughly). For example, modifying the flag field list_item definition's AllowedValues constraint message:

    ..... field definition
    /** @var BaseFieldDefinition $field */
    $field = \Drupal::service('farm_field.factory')->baseFieldDefinition($field_info);
    $field->getItemDefinition()->addConstraint('AllowedValues', ['message' => 'The flag you selected is not allowed.']);
    $fields['flag'] = $field;

If modifying the constraint message like this does not have any other side effects then I think this could be a good path forward. It has the added benefit of a more descriptive constraint message which I imagine would be preferred in most other places too. Unfortunately it still isn't very easy to include the "invalid value" and "allowed values" in the error message without implementing a custom constraint eg: "The "monitoring" flag is not allowed on land assets. Use "priority" instead."

Alternatively, we could naively make the warning message "Could not flag {entity}: validation failed." - but IMO this isn't that helpful anyways.

Also worth noting that individual bulk actions could perform more selective validation checking themselves - for example, the flag action could explicitly check for validation errors on the entity.flag field and provide a more descriptive error message since the action has knowledge of that specific field. For flags this wouldn't be worthwhile since we implemented logic to prevent "invalid" choices from the UI (thus making the situation described in issue summary a bit irrelevant.. :-) but the idea still holds.

BUT my understanding is that this issue is aiming to catch validation errors more generally, when an action might not know about constraint provided by another module. This is the more robust solution anyways. I'd love to see this working.

TL;DR I feel like this issue is *very* important, but could also be quite a bit of work from the get go. So maybe not a beta-blocker? It is likely something we would continue iterating on as edge cases are revealed (new actions are created) and we choose to add additional and more descriptive validation errors.

What do ya think @mstenta?

Here's a pass at a simple validation message when entity validation fails. I think these are all of the places we have actions that need to perform validation.... will also need to replicate the state change validation in the log module.

This is a hard thing to test without additional custom constraints. I tested this by modifying/adding a new flag that was limited to only a single log bundle and removed logic from the entity flag action form to allow *all* flags when using the action. This lets you see the validation warning message happen.