Reproduce steps:

  1. Install drupal8.6 and group;1.x-dev and commerce
  2. Create group_content:commerce_promotion plugin use these code
    /**
     * Provides a content enabler for nodes.
     *
     * @GroupContentEnabler(
     *   id = "group_promotion",
     *   label = @Translation("Group promotion"),
     *   description = @Translation("Adds promotion to groups both publicly and privately."),
     *   entity_type_id = "commerce_promotion",
     *   entity_access = TRUE,
     *   reference_label = @Translation("Title"),
     *   reference_description = @Translation("The title of the promotion to add to the group"),
     * )
     */
    class GroupPromotion extends GroupContentEnablerBase {
    }
    
  3. Create a group_content-commerce_promotion, when click save, the site crash
    The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\Exception\UndefinedLinkTemplateException</em>: No link template &#039;canonical&#039; found for the &#039;commerce_promotion&#039; entity type in <em class="placeholder">Drupal\Core\Entity\Entity-&gt;toUrl()</em> (line <em class="placeholder">224</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/Entity.php</em>). <pre class="backtrace">group_content_entity_submit(Array, Object)
    call_user_func_array(&#039;group_content_entity_submit&#039;, Array) (Line: 111)
    Drupal\Core\Form\FormSubmitter-&gt;executeSubmitHandlers(Array, Object) (Line: 51)
    Drupal\Core\Form\FormSubmitter-&gt;doSubmitForm(Array, Object) (Line: 589)
    Drupal\Core\Form\FormBuilder-&gt;processForm(&#039;commerce_promotion_add_form&#039;, Array, Object) (Line: 318)
    Drupal\Core\Form\FormBuilder-&gt;buildForm(&#039;commerce_promotion_add_form&#039;, Object) (Line: 48)
    Drupal\Core\Entity\EntityFormBuilder-&gt;getForm(Object, &#039;add&#039;, Array) (Line: 361)
    Drupal\group\Entity\Controller\GroupContentController-&gt;createForm(Object, &#039;group_promotion&#039;)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;Drupal\Core\EventSubscriber\{closure}() (Line: 582)
    Drupal\Core\Render\Renderer-&gt;executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber-&gt;Drupal\Core\EventSubscriber\{closure}() (Line: 151)
    Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw(Object, 1) (Line: 68)
    Symfony\Component\HttpKernel\HttpKernel-&gt;handle(Object, 1, 1) (Line: 67)
    Drupal\simple_oauth\HttpMiddleware\BasicAuthSwap-&gt;handle(Object, 1, 1) (Line: 57)
    Drupal\Core\StackMiddleware\Session-&gt;handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\KernelPreHandle-&gt;handle(Object, 1, 1) (Line: 99)
    Drupal\page_cache\StackMiddleware\PageCache-&gt;pass(Object, 1, 1) (Line: 78)
    Drupal\page_cache\StackMiddleware\PageCache-&gt;handle(Object, 1, 1) (Line: 41)
    Drupal\jsonapi\StackMiddleware\FormatSetter-&gt;handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware-&gt;handle(Object, 1, 1) (Line: 38)
    Drupal\webprofiler\StackMiddleware\WebprofilerMiddleware-&gt;handle(Object, 1, 1) (Line: 52)
    Drupal\Core\StackMiddleware\NegotiationMiddleware-&gt;handle(Object, 1, 1) (Line: 23)
    Stack\StackedHttpKernel-&gt;handle(Object, 1, 1) (Line: 665)
    Drupal\Core\DrupalKernel-&gt;handle(Object) (Line: 19)
    </pre>
    

The error caused here:
group.module

  if ($entity->access('view')) {
    $form_state->setRedirectUrl($entity->toUrl());
  }

Let's see the code of $entity->toUrl() (\Drupal\Core\Entity\Entity).

  public function toUrl($rel = 'canonical', array $options = []) {
    if ($this->id() === NULL) {
      throw new EntityMalformedException(sprintf('The "%s" entity cannot have a URI as it does not have an ID', $this->getEntityTypeId()));
    }

    // The links array might contain URI templates set in annotations.
    $link_templates = $this->linkTemplates();

    // Links pointing to the current revision point to the actual entity. So
    // instead of using the 'revision' link, use the 'canonical' link.
    if ($rel === 'revision' && $this instanceof RevisionableInterface && $this->isDefaultRevision()) {
      $rel = 'canonical';
    }

    if (isset($link_templates[$rel])) {
      $route_parameters = $this->urlRouteParameters($rel);
      $route_name = "entity.{$this->entityTypeId}." . str_replace(['-', 'drupal:'], ['_', ''], $rel);
      $uri = new Url($route_name, $route_parameters);
    }
    else {
      $bundle = $this->bundle();
      // A bundle-specific callback takes precedence over the generic one for
      // the entity type.
      $bundles = $this->entityTypeBundleInfo()->getBundleInfo($this->getEntityTypeId());
      if (isset($bundles[$bundle]['uri_callback'])) {
        $uri_callback = $bundles[$bundle]['uri_callback'];
      }
      elseif ($entity_uri_callback = $this->getEntityType()->getUriCallback()) {
        $uri_callback = $entity_uri_callback;
      }

      // Invoke the callback to get the URI. If there is no callback, use the
      // default URI format.
      if (isset($uri_callback) && is_callable($uri_callback)) {
        $uri = call_user_func($uri_callback, $this);
      }
      else {
        throw new UndefinedLinkTemplateException("No link template '$rel' found for the '{$this->getEntityTypeId()}' entity type");
      }
    }

    // Pass the entity data through as options, so that alter functions do not
    // need to look up this entity again.
    $uri
      ->setOption('entity_type', $this->getEntityTypeId())
      ->setOption('entity', $this);

    // Display links by default based on the current language.
    // Link relations that do not require an existing entity should not be
    // affected by this entity's language, however.
    if (!in_array($rel, ['collection', 'add-page', 'add-form'], TRUE)) {
      $options += ['language' => $this->language()];
    }

    $uri_options = $uri->getOptions();
    $uri_options += $options;

    return $uri->setOptions($uri_options);
  }

If the entity don't have 'canonical' or bundle's uri_callback, then an Exception will be throw out.

CommentFileSizeAuthor
#3 interdiff-3005554-2-3.txt604 byteslawxen
#3 3005554-3.patch1.24 KBlawxen
#2 3005554-2.patch970 byteslawxen

Comments

caseylau created an issue. See original summary.

lawxen’s picture

Status: Active » Needs review
StatusFileSize
new970 bytes
lawxen’s picture

StatusFileSize
new1.24 KB
new604 bytes

Change to Just catch UndefinedLinkTemplateException, not all exception

kristiaanvandeneynde’s picture

Project: Group » Commerce Core
Version: 8.x-1.x-dev » 8.x-2.x-dev
Component: Group (group) » Promotions
Assigned: lawxen » Unassigned
Status: Needs review » Needs work

I don't think this is a problem in Group, but rather Commerce.

Every entity should support all methods on Entity or override them to adapt their behavior to make sense. Entity::toUrl() being no exception to that rule. So in my book it's up to Commerce to either:

  • Have a canonical link template for commerce_promotion entities
  • Declare a uri_callback to handle this

That said, some quick googling has shown that some core maintainers feel not every entity should have a canonical URL. A statement I myself agree with. So I'd rather get some feedback from core maintainers on the subject and see a fix go into core (in Entity::toUrl() perhaps) or an advisory being issued urging maintainers to make sure the toUrl() method works on their entities.

See #2402533-83: Provide File::createFileUrl() as a replacement for the deprecated File:url() implementation for instance.

kristiaanvandeneynde’s picture

Title: Create group content crash if the target entity don't have view page('canonical'). » Promotion entities throw an exception when toUrl() is called on them
berdir’s picture

I don't understand what you would want to change/fix in core?

UndefinedLinkTemplateException is a documented exception in core that is thrown when calling that with an unsupported rel.

That's why there is a hasLinkTemplate() that should be used instead of just catching the exception.

alexpott’s picture

There are quite a few entities that have no URL and that's fine. Anything that works with generic entities needs to code with that in mind. As the interface for ::toUrl() states:

   * @throws \Drupal\Core\Entity\EntityMalformedException
   * @throws \Drupal\Core\Entity\Exception\UndefinedLinkTemplateException

Yes the doc might be better but I don't want to try and work out what the URL representation of a content moderation state content entity should be.

bojanz’s picture

Project: Commerce Core » Group
Version: 8.x-2.x-dev » 8.x-1.x-dev
Component: Promotions » Group (group)

I agree with #6 and #7.

Many Commerce entities have no collection route. Group should fall back to the edit-form route if collection is missing.

kristiaanvandeneynde’s picture

It's the signature of Entity::toUrl() that bothers and concerns me:

  public function toUrl($rel = 'canonical', array $options = []);

If it's okay for entities to not have a canonical link template, then why do we default to it?

You'd expect $entity->toUrl() to work on any entity if core is providing the defaults for you, right?

kristiaanvandeneynde’s picture

Re #6 caseylau made a good point on Slack:

Just use hasLinkTemplate() for checking is not enough, because the entity type or bundle can define their own uri_callback

Does every piece of code that wants to call $entity->toUrl() on generic entities now have to both check whether the canonical link template exists or whether the entity type has a uri callback that will handle said template? Seems like a DX nightmare, to be honest.

Re #7 I'm not arguing all entities should have a canonical URL, I'm saying core seems to indicate that they do. I also agree some content entities would be far better off without a canonical URL.

lawxen’s picture

That's why there is a hasLinkTemplate() that should be used instead of just catching the exception.

@Berdir hasLinkTemplate() is not enough, entity type itself and bundle can define their uri_callback, toUrl() has written code on these check, client user make these duplication checks maybe not appropriate.

kristiaanvandeneynde’s picture

Actually, I was adamant on the fact that I was informed a long while ago canonical links are required. I found it! From EntityTypeInterface::getLinkTemplates():

   * Every entity type should, at minimum, define "canonical", which is the
   * pattern for URIs to that entity. Even if the entity will have no HTML page
   * exposed to users it should still have a canonical URI in order to be
   * compatible with web services. Entities that will be user-editable via an
   * HTML page must also define an "edit-form" relationship.

That won't necessarily fix my use case (trying to redirect to a page), but I knew I had read somewhere that canonical link templates were required.

kristiaanvandeneynde’s picture

Project: Group » Drupal core
Version: 8.x-1.x-dev » 8.6.x-dev
Component: Group (group) » entity system

Moving this to core with the following suggestion:

  • New entity type key called default_link, to sit neatly below links
  • Default to canonical for content entities and edit-form for config entities
  • Change signature of EntityInterface::toUrl() and ::url() to default $rel to NULL instead of canonical. At the top of the function they fill in $rel by querying the entity type for the default link, but only if $rel was NULL.
  • Remove ConfigEntityBase::url() as it now no longer needs to override the parent
  • Change the documentation in EntityTypeInterface to no longer say canonical is required, but that content entities which do not specify it should set the default_link key
avpaderno’s picture

Version: 8.6.x-dev » 8.9.x-dev
avpaderno’s picture

EntityInterface::url() is deprecated; EntityInterface::toUrl() should be instead used.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

acbramley’s picture

Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

This came up in the BSI daily triage. Is this still applicable on the latest versions of core and group?

kristiaanvandeneynde’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

This was fixed in core already.

The docs now state:

   * Entities which can be viewed should define "canonical", which is the
   * pattern for URIs to that entity including REST. Entities that will be
   * user-editable via an HTML page should define an "edit-form" relationship.

And toUrl() has been changed to no longer default to 'canonical'

 // Use the canonical link template by default, or edit-form if there is not
    // a canonical one.
    if ($rel === NULL) {
      if (isset($link_templates['canonical'])) {
        $rel = 'canonical';
      }
      elseif (isset($link_templates['edit-form'])) {
        $rel = 'edit-form';
      }
      else {
        $exception_message = "Cannot generate default URL because no link template 'canonical' or 'edit-form' was found for the '{$this->getEntityTypeId()}' entity type";
      }
    }

Will dig up the issue that fixed this.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.