Disabled view modes still saved during hook_entity_update

Confirmed. Tested through saves and revisions.

It is still created panelizer_entity rows for the disabled view mode and not pruning existing entries (of same revision ID, at least.)

Screenshot via phpMyAdmin. I disabled the content type after saving revision ID 2, revision ID 3 should not have the entry created.

This is the issue - correct? It keeps saving even though it shouldn't. Or should it also wipe out existing entries for that view mode if disabled? I'll give a patch a shot, but want to clarify end goal.

My debug shows following:
Panelizer loads all view modes for given entity and ignore view mode status flag.
We need load only enabled view modes.
Also this bug leads to the fact that all enabled view modes will be overridden with disabled one.
Patch for skiping disabled view modes attached.

need review

1. +++ b/plugins/entity/PanelizerEntityDefault.class.php
   @@ -1117,6 +1117,10 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
   +        if (!$this->plugin['bundles'][$bundles[$entity_id]]['view modes'][$view_mode]['status']) {
   

   This check looks weird. Are we confident about all keys are available?

2. +++ b/plugins/entity/PanelizerEntityDefault.class.php
   @@ -1117,6 +1117,10 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
   +        }        ¶
   

   trailing whitespaces

The patch in #4 solved the immediate problem, but created others. It wasn't running an isset which was throwing errors (image attached).

I've cleaned it up and added an isset check.

Ooops, that wasn't quite right either. Here we go.

+++ b/plugins/entity/PanelizerEntityDefault.class.php
@@ -1117,6 +1117,12 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+        // Skip disabled view modes.
+        $check_needed = isset($this->plugin['bundles'][$bundle]['view modes'][$view_mode]) ? TRUE : FALSE;

Stupid nitpick. But couldn't $check_needed just equal array_key_exists() instead of having to decide true/false.

Not like it really changes what's done here, just a thought :D

Easy fix, I like easy fixes. It's attached.

Regarding your question above in #2, I don't see why disabling a view mode should delete previous entries. So I'd say that this is all that's required here, but we'll see what Damien has to say.

Awesome!

As for my question in #2, I more so wasn't sure. In my opinion your patch is right approach, and adding something to go back and delete previous entries could lead to problems.

If there's issues that crop up from "But I disabled, and when I came back I had unexpected results!" then let it be at that time :) I think it's a small use case, but wanted to bring it up anyways.

One more thing

$bundle may be undefined we must use $bundles[$entity_id] instead.
Patch attached.

+++ b/plugins/entity/PanelizerEntityDefault.class.php
@@ -1117,6 +1117,12 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+        if ($check_needed && $this->plugin['bundles'][$bundles[$entity_id]]['view modes'][$view_mode]['status'] === FALSE) {
...
         $name = $this->get_default_display_name($bundles[$entity_id], $view_mode);

Slick catch. Line 1128 uses $bundles array. Hoping to have time to test this today and RTBC - instead of being that guy just making comments.

1. +++ b/plugins/entity/PanelizerEntityDefault.class.php
   @@ -1117,6 +1117,12 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
   +        $check_needed = array_key_exists($view_mode, $this->plugin['bundles'][$bundles[$entity_id]]['view modes']);
   

   This $check_needed is actually checking whether the view mode is customized through Field UI.

   Example: I enabled Full Content, Teaser, Search Index from "Manage Display." RSS isn't visible on Panelizer config, and $check_needed is false.

2. +++ b/plugins/entity/PanelizerEntityDefault.class.php
   @@ -1117,6 +1117,12 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
   +        if ($check_needed && $this->plugin['bundles'][$bundles[$entity_id]]['view modes'][$view_mode]['status'] === FALSE) {
   

   The view mode array is status, default, and choice. These, when debugging, were 0 or 1, not boolean, so nothing actually got caught here.

Also did some simple benchmarking using Devel's timers. Standard Drupal install, Core toolbar, just ran "drush en panelizer -y".

• Without any patches: 227-240ms
• Patch from #12 About same.
• Revised patch: 204-210ms