Add entity_url() and entity_l() wrapper functions to simplify using EntityInterface::uri()

+1 for those helpers. Those would be nice methods as well, however I fear to clutter the EntityInterface with lots of unrelated utility methods if we continue like that. That said, let's start doing more helpers/other systems that just take an entity as parameter.

Here is a patch. I keep the $text argument name in l(). I was wondering about argument order (in l(), the first argument is $text), however, I think here as wrapper functions, the first argument should always be $entity.

Addressed #5.

We may want to leave unexpected results behave "better" instead of failing when they could still provide usable results. That would be a design choice, though (typical PHP vs "fail early"). Otherwise good for me.

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+ * Returns the url of an entity.

As in all occurrences in this patch, URL is normally uppercase in text.

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+  }

This test does not cover the case of uri() returning some non-NULL value (although it should). Although the default implementation in Entity::uri() always returns array or NULL, inherited implementations could return something else, since the return value is not checked by type hinting. We might want to leave such a result untouched and return it instead of converting it to NULL.

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+    }

As for entity_url(), we probably want to use (string) $uri if it is a non-NULL scalar anyway.

Also, it's used nowhere as of now. So I guess at least we should add some basic tests for it.

- Fixed capitalization of URL (anyway, I think we should we should do the same thing for 'uri' too).
- I don't fix the other two, as described in the EntityInterface, ::uri() should return an array or NULL. Also, anything other than an array is unusable.

It was a cross post so that the tag was removed. However I don't think tests are needed. They are basically wrapper functions, and we already have tests for Entity::uri().

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+  if (is_array($uri)) {

Let's use isset() here as this is faster (microoptimizations..).

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+ *   An associative array of additional options to pass to the url() function.

It's passed to l(), so I guess it should be documented as for l().

+++ b/core/modules/entity/entity.module
@@ -413,6 +413,56 @@ function entity_uri($entity_type, $entity) {
+  if (is_array($uri)) {

Same here.

They are basically wrapper functions, and we already have tests for Entity::uri().

Yes, still we want make sure the basic wrapper works and extending the test to take care of that should be rather simple.

Fixed some errors in #10 and add tests for Entity::uri() and entity_url.

Just one more assertion for entity_l(), maybe ?

Also, to nitpick, variables pointing to user accounts are usually named $account(something) not $user(something)

- If entity_url passes, there is no way entity_l won't pass. They are pratically the same and the behavior of url() and l() are well known.
- In tests, in most cases, $userN are used instead of $account for new created accounts.

+++ b/core/modules/entity/lib/Drupal/entity/Tests/EntityApiTest.php
@@ -91,4 +91,17 @@ class EntityApiTest extends WebTestBase {
+    $this->assertTrue(is_string(entity_url($user1)), 'Entity URL has been created.');

Let's better test the right string is returned, i.e. url(...). Just having a string doesn't make a right URL.

- If entity_url passes, there is no way entity_l won't pass. They are pratically the same and the behavior of url() and l() are well known.

entity_url() doesn't invoke entity_l() so I can't follow the reasoning. Let's make sure we have complete tests for both.

Let's better test the right string is returned, i.e. url(...). Just having a string doesn't make a right URL.

As I commented in the test, url is managed by the entity controller, we don't know it. So I don't know how to write a better test, unless I copy the code in entity_url into the test, then compare the result with entity_url's, which, IMO, is not very useful.

entity_url() doesn't invoke entity_l() so I can't follow the reasoning. Let's make sure we have complete tests for both.

Both entity_url() and entity_l() use the same logic: they use EntityInterface::uri() output and send to url() and l(). The test is basically uri() test. So if one passes, the other should pass (could you give a counterexample?).

That said, I think we could add another test: assert if entity_l() result has the format of <a[.+?]>THE-GIVEN-TEXT</a>. But I'm not sure it is useful. It'd better add tests for l() and url().

As I commented in the test, url is managed by the entity controller, we don't know it. So I don't know how to write a better test, unless I copy the code in entity_url into the test, then compare the result with entity_url's, which, IMO, is not very useful.

Just make use of url() for the test, i.e. compare url('user/X') with entity_url($user), so we verify the result is as expected.

Both entity_url() and entity_l() use the same logic: they use EntityInterface::uri() output and send to url() and l(). The test is basically uri() test. So if one passes, the other should pass (could you give a counterexample?).

That holds assuming that both functions are written the same. The tests are there to verify both work independent of that assumption. So, again just add a simple assertion that makes sure l($entity->label(), url("user/1")) equals the output of entity_l().

How can we assume that entity_url($user) would return url('user/X')? Only the user module knows that, and there is already a test for that in the user module.

I know that simpletest is not unit test, but...

I leave this issue for more suggestion.

Lets enhance the test to check custom title and link options.

Looking good.

Hmm I'm not really sure between the procedural helpers vs. class methods but yeah it doesn't seem useful to have these in the main interface at least.

Doesn't have to happen here, but there must be somewhere we can use this on core already? Could we include one conversion in the patch then add a novice follow-up to do the rest?

Drupal\user\RegisterFormController::save() can profit from the new functions. For this is the only place I could find, I suggest that we fix this in this patch.

This one's looking good again.

Yep, looks great!

[edited]

The conflict was because of two @see phpdoc to url() in EntityInterface that have been moved to StorableInterface.

#31: entity-l-conversion-1637342-31.patch queued for re-testing.

I were wrong in #31, editing comment for not confusing anyone else.

This doesn't apply anymore :/
Needs rerolling again.

Rerolled again.

oops!

Rerolled #35 with the missing changes to the preprocess functions shown in #41.

Rerolled #42 due to recent entity system changes.

If this patch gets re-rolled again, can the following be added as well to clean up the docs?

+++ b/core/includes/entity.incundefined
@@ -344,6 +344,56 @@ function entity_prepare_view($entity_type, $entities) {
+ * @param $text

Can we add @param type hint here? Also explanation should start with '(optional)' as well as in the description for $options variable.

There's contrib module http://drupal.org/project/entity_path