Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Assigned: Unassigned » damiankloip

Going to look at this.

damiankloip’s picture

Assigned: damiankloip » Unassigned

Hmm, not sure we can do this, as most of what this is used for are on display instances. E.g. getting 'uses_hook_menu', which is from the plugin definition.

tim.plunkett’s picture

We could try writing it out to the file and see which is more performant.

dawehner’s picture

Just realized that using efq could be even a performance boost.

Currently it initializes ViewExecutables for all views, but with efq you simply wouldn't have to anymore.

damiankloip’s picture

The prob is that currently displays are assumed to be enabled, or 'enabled' could be set to 0 or 1. I'm not sure if we can do an efq query where menu/path IS NOT NULL *and* enabled IS NOT NULL OR <> 0 ?

damiankloip’s picture

Status: Active » Needs review
FileSize
1.42 KB

So we could do something along the lines of this? Then atleast we are filtering down the initial result set.

damiankloip’s picture

FileSize
1.46 KB

Oops.

tim.plunkett’s picture

+++ b/core/modules/views/views.moduleundefined
@@ -1107,32 +1107,24 @@ function views_get_enabled_display_extenders() {
 function views_get_applicable_views($type) {
...
+    ->condition('display.*.display_options.path', NULL, 'IS NOT NULL')

I guess we only use this for 'uses_hook_menu' now that blocks are derivatives, so this won't break, but just noting we'll have to axe the $type or do this conditionally

+++ b/core/modules/views/views.moduleundefined
@@ -1107,32 +1107,24 @@ function views_get_enabled_display_extenders() {
+  foreach (entity_get_controller('view')->load($query) as $view) {

foreach (drupal_container()->get('plugin.manager.entity')->getStorageController('view')->load($query) as $view) {

+++ b/core/modules/views/views.moduleundefined
@@ -1107,32 +1107,24 @@ function views_get_enabled_display_extenders() {
-        $result[] = array($executable, $id);
...
+        $result[] = array($executable, $id)

Missing ;

damiankloip’s picture

Issue tags: +VDC
damiankloip’s picture

FileSize
1.39 KB
1.81 KB

Thanks Tim. Generally it's a good idea if I actually check patches first ;) So we should use the $type param and change it it path for now maybe. Also changed your other points.

tim.plunkett’s picture

FileSize
1.54 KB
1.53 KB

So this assumes that if one display has a path, then all displays of that view have a path. Here's an ugly hack for that, but we'll need a better solution.

tim.plunkett’s picture

FileSize
2.29 KB
2.07 KB

Here's another option, to preserve the existing behavior and prevent an API change.

damiankloip’s picture

Yeah, I guess the only trouble with that is that disabled views would be returned too, unless you always passed in the ids..

dawehner’s picture

FileSize
2.59 KB

What about that?

dawehner’s picture

FileSize
2.04 KB

Let's edit the patch file :)

damiankloip’s picture

I like that.

damiankloip’s picture

+++ b/core/modules/views/views.moduleundefined
@@ -1107,32 +1114,34 @@ function views_get_enabled_display_extenders() {
+    ->condition('disabled', '0')

Let's just use FALSE here instead. Then we're consitent with the patch in #1901076: Use config query to get enabled and disabled views. too.

dawehner’s picture

FileSize
2.16 KB

New version.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Considering that my patches weren't used, I feel comfortable RTBCing this. (If it comes back green)

damiankloip’s picture

That looks like we're all happy with this now. Nice team work.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal-1900962-18.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.17 KB
496 bytes

DOH

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Hah, nice catch. I manually tested this time and it works. That interdiff was the only problem.

chx’s picture

->condition("display.*.display_plugin", $ids, 'IN'). WIN.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

I know catch said he was somewhat concerned about this getting abused everywhere, so I was going to assign it to him, then noticed that he created the issue in the first place. :D

Committed and pushed to 8.x. Thanks!

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