Issue #3369035: Notification entity conditions can not rely on other template configuration #3437
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Entity conditions are executed in ActivityLoggerFactory::createMessages before some of the other checks are evaluated. This can cause an entity condition that may only be usable on specific bundles to be called on other data.
Solution
We re-order the checks so that we check for matching properties first and only at the end execute the more expensive entity condition plugins. This also makes it easier to move the conditions into a database query when we move into a different configuration structure in the future.
Note: a nicer solution would be to make the conditions a more cohesive whole, but that's too big a change for now, so we just change the evaluation of conditions from generic (e.g. bundle) to specific (condition on data of the entity) to make our developer lives easier.
Issue tracker
https://www.drupal.org/project/social/issues/3369035
How to test
Discovered and covered with #3384
activity_bundle_entities
and uses the condition inactivity_entity_condition
Without the change you'll see that the
ActivityEntityCondition
plugin you created crashes the notification process because the condition is executed for entities it won't care aboutAfter the change the condition is only called if the other fields on the notification template matches.
Definition of done
Before merge
After merge
Release notes
For developers: EntityCondition's for notifications are no longer checked if any of the other conditions of a template does not apply.