Comments

fubhy’s picture

StatusFileSize
new4.45 KB

Oh... That was the wrong patch :P

I am wondering whether it might be cool to have that for entity_uri() too or not.

fago’s picture

Status: Needs review » Needs work
+++ b/views/handlers/entity_views_handler_field_label.inc
@@ -0,0 +1,103 @@
+    if (!is_object($entity) && isset($entity) && $entity !== FALSE) {
+      $entity = entity_load_single($type, $entity);

That's bad. We really should make use of entity_load() to multiple load all results, what shouldn't be hard to do. I don't think we need all the complex data-selection helpers in the simple entity-label case either. Maybe, better take the node-link,node-edit,node-delete views-fields shipping with views as examples? Also, we should implement an access check and only display a link if access is given.

Oh, one more. We should check entity-info to be sure the entity-types have a label key before enabling this.

fabsor’s picture

Status: Needs work » Needs review
StatusFileSize
new3.52 KB

Here is a patch that derives from the standard views "node link" plugins, with some stuff from node link sprinkled on top to provide the "link to entity" feature. It uses the views_handler_field_entity handler as a base which loads entities for us. I also added an access check with entity_access and another check for checking that we can set an entity label in hook_views_data().

fago’s picture

Status: Needs review » Needs work

Thanks! Here a review:

+++ b/views/entity.views.inc
@@ -68,6 +68,17 @@ function entity_views_data() {
+    if (isset($info['entity keys']['label']) || isset($info['label callback'])) {

Let's better use !empty() for checking, so FALSE will be treated as "disabled".

+++ b/views/handlers/entity_views_handler_field_label.inc
@@ -0,0 +1,52 @@
+   * Add a link to entity option.
+   * @see views_handler_field::option_definition()

There should be an empty line before @see, let's follow that although it might not be the case in the existing code everywhere.

+++ b/views/handlers/entity_views_handler_field_label.inc
@@ -0,0 +1,52 @@
+    	'#title' => t("Link this field to it's entity"),

A tab made it in here.

+++ b/views/handlers/entity_views_handler_field_label.inc
@@ -0,0 +1,52 @@
+      if ($this->options['link_to_entity'] &&  $uri = entity_uri($this->entity_type, $entity)) {

We should check whether the entity type has entity_uri() implemented before allowing people to enable this feature.

fabsor’s picture

Status: Needs work » Needs review
StatusFileSize
new3.93 KB

Here is another shot.

Checking if entity_uri is supported is a bit complicated, but I guess it has to be since we can have uri callbacks on bundles as well. I'm adding that to the definition in hook_views_data() to avoid having to clutter the handler.

hydra’s picture

Looks good to me. Applyed and worked like expeced! thank you for your work!

dawehner’s picture

+++ b/views/entity.views.incundefined
@@ -68,6 +68,23 @@ function entity_views_data() {
+          'has uri' => $has_uri,

What about setting the label entity key if possible to allow click sort under certain cases?
Therefore implement the add_field method and set the "click sortable" flag.

Letharion’s picture

joachim’s picture

Status: Needs review » Needs work

This should probably also remove the special handling of the entity label key, which causes #1928940: EntityDefaultMetadataController clobbers a property that is being used for the label.

gauravjeet’s picture

Status: Needs work » Needs review
kristiaanvandeneynde’s picture

Issue summary: View changes
+++ b/views/handlers/entity_views_handler_field_label.inc
@@ -0,0 +1,57 @@
+    if (($entity = $this->get_value($values)) && entity_access('view', $this->entity_type, $entity)) {

Isn't it up to the view to allow or deny access? Running an entity_access on every row seems like a lot of bloat. Just playing the devil's advocate here, because I can see the benefit of the access check myself.

About the patch: Worked like a charm on a simple view, seemed to have issues on an aggregated view. Could anyone else test on an aggregated view to make sure it wasn't just my local copy?

On a slightly related note:
If we modify this patch a little bit, we could run Entity::getTranslation() on any translatable property, making entity properties completely i18n-supported in Views. You could still give the label special treatment afterwards.

sassafrass’s picture

I applied the patch in #5. I used it on an aggregate view and the link worked as expected.

bobodrone’s picture

I re-rolled the patch to work with latest stable version

colinstillwell’s picture

Re-rolled against 7.x-1.6.

colinstillwell’s picture

Trying again! Could not apply the patch cleanly because of the newline.

chris matthews’s picture

Status: Needs review » Reviewed & tested by the community

The 2 year old patch in #17 to entity.info, entity.views.inc and entity_views_handler_field_label.inc applied cleanly to the latest entity 7.x-1.x-dev and already has several community reviews/tests so changing the status to RTBC.