Error when saving config entity with "Link to entity" checked if field formatters

Patch #5 didn't pass the test so move to need work.

Hi @KittenDestroyer,

The message entity doesn't have the links like canonical that is being used in getEntityUrl.

protected function getEntityUrl(EntityInterface $entity) {
    // For the default revision, the 'revision' link template falls back to
    // 'canonical'.
    // @see \Drupal\Core\Entity\Entity::toUrl()
    $rel = $entity->getEntityType()->hasLinkTemplate('revision') ? 'revision' : 'canonical';
    return $entity->toUrl($rel);
  }

That's why this error is coming.

if ($this->getSetting('link_to_entity') && !$entity->isNew()) {
      $url = $this->getEntityUrl($entity);
    }

The condition which you added "!$entity->isNew()" will affect all entity. So, it will be good to hide the "Link to the Content" option for that entity, which doesn't have canonical links.

Thanks,
Ren

@KittenDestroyer #13 worked for me.

Hi @KittenDestroyer

$entity = $items->getEntity();
    $entity_type = $entity->getEntityType();

    if ($this->getSetting('link_to_entity') && !$entity->isNew() && $entity_type->hasLinkTemplate('canonical')) {
      $url = $this->getEntityUrl($entity);
    }

There are no need to check the entity is a new condition. So, it should be removed.

Thanks,
Ren

Yes, we do need the !isNew() check, that fixes node preview with a field is configured like that.

This does need a test.

@Berdir
I've kept isNew() check as it is.
Added following items:
- Add test only patch
- Add actual test cases.

Fixing coding standard issues.
Taken diff from comment #13 only.

+++ b/core/modules/field/tests/src/Functional/String/StringFieldTest.php
@@ -112,4 +113,55 @@ public function _testTextfieldWidgets($field_type, $widget_type) {
+   * Test "link_to_entity" feature on fields which are added to config entity.
+   */
+  public function testLinkToContentForConfigEntity() {
+    $field_storage = FieldStorageConfig::create([
+      'type' => 'string',
+      'entity_type' => 'contact_message',
+      'field_name' => $field_name = strtolower($this->randomMachineName()),
+    ]);

A bit unsure about adding a dependency ton contact module in field module, even if it's just a test.

What about doing it with a test entity type in a kernel test, like the existing test coverage in \Drupal\Tests\field\Kernel\String\StringFormatterTest::testStringFormatter? We can either use a test entity type or any of the content entity types in web/core/modules/system/tests/modules/entity_test/src/Entity, it doesn't actually need to be a config entity type.

@Berdir I think we probably need a config entity for the same.
I tried to reproduce for node and user by adding a field and following the steps, I was not able to reproduce it.
I'll try to refactor it and implement it with Kernel tests.

Sure, node/user is fine, those have a canonical link template, you need something that doesn't. For example \Drupal\entity_test\Entity\EntityTestLabel.

@Berdir
I've refactored the test cases and now we are using kernel tests.
Currently, I've just added test cases for a testing entity by doing render.

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/StringFormatter.php
@@ -92,14 +92,15 @@ public static function defaultSettings() {
+    if ($entity_type->hasLinkTemplate('canonical')) {
+      $form['link_to_entity'] = [
+        '#type' => 'checkbox',
+        '#title' => $this->t('Link to the @entity_label', ['@entity_label' => $entity_type->getLabel()]),
+        '#default_value' => $this->getSetting('link_to_entity'),
+      ];

Here in the above snippet, we are displaying link_to_entities checkbox for the entities which has a canonical path.
Do you think if we need to cover this under functional tests and check if the checkbox is visible to certain entities only?

Good question. As far as I see, we don't have existing test coverage for that checkbox nor the summary output and generally rarely do for formatter settings UI I think. So maybe we can get this in without, but will have to be decided by the core maintainers, lets see what they think :)

I did run the the test locally and verified that it fails without the fix.

Agreed we don't need to test the individual formatter settings form here, testing the entity display config seems fine.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!