Allow field tokens also for base fields

Don't forget to set issues to needs review when posting patches, so that they get tested.

Fix failed testing and switch to renderPlain to avoid fatal errors when rendering for example tokens in metatag fields.

Re-rolling.

Thanks for the reroll. I think that patch is doing too much. We shouldn't remove any existing tokens here IMHO. Instead, I'd prefer to more or less go back to the original patch above, add the necessary bits from #2560191: Provide tokens for all entities out of the box to define the entity type, make sure to also remove the instanceof check when defining field info and most importantly, tests.

We should be able to have a test similar to the existing one for node except it would be for EntityTest from entity_test.module including testing the output and info of various base fields like ID, label, and so on.

Follow-up: Then we need to review which of the existing tokens can be dropped because they're also covered through this generic implementation. We will also risk duplicate tokens for existing entity types, which is a bit problematic. For example, node defines node:author, this would expose node:uid which would however essentially result in the same.

Attached find a new patch based on the previous ones that adds support for all content entity types and base fields against 8.x-1.x (918d0d5)

Thanks.

We already have code that adds token info for configurable fields in field_token_info_alter(). Instead of adding a new block, we should simply update that to remove the FieldStorageConfigInterface check. Its not really at the right place but we can easily move it once it's working. Or maybe even here.

There's also token_token_info_alter() which for example defines a label token for each entity type that exists as a token type. We can update that to define entity types that don't exist yet. We should also support the token_type setting and respect that if set.

Similar in the second part, instead of explicityl looking for both interfaces, just remove that check. We want to support anything.

One thing to think about, at least for a first version, might also be to only define tokens for base fields for entity types that we define ourself. The reason is that those that already exist likely also have many tokens that overlap/duplicate those that we'd define (e.g. author vs. uid). We could avoid that if we only do it for entity types that have no custom integration.

patch #16 is not running for me: Drupal 8.1.1 and Token 8.x-1.x-dev and PHP 7.x

some patches using (e.g. in token.tokens.inc around line 1265)
($entity->get($field_name)
(e.g. "Chained tokens for field tokens")

and some others
($entity->get($name)
(e.g. this one and "Replace field tokens by empty strings when there is no field value")

i'am not sure what's right or wrong...

Made the changes suggested by @Berdir and added a test.

Adding a check that a uid token isn't generated for the node entity type.

Nice, really close now.

1. +++ b/tests/src/Kernel/FieldTest.php
   @@ -201,4 +203,49 @@ class FieldTest extends KernelTestBase {
   +    // Create a new contact_message entity and verify that tokens are generated
   +    // for its base fields.
   

   Can we add a comment here why we chose a contact message:

   We use contact messages as that entity type does not provide any tokens by default.

2. +++ b/tests/src/Kernel/FieldTest.php
   @@ -201,4 +203,49 @@ class FieldTest extends KernelTestBase {
   +
   +    // Verify that Node doesn't have a uid token.
   +    $this->assertNoTokens('node', ['node' => $entity], ['uid']);
   

   $entity is not a node :)

   Check token info instead, that is enough. getTokenInfo('node', 'uid')

3. +++ b/token.tokens.inc
   @@ -69,7 +70,13 @@ function token_token_info_alter(&$info) {
   +        'new-entity-type' => true,
   

   Lets use 'module' => 'token'. We do the same for field tokens for the same purpose.

4. +++ b/token.tokens.inc
   @@ -1164,16 +1171,19 @@ function field_token_info_alter(&$info) {
   -      if (!($field instanceof FieldStorageConfigInterface)) {
   +      // Ensure the tokens exist.
   +      if (!isset($info['types'][$token_type])) {
   

   now it is only about ensuring that the token type exist.

5. +++ b/token.tokens.inc
   @@ -1164,16 +1171,19 @@ function field_token_info_alter(&$info) {
   +      // Ensure the token implements FieldStorageConfigInterface or was newly
   +      // created.
   +      if (!($field instanceof FieldStorageConfigInterface) && (empty($info['types'][$token_type]['new-entity-type']))) {
            continue;
   

   or is defined in token module.

There. However, I'm sure there's a more elegant way to check what tokens are defined for the Token module..

if (!($field instanceof FieldStorageConfigInterface)) {
  if (isset($info['types'][$token_type]['module'])) {
    if ($info['types'][$token_type]['module'] !== 'token') {
      continue;
    }
  }
  else {
    continue;
  }
}

Small cleanups, tentative RTBC.
Added a todo to fix _token_field_label() in a followup, cause it's not following core logic anymore.

1. +++ b/token.tokens.inc
   @@ -70,10 +70,11 @@ function token_token_info_alter(&$info) {
        if (!isset($info['types'][$token_type]) || !isset($info['tokens'][$token_type])) {
   -      // Define entity type that doesn't exist yet.
   +      $entity_type_label = Unicode::strtolower($entity_info->getPluralLabel());
   +      // Define tokens for entity type's without their own integration.
          $info['types'][$entity_info->id()] = [
            'name' => $entity_info->getLabel(),
   -        'description' => t('Tokens related to %entity entity.', ['%entity' => $entity_info->getLabel()]),
   +        'description' => t('Tokens related to @entity_type.', ['@entity_type' => $entity_type_label]),
            'needs-data' => $entity_info->id(),
   

   strotolower() is hardcoding something that only some languages do, it will look strange in german for example.

   also we should at least include plural in the placeholder to make it clear to translators.

   Maybe the easiest way out: Do not add a description. Does it actually add something useful?

2. +++ b/token.tokens.inc
   @@ -1178,15 +1179,12 @@ function field_token_info_alter(&$info) {
   -        }
   +      $provider = '';
   +      if (isset($info['types'][$token_type]['module'])) {
   +        $provider = $info['types'][$token_type]['module'];
   +      }
   +      if (!($field instanceof FieldStorageConfigInterface) && $provider != 'token') {
   +        continue;
          }
   

   We have _token_module() to get the module of a token, maybe we could also add a function (or a method on our token service replacement) for this use case?

   Can be a follow-up.

3. +++ b/token.tokens.inc
   @@ -1226,7 +1224,9 @@ function field_token_info_alter(&$info) {
   + *
   + * @todo Resync this method with views_entity_field_label().
     */
    function _token_field_label($entity_type, $field_name) {
   

   The return value of the function is actually out of sync on purpose, the structure that that function returns is very weird and way too complicated for what we need. However, the deterministic sort based on the label might make sense.

strotolower() is hardcoding something that only some languages do, it will look strange in german for example.

Yes, but that is a known core bug. We have an open issue about getLowercaseLabel() and friends.

I'm fine with not having a description.

Here we go, no description.

Forgot to update origin.

+  public function testBaseFieldToken() {

Might make more sense to call this testBaseFieldTokens(), can be changed pre-commit.

Berdir asked me to test manually, I did, with Commerce and Pathauto (latest -dev), works fine.

Pushed, thanks everyone!

diff --git a/tests/src/Kernel/FieldTest.php b/tests/src/Kernel/FieldTest.php
index 09f31db..66c2182 100644
--- a/tests/src/Kernel/FieldTest.php
+++ b/tests/src/Kernel/FieldTest.php
@@ -200,7 +200,7 @@ class FieldTest extends KernelTestBase {
   /**
    * Test that tokens are properly created for an entity's base fields.
    */
-  public function testBaseFieldToken() {
+  public function testBaseFieldTokens() {
     // Create a new contact_message entity and verify that tokens are generated
     // for its base fields. The contact_message entity type is used because it
     // provides no tokens by default.
@@ -234,9 +234,9 @@ class FieldTest extends KernelTestBase {
     // Test the metadata of one of the tokens.
     $tokenService = \Drupal::service('token');
     $token_info = $tokenService->getTokenInfo('contact_message', 'subject');
-    $this->assertEqual($token_info['name'], 'Subject');
-    $this->assertEqual($token_info['description'], 'Text (plain) field.');
-    $this->assertEqual($token_info['module'], 'token');
+    $this->assertEquals($token_info['name'], 'Subject');
+    $this->assertEquals($token_info['description'], 'Text (plain) field.');
+    $this->assertEquals($token_info['module'], 'token');
 
     // Verify that node entity type doesn't have a uid token.
     $this->assertNull($tokenService->getTokenInfo('node', 'uid'));

Not sure if the API in core changed since this patch was committed, but the line referenced in this patch doesn't catch base fields added by hook_base_field_info().

Base fields implement \Drupal\Core\Field\FieldStorageDefinitionInterface and do not implement \Drupal\field\FieldStorageConfigInterface. This 1 line change seems to make it work.

Moved that last patch over to #2877268: uuid support since that is open and this is closed.