Problem/Motivation

we make several base fields to use just Entity field formatters and not special purpose field handlers in views. However, we have a lot of other field handlers in views that have more functionality than the corresponding field formatters.

Proposed resolution

Replace node_type with field formatter

Remaining tasks

create patch

User interface changes

Entity fields will have more formatting options available, and they'll be consistent with what is available in Views.

API changes

None

CommentFileSizeAuthor
#18 interdiff-2454171-18.txt1.23 KBdamiankloip
#18 2454171-18.patch11.82 KBdamiankloip
#14 interdiff-2454171-14.txt1.05 KBdamiankloip
#14 2454171-14.patch11.9 KBdamiankloip
#11 interdiff-2454171-11.txt905 bytesdamiankloip
#11 2454171-11.patch11.82 KBdamiankloip
#3 2454171.patch11.73 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: +VDC
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: Unassigned » damiankloip
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: damiankloip » Unassigned
Status: Active » Needs review
FileSize
2454171.patch11.73 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community 
index a3387be..69a3478 100644
--- a/core/modules/node/config/optional/views.view.content.yml

--- a/core/modules/node/config/optional/views.view.content.yml
+++ b/core/modules/node/config/optional/views.view.content.yml

Ts, you actually exported the view, and not just hacked the config directly.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2454171.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Oops ok. Need to remove some of the yaml data. When we save handlers we should not keep old data. This seems to be what's happening?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Oops ok. Need to remove some of the yaml data. When we save handlers we should not keep old data. This seems to be what's happening?

Well the update you made is probably fine, the fails are unrelated.

Status: Needs work » Needs review

damiankloip queued 3: 2454171.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2454171.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented

Yep, definitely related :)

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
2454171-11.patch11.82 KB
interdiff-2454171-11.txt905 bytes
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: 2454171-11.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
2454171-14.patch11.9 KB
interdiff-2454171-14.txt1.05 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

We have this todo in \Drupal\views\EntityViewsData::processViewsDataForEntityReference

    if ($field_definition->getName() == $this->entityType->getKey('bundle')) {
      // @todo Use the other bundle handlers, once
      //   https://www.drupal.org/node/2322949 is in.
      $views_field['filter']['id'] = 'bundle';
    }

Do you think we can simply drop them now?

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Title: Replace node_type handler with generic views handler » Replace node_type Views handler with Field API formatter
Priority: Normal » Critical
Issue tags: +Critical Office Hours

Raising to critical, because this looks like it's in the first (necessary for access control) category of the "proposed resolution" of #2393339: [META] Make sure Views base fields are using Field API for formatting, and do not lose functionality, but please correct me if I'm mistaken.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Went through the views and these are indeed the only adjustments you have to do

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_type_filter.yml
@@ -75,9 +75,23 @@ display:
diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml

diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml
index 32b0692..d73c2ef 100644

index 32b0692..d73c2ef 100644
--- a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml

--- a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_exposed_admin_ui.yml
@@ -31,7 +31,6 @@ display:

@@ -31,7 +31,6 @@ display:
           field: type
           id: type
           table: node_field_data
-          plugin_id: node_type
           entity_type: node
           entity_field: type
         body_value:

This is a change on a filter, let's drop that change, even it might be technically the right thing to do.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
2454171-18.patch11.82 KB
interdiff-2454171-18.txt1.23 KB

Yep, agreed. Here you go.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Thank you!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 5c2ec02 and pushed to 8.0.x. Thanks!

#2462589: Provide test coverage for access checking of all views fields. will provide the necessary access tests.

  • alexpott committed 5c2ec02 on 8.0.x 
    Issue #2454171 by damiankloip: Replace node_type Views handler with...

Status: Fixed » Closed (fixed)

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