Fall back to 'edit-form' as default $rel in EntityBase::toUrl()

This is the least intrusive version possible. It still fails in the same way for entities with neither a canonical nor an edit-form link.

I think it would be slightly better to in that case throw a more helpful exception in that case and possibly we could also consider other link templates than edit-form as fallback, but I wanted to make the smallest possible change first.

As an additional improvement we could remove the override of ConfigEntityBase::toUrl() which defaults $rel to edit-form directly, but didn't do that yet, either.

This implements the additional things noted in #2.

The additional exception being thrown, while technically a behavioral change, shouldn't really affect anyone as it's the same exception class, so in effect there is not really a change, just a (slightly) more meaningful message.

The default value change in ConfigEntityBase is somewhat of a behavior change for config entities that do provide a canonical link. Those are rare, but Search API indexes, for example, provide a status overview of the index in addition to the edit-form. Search API already provides a custom delete form where the cancel link links to the canonical URL, but a similar contrib or custom module would notice a change if it were using core's default entity delete form. This is an improvement, however, as it does make sense to link to the canonical URL in that case. The only reason the override was there in the first-place was to allow the "default URL" case for config entities at all, which is still possible with this.

Removing credit from sonam_sharma for the empty MR.

Change makes sense but think we will need a change record for the new behavior please

Thanks!

Good point, added a change notice.

Also fixed some stray characters in the exception message. I thought about how to test this properly and looked into actually adding a functional test for the delete form cancel URL, but our test entity types override the cancel URL of the delete form for some reason (see \Drupal\entity_test\EntityTestDeleteForm::getCancelUrl()). We do have EntityUrlTest, however, so that seemed reasonable enough to add a small test to.

Change looks good. Think ready for committer review.

The last submitted patch, 7: 3371753-7.patch, failed testing. View results →

Seems random

In my opinion adding the 'edit-form' as a fallback to entities without canonical URL seems reasonable, but I'd love to hear what the subsystem maintainers or framework managers think about this.

The last submitted patch, 7: 3371753-7.patch, failed testing. View results →

Random failure

I'm triaging RTBC issues → .

The IS is up to date and the proposed resolution matches the change. All questions have been answered. I read the CR and made changes for readability. I have added an item to the remaining tasks to review the change record.

Other than that the only thing is review by a subsystem maintainer review or framework manager as stated in #11.

The last submitted patch, 7: 3371753-7.patch, failed testing. View results →

Looks like a random fail: 🐛 TestSiteApplicationTest::testInstallWithNonExistingFile() fails when another test creates database tables during the test run Fixed .

Untagging for subystem maintainer review since I am one. I think it makes sense to prioritise the canonical URL if one exists instead of always defaulting to edit, and for me the change record is enough for the small behaviour change.

Committed 9c4bc86 and pushed to 11.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

This caused an edge case regression 🐛 [regression] toUrl can incorrectly return edit-form url when another link template shares the canonical url Active

This also caused a fatal in aggregator module 🐛 Blocks cause WSOD; Cannot generate default URL Active

I was wondering how this could have broken BC, as I thought I/we had been careful about that, but I completely missed that Entity::toUrl() does in fact handle entities with a uri_callback and that those entities do not necessarily define any link templates.

Sorry about that! Opened 🐛 [regression] Entity::toUrl() without argument is broken for entity types with a URI callback Needs review as a (hopefully) quick follow-up to revert the unintended API change.

This issue summary recommends a fall-back to the edit form, but there are entities that don't have edit forms (files, for example). We have a site with translation that's unable to delete files with this patch. We can also reproduce the problem reliably on other sites if we attempt to delete a file without any destination query argument.

Does anyone know if there is another follow-up issue to address that? I searched but couldn't find one.

That issue appears unrelated to our problems with files. We're now working over here: https://www.drupal.org/project/drupal/issues/3424701 🐛 Cannot delete file when using language negotiation domains Active