Allow entity URI callbacks to indicate that the entity has no URI, and make the User module use that for anonymous users

Arrived here from #1392654: Commerce can return invalid return values for uri callbacks., which would also benefit from this. Actually, commerce is already returning FALSE from entity uri callbacks in a couple of places.

Please ignore the attached patch.

I opened a separate issue for D7, #1578208: Allow URI callbacks to return FALSE, and posted the patch there.

Trying to restore status to "needs review". Not sure how to delete the attachment in #5.

#4 patch sounds good for me, I experienced the same bug with commerce and I need to run a patched core in production, the #4 is simple and working.

This should be backported to D7, I think that a lot of users/developers will experience the same PHP warnings if they play a bit too much with entities.

Patch does not apply anymore as this code was moved around a lot. Rerolled.

Is there any chance for this to be rolled out to D7 please? It looks like this is stopping #1392654: Commerce can return invalid return values for uri callbacks. from getting fixed. Thank you!

Patch #4 looks good for d7, but misses documentation for the callback. For d8, the code got moved to Entity::uri() - thus needs to be fixed there.

I'd not call this a "feature" - it's more an API flaw. Re-categorizing to task.

Simple reroll of #4 for D7 to reflect includes/common.inc instead of core/modules/entity/entity.module

@fago, I'm not sure what you mean by "misses documentation for the callback", as the existing function doc block still seems to apply.

This did the trick for me as wel but the for D7 (core 7.22) and Entity API 1.0

After the upgrade of the Entity API to 1.0, I got these notices:

Notice: Undefined index: path in template_preprocess_entity() (line 1020 /sites/all/modules/contrib/entity/entity.module)
Notice: Undefined index: path in template_preprocess_entity() (line 1028 /sites/all/modules/contrib/entity/entity.module)

Then I applied this patch, and then the notices seems to be resolved.

I can confirm the #16. The patch in #15 also solved my issue.

#15 still looks OK, but you don't need the last 3 lines, explicetly return NULL is useless because it's PHP default return value when no return statement is reached. The comment states the obvious and/or should be moved into function PHP doc.

@fago: what do you suggest now that #1803586: Give all entities their own URI is in? :/ Should we still be aiming for a back-portable solution?

Note: there are nearly RTBC patches for basically the same bugs over at #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object They just need tests. Seems like one of these issues is duplicate. Convention says this one should remain since it's older and #1991464 should be the duplicate. But, that's slightly larger scope, since it's also fixing user_uri() itself. I just ran into the same problem this issue is trying to solve while I was at it.

Also, I just reopened #1803586: Give all entities their own URI pointing people here and to #1991464. At the very least, that issue needs a change notice. But, it probably needs to be re-thought -- either it should be reverted or we need a solution for not being forced to return broken URIs.

15 also worked for me. Thanks!

Let's see if we can get things moving again. Manual re-roll of #15.

These changes mean that uri() actually abides by the contract in the function's phpdoc.

If looking for the canonical URI, and it was not set in the links array or in a uri_callback function, the path is set using the default template: entity/entityType/id

#15: 1275902-15-entity_uri_callback-D7.patch queued for re-testing.

Since #1803586: Give all entities their own URI got committed in 8.x, this applies to 7.x only.

That's what I thought.

15: 1275902-15-entity_uri_callback-D7.patch queued for re-testing.

The patch for D7 works for me on a live site. I no longer get notices with Drupal Commerce.

Same here patch works for D7 using commerce kickstart2.

Just +1'ing the approach of #15. The doc block for the function even states that the function should return NULL if "the entity has no URI of its own", but in practice the code only returns NULL if the entity has no valid URI callback. Would love to see this amended, as it comes up in a wide variety of Commerce related issue queues (and certainly affects others, as discussed in #1332058: Need a way to distinguish "public/final" URIs for entities from admin and redirected ones).

#15 works for me too on a live site.
the errors notice was caused by commerce_customer_profile entities, that was coming from a migrate.

Notice: Undefined index: path in template_preprocess_entity()

The notice disappear after applying this patch.

I'm somewhat lost between all the various issues here, but:

1. Drupal 8 does this currently:

       // Invoke the callback to get the URI. If there is no callback, use the
       // default URI format.
       if (isset($uri_callback) && function_exists($uri_callback)) {
         $uri = $uri_callback($this);
       }
       // Only use these defaults for a canonical link (that is, a link to self).
       // Other relationship types are not supported by this logic.
       elseif ($rel == 'canonical') {
         $uri = array(
           'path' => 'entity/' . $this->entityType . '/' . $this->id(),
         );
       }
       else {
         return array();
       }
   
       // Pass the entity data to url() so that alter functions do not need to
       // look up this entity again.
       $uri['options']['entity'] = $this;
       return $uri;
   

   which suggests it does need the functionality in this patch just as much as Drupal 7 does (at least in the case where $rel is not "canonical"), regardless of whether any changes are eventually made in #1803586: Give all entities their own URI.

2. The patch here should update the documentation in callback_entity_info_uri() to reflect the ability to return an empty result.
3. In general, the code could use some cleanup based on the other comments above (and the patch in #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object seems farther along)...

I am not sure where to go next, but it seems premature to commit this while things are being sorted out. Probably this issue should in fact be merged with #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object, and remain on Drupal 8?

#15 works for us too, thanks a lot!

There are issues that exist for this problem in 8, and they aren't going to be forgotten if we just merge the darn change.

I've attached a patch that includes the work from #15 with fixes for the remarks from @David_Rothstein and @pounard as well as the fix for the rdf module from #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object

#15 works for me.

patch in #33 works for me, passes tests, addresses comments in #31 - setting to RTBC.

I just checked and confirmed my suspicion from #31 that this affects Drupal 8 too. So it needs to be fixed there first:

• In Drupal 8, the code is in https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!Entity.php... and I don't see how it can be used to indicate that a particular entity has no URL.
• And at least the specific issue with the RDF module definitely exists in Drupal 8. If you create a node with the anonymous user as author and then view the node, you get this in the HTML output:

  <article data-quickedit-entity-id="user/0" typeof="schema:Person" about="/user/0" class="profile">
  

I think I'm going to go ahead and close #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object as a duplicate of this issue, since there's been more recent work here and most of the work there has been incorporated into the above patch. It looks like we're still missing the part that actually changes the User module to return no URL in the case of the anonymous user, though? (That would be the actual real world test that the functionality is working as expected.)

So what this means is to document that toUrl() and similar methods can return NULL and do so for user.

In Drupal 7.56 I was getting

Notice: Undefined index: path in user_menu_breadcrumb_alter() (line 4087 of /var/www/html/xxxt/public_html/modules/user/user.module).

I applied the patch in #33 and they are gone :)
Thanks @wamilton and you all!

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Restoring tag for stats

No worries at all!