Token Exists condition no longer accepts tokens

The purpose of Token: exists is to test a token by name. And order_item:purchased_entity:entity:weight:number is not a token name nor would be [order_item:purchased_entity:entity:weight:number]. The former is a token named order_item with a chained list of properties. The latter would resolve to the value of that token. In this example, the token name would just be order_item, and this condition can only test exactly that.

The condition Entity: field value is empty should work though when you load the entity [order_item:purchased_entity] into a token (e.g. orderedentity) and then use that as the entity in that condition and check for its field value.

Proposing a change in the MR !392 to keep the token reference validation in place, but allow the configuration to refer to order_item:purchased_entity:entity:weight:number (without brackets) such that the evaluate function now tries to replace token data by adding the brackets if not otherwise found already. This should give us both, the config validation as well as the deep resolution of token names.

More details about this in this Slack thread.

Hi,

This looks like a good fix for the issue, allows you to support this need.

I'm wondering about consistency and user expectations, though, particularly around how these are chained together.

We know that if we're inside a token, you walk these relationships with : (colon) characters. However, if you're not inside a token -- if you're in a Twig context, or Javascript, or other places in ECA that refer to sub-fields, the separator is a . (period).

order_item:purchased_entity:entity:weight:number

vs

order_item.purchased_entity.entity.weight.number

And of course PHP is entirely different, depending on whether you're looking at an array or object, and declaring or assigning...

How is a non-developer supposed to know what to use?

I'm not sure I can justify this, but at least the way I think about it currently, I think we should use : as the separator ONLY INSIDE tokens with square brackets, and if there's no brackets, use dots.

That would just mean adding a string replacement inside your MR to replace colons with dots... This wouldn't actually break the colon syntax, but I think it's a pattern that might be more familiar?

I'm wondering about consistency and user expectations, though, particularly around how these are chained together.

This is a good point. However, the different syntax for e.g. tokens, twig, javascript and others is not originated by ECA, it comes from the various technologies or APIs that are behind those.

However, we could easily replace periods by colons in the context of this plugin. That wouldn't make any difference. So, users could use either order_item:purchased_entity:entity:weight:number or order_item.purchased_entity.entity.weight.number and both would respond with the same result.

However, we've broken tests now and need to clean them all up in \Drupal\Tests\eca_base\Kernel\TokenExistsTest::testTokenExists where the tests are falsely test for token names with brackets, expecting the same result as without, which is no longer correct and shouldn't have been before, but this went through unnoticed, since the validation happens when saving the model and that is not part of the tests.

I think all the tests with brackets should now respond with FALSE. But we should maybe also add new tests?

I wanted to address the latest bits and realized that the solution was already in place: we can already use order_item:purchased_entity:entity:weight:number as the token name, and both form validation and later the condition evaluation do work as expected. So, there is nothing to do in the code.

jurgenhaas → closed merge request !392