I'm using a bundle class to override the toUrl() method on a particular node type. If some condition is met (the specifics are not important) I don't want teasers and cards to render any links to the node because there's a corresponding check in the bundle class access() method to prevent the node from being accessible at its own page.
public function toUrl($rel = 'canonical', array $options = []) {
if ($rel === 'canonical') {
if (//some condition) {
return Url::fromRoute('<nolink>');
}
}
return parent::toUrl($rel, $options);
}
When I save such a node that meets the "" condition, pathauto is creating an alias for the / system path.
It doesn't cause any major problems from what I can tell. For example, this does not serve the node when I go to the front page of my site. It does however result in a few annoying things.
If the source url is <nolink> pathauto should avoid creating an alias.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | pathauto-nolink-3367043-2.patch | 699 bytes | danflanagan8 |
| Screen Shot 2023-06-15 at 9.12.56 AM.png | 9.12 KB | danflanagan8 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
danflanagan8Here's a patch. This should have tests. I'm also wondering if and should be handled like this too.
Comment #3
berdirNot against adding a check, but I'm not sure if that really works consistently and is meant to be supported? Isn't that just going to result in links with an empty href in many places like content overviews? What about things like xml sitemaps and other places that generically link nodes? And I think it won't make the detail page inaccessible, you still need an access check or something like rabbithole for it?
If you need logic for that in your templates anyway, I'd just add a hasLink() method or something and check for that and not call toUrl() then.
Comment #4
danflanagan8Hi @Berdir,
Rendering cards and teasers and views fields is actually the scenario where this is really nice. By having
toUrlreturn<nolink>, I can let field formatters handle all the linking. If I return<none>rather than<nolink>, then yes there are empty links all over the place. But this at least is the really smooth thing about<nolink>.You're absolute right about that. I forgot that there's a
access()method on the bundle class. If the condition is met, that denies access to non-editors when trying to view the node at the canonical route. I modeled that bit of code off of something in the micronode project. I'll update the IS.The
access()method on the bundle class makes this all work with simple_sitemap, at least.This behind the scenes stuff specifically prevents me from having to put logic in templates as I can rely on the field formatters to do the right thing. I'm able to use a single card template for all my node types even though most of them are "normal". There's a single check for
{% if url %}when rendering the title, but that's it.I'm not totally sure either. I've been messing around with this and finding problems here and there, but it mostly works and the fixes have all been fairly simple. I really like that the field formatters render things as linked and unlinked as I want. I wish field formatters treated an inaccessible route as , but they (mostly at least?) render links that 403. Here are a couple relevant issues:
#3246579: Entity reference label formatter may render link to inaccessible entity
#3336994: StringFormatter always displays links to entity even if the user in context does not have access
Though I guess that likely mostly boils down to how the
link_generatorservice handles things and there are probably reasons that it works the way it does.Comment #5
bvoynickAlso running in to some of these pathauto issues @danflanagan8 is reporting.
We're using this type of logic for similar reasons: it's a very centralized way to "turn off" the canonical-having nature of specific entities, of types that need to optionally have it. Using a toUrl() override plus a #pre_render callback on the link element, most things that construct links will automatically not link to these non-existent detail pages.
It's true that an access check service, or some other approach such as overriding the access() entity method, is also necessary in case someone does make their way to the canonical route anyway. Doesn't make it unworkable though, and 2-3 places are far easier to track down than every last place an item might be output: things like the Label entity reference formatter, which otherwise needs to be overridden, and all other such programmatic toUrl() calls. (For example, even in the default Content overview, nodes that implement these kinds of overrides show up without the title being linked. Can't remember if that's just from the toUrl() override or if that relies on the #pre_render, but otherwise It's pretty fantastic.)
Comment #6
danflanagan8Hi @bvoynick! It's very reassuring that someone else is trying to do this same kind of thing for all the same reasons.
I just linked a related issue in the redirect module: #3342409: redirect_form_node_form_alter calls getInternalPath on potentially unrouted url
I think with this patch, the other pathauto patch (#3357928: deleteEntityPathAll calls getInternalPath on potentially unrouted url), the redirect patch, and the core patch (#3342398: Path module calls getInternalPath without checking if the url is routed) everything is working for us.
If you find any additional bugs with the approach I'd be interested in hearing about them. I'm on Drupal Slack with the same handle. Cheers!
Comment #7
danflanagan8Comment #8
mlncn commentedThis is working! And the code is a straightforward failsafe.
I do think the mention of rabbit hole is apropos; here is our code basing whether to have the link or not (using danflanagan8's method here) on rabbit hole settings.
This may become the way of altering the (existence of) links for entities, as Drupal core is trying to get rid of uri_callback in #2667040: Deprecate uri_callback in routes for entities
(I will note that, annoyingly, menu links do not pick up on this removal of links at the entity level. These can be affected with hook_url_alter, however, which node reference and such links cannot.)
Comment #9
berdirFine, but merge request it must be.
Comment #10
divyansh.gupta commentedWorking on it !!
Comment #12
divyansh.gupta commentedI was able to reproduce this bug and the patch successfully applied and resolved the issue for me.
Please review
Comment #14
mably commentedLooks like tests are still missing.
Comment #15
mably commentedAdded kernel test for the fix
Added test coverage for the fix in
PathautoGenerator::createEntityAlias()that prevents alias creation when an entity'stoUrl()returns a<nolink>route.New test module:
pathauto_nolink_testCreated a test entity type (
PathautoNolinkTest) with a booleanno_linkfield. Whenno_linkis TRUE,toUrl('canonical')returnsUrl::fromRoute('<nolink>')— mimicking the real-world pattern described in the issue where developers overridetoUrl()to prevent canonical linking for certain entities.New test:
testNolinkEntityAliasNotCreated()Covers both code paths:
createEntityAlias()returns NULL, no alias is created for the "/" source path./nolink/visible-entity), confirming the fix doesn't break standard alias creation.Verified the test fails without the fix — without the
<nolink>check, an alias withsource: '/'gets created for the entity.Comment #16
mably commentedComment #17
mably commentedImplemented suggested fix from review.