I created a new draft from a published node, edit it and save. The next screen is View draft screen.
Here i am not seeing any of the updates made to the node.
Is it a bug or am i missing something here?

Files: 
CommentFileSizeAuthor
#82 panelizer_is-1402860-82-fix-ipe-end-js-alert.patch1.52 KBPlacinta
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

stevector’s picture

Status:Active» Postponed (maintainer needs more info)

I am not able to reproduce this. I have made a published node. I make a new draft. I see the published version at node/%node and the draft with the changes I just made at node/%node/draft. Are you seeing the same published version at at both node/%node and node/%node/draft ?

dhina’s picture

Yes.I am seeing the same published version at both node/%node and node/%node/draft.
(I am using panels/panelizer)

dhina’s picture

I was looking into the code and found out that in this function,

function workbench_moderation_node_view_draft($node) {
  $current_node = workbench_moderation_node_current_load($node);
  return workbench_moderation_router_item_page_callback($current_node);
}

$current_node gets the edited draft.
But the return statement returns with the current published node.

dhina’s picture

turns out to be my config issue.
Now i can view draft node properly.
(Still having issues with panelizer creating new draft.may have to open a bug there.)

eosrei’s picture

Title:View draft not showing the node updates» Panelizer is incompatible with moderation
Project:Workbench Moderation» Panelizer
Version:7.x-1.1» 7.x-2.x-dev
Assigned:Unassigned» eosrei
Status:Postponed (maintainer needs more info)» Active

Panelizer uses the id of the published revision rather than the draft (if it exists) when Workbench Moderation is enabled. On save, the draft content is replaced by a new revision using content from the published revision. Draft content is lost as the moderator sees it.

This is the code from workbench_moderation which gets the current node revision.

function workbench_moderation_node_current_load($node) {
  // Is there a current revision?
  if (isset($node->workbench_moderation['current']->vid)) {
    // Ensure that we will return the current revision
    if ($node->vid != $node->workbench_moderation['current']->vid) {
      $node = node_load($node->nid, $node->workbench_moderation['current']->vid);
    }
  }
  return $node;
}

I'd like to determine if there is a way to make these two modules compatible.

merlinofchaos’s picture

Category:bug» feature
Status:Active» Postponed

This weakness of workbench moderation is part of why I wrote ERS which does not have this problem, or at least, has ways around this problem that did not require modifying Panelizer to look for a different revision ID.

I'm not really sure there's a clean way to make these two modules work together.

eosrei’s picture

Assigned:eosrei» Unassigned

Thanks! This is what I suspected.

Jean Gionet’s picture

I was beating my head on my desk trying to figure out WHY when I made changes to the layout that it kept on reverting to the previous layout setting.
If you are using Workbench Moderation you have to publish your draft after a layout change for the layout changes to take effect and be viewable.

DamienMcKenna’s picture

Should it be on Workbench's head to handle overriding the CTools / Panelizer operations, given it is Workbench that is changing what it means to load the display page for a given entity ID? Does Workbench work with Panels?

merlinofchaos’s picture

I don't think workbench moderation can work with panelizer. The two modules would have to cooperate at a pretty high level in order to work together.

DamienMcKenna’s picture

We're using the Revisioning module with a site and I'm poking into the problem.

What we're currently seeing with the v7.x-2.x-dev version of Panelizer is that when you save any per-object overrides, the form reloads with none of the changes visible, everything's exactly the way it was prior to the changes being saved. I've tested this both with the "Create new revision" option selected and deselected, that didn't make any difference.

After verifying that there was a problem I then checked the database. Surprisingly enough the records in panelizer_entity, panels_display and panels_pane are correctly updated, they just aren't loaded correctly on the panelizer "content" form.

DamienMcKenna’s picture

Digging into it further I've found that the panelizer_edit_content_form() form loads the current entity using the current_revision_id whereas every time Panelizer saves it creates a new revision, even if you tell it not to.

DamienMcKenna’s picture

Scrap that, was looking at the wrong installation.

DamienMcKenna’s picture

Status:Postponed» Needs review
StatusFileSize
new474 bytes
Test request sent.
[ View ]

Try this for size. I was digging through the $form object in panelizer_add_revision_info_form() and realized that the '#submit' handler was being added to the root of the $form, i.e.:

<?php
    $form
['#submit'][] = 'panelizer_add_revision_info_form_submit';
?>

However the existing form submission handler was being defined in $form['buttons']['submit']['#submit']. I changed the line above and it started working correctly!

In my limited testing this appears to resolve the problem using the Revisioning module, I'd like to get feedback on a) whether it fixes the problem with Workbench Moderation and b) whether merlinofchaos thinks it's the correct way of handling the problem, for all I know the patch could be opening up another can of worms.

danielnolde’s picture

This seems to produce some really nasty problems in some cases, mostly when having a revision that's not yet published, and using the "panelizer" tab. Basically you can lose or not see your panelizer changes just made, or ending up with inconsistent or almost empty panes. Not sure how to handle or go forward with this, but a.t.m., the only way to use panelizer on sections with consistent behaviour is to always use the IPE and always use it with the published revision being the latest one.
Does anyone of you have made the same experience?
I gather the patch in #14 is only trying to fix the problems with revisioning but not workbench_moderation?

DamienMcKenna’s picture

@danielnolde: I'd like to hear whether it resolves any issues for anyone else.

Xen’s picture

Seems to work for the simple case of editing the panel. However, changing the layout still creates a new revision. Perhaps the same fix should be applied to all four subpages?

Regarding the overall issue, the reason why I hit my head on #1804156: Panelizer always creates a new revision was related to trying to make a workaround to make panelizer and workbench_moderation play ball. I believe I have found a working solution by having workbench_moderation implement hook_entity_load, and ensure that the entity is the current revision when the path is node/*/panelizer*, which is the same technique ERS uses.

Further form_altering the panelizer forms to ensure that the checkbox for creating a new revision is checked when the current version is the published version, will hide the logic from the end user and let panelizer edit the current version, and only create a new draft when the current version is published, which I think makes sense.

This could either be in workbench_moderation, or put in an add-on module.

DamienMcKenna’s picture

@Xen: Ah, yes, I see that now. Also, each of the other forms has a different structure, a different location for listing the submit handlers.

DamienMcKenna’s picture

Status:Needs review» Active

Lets move discussion of the "make it work with revisions" to #1804156: Panelizer always creates a new revision and leave this focused on identifying what's necessary for Workbench compatibility.

Xen’s picture

Project:Panelizer» Workbench Moderation
Status:Active» Needs review

Here's a patch for workbench_moderation that implements what I mentioned above. It's dependent on #1804156: Panelizer always creates a new revision to really work, but it's testable whether panelizer gets the right revision.

DamienMcKenna’s picture

@Xen: you forgot to attach the patch :)

Xen’s picture

StatusFileSize
new2.12 KB
Test request sent.
[ View ]

What the...

Donno what happened, but d.o has seen the file before, because it adds a _0 suffix..

Oh, very well, here it is...

acbramley’s picture

Status:Needs review» Needs work

Applied patch and got this error on the panelizer page:

PDOException: SQLSTATE[42803]: Grouping error: 7 ERROR: column "r.vid" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT r.vid AS rvid, r.nid AS nid, MAX(vid) AS vid ^: SELECT r.vid AS rvid, r.nid AS nid, MAX(vid) AS vid FROM {node_revision} r WHERE (r.nid IN (:db_condition_placeholder_0)) GROUP BY nid ORDER BY r.vid DESC; Array ( [:db_condition_placeholder_0] => 37 ) in workbench_moderation_entity_load() (line 233 of .../sites/all/modules/contrib/workbench_moderation/workbench_moderation.module).
acbramley’s picture

StatusFileSize
new26.02 KB

With the fix from #1804156: Panelizer always creates a new revision when you use the Panelizer tab and untick "create a new revision", you get an inconsistent workbench state in the published revision. See screenshot.

DamienMcKenna’s picture

While working on #1798294: Can't edit non-current node revisions I've added a patch to add drupal_alter() for customizing the workbench node revisions list: #1868144: Allow the node history list to be customized

xtfer’s picture

@Xen: I'm still looking at your patch, but a couple of things jump out at me initially...

+++ b/workbench_moderation.moduleundefined
@@ -210,6 +210,56 @@ function workbench_moderation_menu_alter(&$items) {
+  static $me = FALSE;
+  if ($me || $type != 'node' || !drupal_match_path($_GET['q'], 'node/*/panelizer*')) {
+    return;
+  }

$me always resolves to FALSE in this expression. Is it required?

+++ b/workbench_moderation.moduleundefined
@@ -210,6 +210,56 @@ function workbench_moderation_menu_alter(&$items) {
+    !empty($form['revision_information']) &&

Should this be wrapped in an isset()?

frakke’s picture

I have added a patch to panelizer (7.x-2.x-dev) which fixes a rather serious bug causing node revisions to actually loose panels displays.
http://drupal.org/node/1871552#comment-6866490

frakke’s picture

StatusFileSize
new3.38 KB
Test request sent.
[ View ]

I've modified the patch from #22 from Xen to only trigger aggressive latest entity load when not in shutdown mode, since the workbench store function needs to be able to get a specific revision.

This might also fix #24 which seems similar to the issues, we had.

grndlvl’s picture

StatusFileSize
new6.65 KB
Test request sent.
[ View ]

Here is another. Except this is against 7.x-3.1 and adds support for the IPE as well.

There is still a lingering issue w/ the IPE integration for support entitycache. Entitycache will cause some unexpected results still.

Also there are a couple of todos.

alexkb’s picture

We're not using IPE, but we are using panelizer. Xen's #22 patch (mostly) worked for us, but we had to add another conditional check in hook_entity_load:

<?php
 
function workbench_moderation_entity_load(&$entities, $type) {
  static
$me = FALSE;
  if (
$me || $type != 'node' || !drupal_match_path($_GET['q'], 'node/*/panelizer*') && (!drupal_match_path($_GET['q'], 'node/*/draft'))) {
...
?>

This worked ok for some period of time, and then something started going wrong with unapproved revisions being visible before they had gone through the complete workflow cycle. We put this down to the node table getting set with the pending vid (even though workbench had it marked as unpublished). It seemed to be due to this hook_entity_load function.. suffice to say it was a ball ache.

Currently we're making do with an conditional check in the workbench_moderation_store() so that the node_save only gets called on certain conditions, i.e.

<?php
 
if (arg(2) == "edit" || $live_revision->workbench_moderation['current']->state == "published") {
   
node_save($live_revision);
  }
?>

This is a temporary fix at the moment, and w'll need to test it and figure out the real problem first, but thought I'd share what I've got for now.

tobby’s picture

I'm using grndlvl's patch in #29, and it's largely working for me. However, I do have some content types with default Panelizer layouts, and any changes made via IPE are lost if I make a new draft (and I have no published drafts). Effectively, saving the new draft resets the Panelizer did back to 0 (the default layout).

I added this to a hook_node_update() (in a separate module):

<?php
function MYMODULE_node_update($node) {
  if (!empty(
$node->old_vid)) {
   
// fetch the did from the old revision
   
$old_did = db_query("SELECT did FROM {panelizer_entity} WHERE entity_id = :nid AND revision_id = :oldvid ORDER BY revision_id DESC",
        array(
":nid" => $node->nid, ":oldvid" => $node->old_vid))
      ->
fetchField();
    if (!empty(
$old_did) && !empty($node->panelizer['page_manager']) && empty($node->panelizer['page_manager']->did)) {
     
$node->panelizer['page_manager']->did = $old_did;
    }
  }
}
?>

This will use the did of the previous node revision, preserving any changes made in IPE. So far, this even seems to work if the revision I edit is *not* the current revision.

I'm not sure exactly where this code belongs (such as rolling it as a patch for workbench_moderation.module, etc), but I wanted to share my solution for a fairly specific edge-case that I know has affected at least a few other folks.

recidive’s picture

Hello guys, check out this issue on Panelizer queue:

#2053721: Saving panelizer for a new node revision should clone the display

I've added a patch with a similar approach for fixing the panelizer/revisions issue.

recidive’s picture

Earl has indicated my patch is reasonable, and is asking for testers.

Please help, if you can, so we get some of the patch in this issue fixed directly in Panelizer so people not using Workbench Moderation, like me, benefit from this as well.

warpedgeoid’s picture

Update
Okay, so I changed the line orderBy('r.vid', 'DESC') to read orderBy('vid', 'DESC') and the error message has disappeared. Apparently, orderBy is adding new columns to the select list, which causes this error with groupBy or orderBy.

Is this the intended behavior for groupBy and orderBy, or are these fields only added with SQL Server? Perhaps this is a bug in the SQL Server DB driver?

Original Post
I'm working through the instructions from Phase2 on how to make Workbench Moderation and Panelizer play nicely together. When trying to access the Panelizer form for a node, I get the following PDO exception:

PDOException: SQLSTATE[42000]: [Microsoft][SQL Server Native Client 11.0][SQL Server]Column 'node_revision.vid' is invalid in the select list because it is not contained in either an aggregate function or the GROUP BY clause.: SELECT r.[nid] AS [nid], MAX(vid) AS expression, r.vid AS _field_0, r.nid AS _field_1 FROM {node_revision} r WHERE ( ([r].[nid] IN (:db_condition_placeholder_0)) ) GROUP BY r.nid ORDER BY r.vid DESC; Array ( [:db_condition_placeholder_0] => 45 ) in workbench_moderation_entity_load() (line 255 of D:\Website Root\<site>\sites\all\modules\workbench_moderation\workbench_moderation.module)

Here is to query that seems to be causing the issue:

    $query = db_select('node_revision', 'r');
    $query->condition('r.nid', array_keys($entities))
      ->orderBy('r.vid', 'DESC')
      ->groupBy('nid')
      ->fields('r', array('nid'))
      ->addExpression('MAX(vid)', 'vid');

    $vids = $query->execute()->fetchAllKeyed();

I assume this must be a SQL Server issue? I've applied patch #29, which adds the query in question.

jedihe’s picture

StatusFileSize
new5.28 KB
Test request sent.
[ View ]

I was able to get Panelizer (3.1) working with Workbench Moderation (1.3); started from tobby's blog post and had to dig some more to get to the current status. These are the patches/code I'm using:

I had to use patch -p1 < file.patch to apply the patches, since git apply file.patch didn't work.

As of now I've tested this:

  1. Updating panelizer from 2.0 to 3.1 got the display properly imported as "full page override" view mode.
  2. Updating Workbench Moderation from 1.0 to 1.3 doesn't seem to cause problems with moderated nodes.
  3. Enabling revisions + moderation for the node type I'm using Panelizer with allows me to create revisions properly.
  4. Moderation history for the node shows correct transitions.
  5. Creating new revisions (whether from node edit page or from panelizer content page) works correctly; display from current revision is the one used (shared if no changes, cloned into new one if there are display changes).
  6. Reverting to an old revision restores both data and display for that revision.
  7. View published page works as expected: both data and display are the ones for the published revision.
  8. View draft page works as expected: both data and display are the ones for the current revision.

Note that I haven't tested field collections yet, which seem to be incompatible with revisions.

Thanks all for the good work put into this, and a big thank you to Tobby for the blog post that helped me find this. May this be finished and included into the stable versions of the modules.

jedihe’s picture

StatusFileSize
new5.1 KB
Test request sent.
[ View ]

Patch in #35 was generated relative to Drupal's root, I'm attaching a patch relative to Workbench Moderation's directory.

I'll also update #35 to link to this new patch.

jlapp’s picture

The patch in #36 had been working well for me, but upon enabling the Entity Cache module the patch stopped functioning. I assume this is the same issue as the "Entitycache will cause some unexpected results" comment in #29.

I traced the issue down to the fact that when using Entity Cache, hook_entity_load() functions are only called when the entity is first loaded/cached. On subsequent loads from the cache, hook_entity_load() is not called, which is the expected behavior of Entity Cache as far as I can tell. Entity Cache does provide a hook, hook_entitycache_load() / hook_entitycache_TYPE_load() which lets you hook into and alter the entities that are loaded by Entity Cache (whether they are loaded from the DB or from the cache). I added an implementation of that hook which calls workbench_moderation_entity_load() to replace the loaded/cached entities with the latest draft revision just like the previous patches have done. My hook looked something like:

<?php
// This doesn't work with Panelizer.
function MY_MODULE_entitycache_node_load($entities) {
 
$draft_entities = $entities;
 
workbench_moderation_entity_load($draft_entities, 'node');
  foreach (
$entities as $id => $entity) {
   
$entities[$id] = $draft_entities[$id];
  }
}
?>

I verified that at the end of my function the draft entities were replacing the published entities, however Panelizer was still only seeing the published entity. I found that if instead of replacing the entire entity object, if I merely removed all of the existing properties and added all of the properties from the draft entity to the entity object loaded by Entity Cache, Panelizer would correctly see the draft entity. So my working (but ugly) code looks like:

<?php
// This works with Panelizer, but seems like a hack.
function MY_MODULE_entitycache_node_load($entities) {
 
$draft_entities = $entities;
 
workbench_moderation_entity_load($draft_entities, 'node');
  foreach (
$entities as $id => $entity) {
    foreach (
get_object_vars($entity) as $name => $value) {
      unset(
$entity->$name);
    }
    foreach (
get_object_vars($draft_entities[$id]) as $name => $value) {
     
$entity->$name = $value;
    }
  }
}
?>

I'm not sure why replacing the entire object would not work. I dug into the two places I thought would be most likely to cause the issue - Entity Cache and Panelizer's own caching and did not find anything, although Panelizer's code can be quite complex. I'm thinking it may be because the Entity Cache hook does not pass the $entities array as a reference so replacing an entity object actually modifies a copy of the $entities array, but acting on the entity objects themselves modifies the objects which are stored by reference.

I don't want to update the patches in this thread to include this hook until this object reference/property issue can be resolved cleanly (or at least explain the need for the hacky property replacement code). Has anyone else seen this incompatibility between the patches in this thread and Entity Cache, and if so, any idea what might be causing this odd behavior with Entity Cache's hook and Panelizer?

loopduplicate’s picture

"explain the need for the hacky property replacement code"

Not sure if this is what you mean but if you pass a function an array of objects, then update any of the objects in the function, when the function is done, the objects remain altered. If the function modifies the array by adding to it or something, then after the function completes, the array stays exactly the same (the same keys point to the same objects) except that any objects in the array that were deleted will be gone.

If you delete an object in the function, the reference is broken - there is nothing to reference. There is nothing you can do at that point to bring back the reference, as far as I know.

With that said, have you tried the following simplified version instead? No idea if this is a good idea or if it will even work, just figured I'd throw my two cents in.

<?php
function MY_MODULE_entitycache_node_load($entities) {
 
workbench_moderation_entity_load($entities, 'node');
}
?>

Cheers and sorry if I missed the point or got something wrong,
Jeff

sylus’s picture

Issue summary:View changes
StatusFileSize
new5.12 KB
Test request sent.
[ View ]

I get a PDO Exception: Grouping error: 7 ERROR: column "r.vid" must appear in the GROUP BY clause or be used in an aggregate function with the above patch.

According to Writing compliant code with both MySQL and PostgreSQL documentation, more specifically at PostgreSQLism: all non-aggregated fields to be present in the GROUP, all distinct values ('nid' and 'vid') should be present at GROUP BY clause.

I've attached a patch for this issue. Otherwise above patch works great in PostgreSQL!

rob_johnston’s picture

This still has a problem with MS SQL Server, as mentioned in #34, but now the error is:

PDOException: SQLSTATE[42000]: [Microsoft][SQL Server Native Client 11.0][SQL Server]Cannot use an aggregate or a subquery in an expression used for the group by list of a GROUP BY clause.: SELECT r.[nid] AS [nid], MAX(vid) AS vid, r.vid AS _field_0 FROM {node_revision} r WHERE ( ([r].[nid] IN (:db_condition_placeholder_0)) ) GROUP BY r.[nid], MAX(vid) ORDER BY r.vid DESC; Array ( [:db_condition_placeholder_0] => 7107 ) in workbench_moderation_entity_load() (line 256 of C:\Users\r_johnston\Projects\DRUPAL\Drupal_WxT\profiles\wetkit\modules\contrib\workbench_moderation\workbench_moderation.module).

Basically, it can't group my MAX(vid). Also, one can see that it is producing 3 fields and the subsequent call to fetchAllKeyed() suggests that only 2 are necessary. Can the query be simplified to something like this:

<?php
    $query
= db_select('node_revision', 'r');
   
$query->condition('r.nid', array_keys($entities))
      ->
orderBy('r.vid', 'DESC')
      ->
groupBy('nid')
      ->
groupBy('vid')
      ->
fields('r', array('nid'))
      ->
fields('r', array('vid'));
?>

which would produce the following SQL:

SELECT r.[nid] AS [nid], r.[vid] AS [vid] FROM {node_revision} r WHERE ( ([r].[nid] IN (:db_condition_placeholder_0)) ) GROUP BY nid, vid ORDER BY r.vid DESC  ?

I think that the end result is the same, no? And it would work for all databases.

fullerja’s picture

Patch in #36 worked for me.

russ86’s picture

Problem with #36, it seems to be working, however I do get this error on all of my panelized nodes when I go to edit the content:

    Notice: Undefined index: page_manager in workbench_moderation_form_alter() (line 289 of /public_html/sites/all/modules/workbench_moderation/workbench_moderation.module).
    Notice: Trying to get property of non-object in workbench_moderation_form_alter() (line 289 of /public_html/sites/all/modules/workbench_moderation/workbench_moderation.module).

NOTE: I also realized, that I just updated CTools to the lastest version (security update). I'm not aware if this error was present before that, but perhaps this could help in resolving.

jojonaloha’s picture

I'm using almost the same patches described in #35, except I'm using #15 from #2053721: Saving panelizer for a new node revision should clone the display, simply because #23 didn't apply cleanly for me using latest stable release of Panelizer.

I'm not using #39 in this issue because I noticed when I have an existing node setup in Panelizer, and I click on the "Panelizer" tab, after applying that patch it says "Not panelized", even though I see that it is panelized when I view the node. If I click the "panelize" link and then the "Panelize It!" button, it overwrites my existing panel.

grasmash’s picture

Status:Needs work» Needs review

@russ86 This updated patch addresses the issue that you described.
@jojonaloha The patch is rolled against 1.x-dev, not the latest stable release.
@rob_johnston I looked into your issue, but I'm not sure what scenario calls that snippet of code. Can you list steps to reproduce?

grasmash’s picture

StatusFileSize
new5.5 KB
Test request sent.
[ View ]

Patch did not attach in previous post.

grasmash’s picture

One problem that still exists with this patch is that permits admins to edit the current, published revision and completely bypass the workbench moderation workflow. The Panels IPE buttons should not exist on the published revision--only on the current draft.

I created a pretty hacky workaround for this:

<?php
/**
 * Implements hook_page_alter().
 */
function mymodule_page_alter(&$page) {
 
// This disables Panels IPE if we are viewing the current, published
  // revision of a node.
 
if (module_exists('workbench_moderation')) {
   
$menu_item = menu_get_item();
    if (
$menu_item['page_callback'] != 'workbench_moderation_node_view_draft') {
     
$menu_object = menu_get_object();
      if (!empty(
$menu_object->vid) && !empty($menu_object->workbench_moderation)) {
       
$node = $menu_object;
        if (!empty(
$node->workbench_moderation['published']->vid)
          &&
$node->workbench_moderation['published']->vid == $node->vid) {
         
$buttons = & drupal_static('panels_ipe_toolbar_buttons', array());
         
$buttons = array();
        }
      }
    }
  }
}
?>

Oddly, usage of hook_page_preprocess() was not effective for this--I had to wipe out the IPE buttons in the static cache.

The better solution would be to allow IPE buttons on published node pages, but to create a new draft when the IPE edit is made to it.

hernani’s picture

StatusFileSize
new5.16 KB
Test request sent.
[ View ]

This patch is similar to #36, applies to workbench_moderation 1.2 but solves the issue mentioned in #47 the same way as last patch posted in the issue does.

Bram Tassyns’s picture

I tried this patch (47) on the latest released version (1.3) and noticed that the following case still fails:
- create panelized, published node
- create a new draft through the 'New draft' tab
- change the panel through IPE in the 'View draft' tab
=> panel is changed both for the published and draft revision

I was also wondering why this fix is applied to workbench_moderation and not the IPE module?
At first glance detecting which node revision was the referrer and saving to the correct revision
(and ensuring that if the panel information is shared among revisions that those revisions aren't modified)
seems like a job for that module...

eugene.ilyin’s picture

StatusFileSize
new5.62 KB
Test request sent.
[ View ]

#47 works for me, but I got problem when I tried to edit node from the tab panelizer (not IPE). I modified patch for solve this problem.

eugene.ilyin’s picture

eugene.ilyin’s picture

Sorry seems I made mistake. Don't use patch #49

brantwynn’s picture

Status:Needs review» Needs work

I have some success with the patch from #45 but there is one issue.

We have a content type that uses Panelizer by default to arrange the fields. If one creates a new node then a new row is added to the panelizer_entity table pointing to both the revision and the nid with the display ID is set to 0. When I create a new draft, the display ID is set to 0 and that's fine. However, once I make changes to the draft using the IPE and save then both the original published node and the draft get updated to use a new matching DID.

The workaround seems to be possible by tricking the system into adding a new panels_display row for this published revision, even though its just using the default.

Workflow to get everything working ok:

  1. Create the panelized node (note there is no record for it in the panels_display table)
  2. Click 'Customize this page' in the IPE, do not make changes but then click 'Save' (note a new record is added to the panels_display table)
  3. Now click 'New draft' and save your draft. (no new record in panels_display yet but thats ok...)
  4. Make some changes in the draft using the IPE, then save (hooray! a new record in the panels_display table and everything is all good)

Example workflow to recreate the bug

  1. Create the panelized node
  2. Click 'New draft' and save your draft.
  3. Make some changes in the draft using the IPE.

In this scenario, you'll see that both records for that NID (the published revision and the draft) are showing up in the panelizer_entity table. They are both assigned the same new DID that was added to the panels_display table. This should not be - the original should remain set to 0 and only the new draft should be set to use the new DID.

DamienMcKenna’s picture

Version:7.x-2.x-dev» 7.x-1.x-dev

Would anyone be willing to get this working on v1 first, which is currently out in the field, then do v2 later?

brantwynn’s picture

Yeah, the current patch that I've tested (#45) applies to 1.x-dev - I think that Version field assignment was incorrect.

brantwynn’s picture

I think the behavior reported in #52 is a result of how the DID is being set in panelizer/plugins/entity/PanelizerEntityDefault.class.php (around line 1200) -- a lot of handsy stuff seems to be going on to ensure that the DID is set correctly to the new display, but there's no logic that looks to ensure that the current draft is the only one being updated.

When I did a dump checking for when drupal_write_record is called during that method, I noticed it gets called multiple times. For example, on a node with just one published revision and a draft revision (so 2 revisions total) the function was called 7 times. The first time it is called right after 'Customize this Page' is clicked. The subsequent times happened after clicking 'Save' in the IPE. Of those 7 times, the panelizer_entity record that was being updated was the node:bundle:default 3 times, then the new revision (it has no $panelizer->name) and then node:bundle:default 3 more times.

Unsure if this is something to do with WBM, this patch or just Panelizer craziness, but it stands to reason that this function should only be called for the revision in question and nothing else.

brantwynn’s picture

The 'Revert to [Node Bundle] default' in Panels IPE for Panelizer also just flushes the entity_panelizer table for that particular NID rather than reverting the current revision - this is likely not the desired behavior.

DamienMcKenna’s picture

@brantwynn: I've added #2311453: Reverting a display in IPE doesn't match non-IPE behaviour to fix that bug in the Panels IPE integration.

brantwynn’s picture

I came up with a very gross workaround for the problem described in #52. Use with caution.

/**
* Implements hook_ctools_render_alter().
* @todo: Remove this abomination of a hack after resolving
*        http://www.drupal.org/node/1402860
*/
function [mymodule]_ctools_render_alter(&$info, &$page, &$context) {
  $data = array_values($context['contexts']);
  $entity = $data[0]->data;
  if (isset($entity->nid) && isset($entity->panelizer)) {
    $default_name = 'node:' . $entity->type . ':default';
    $panelizer_entity = $entity->panelizer['page_manager'];
    if ($panelizer_entity->name == $default_name && isset($panelizer_entity->did)) {
      $query = db_query("UPDATE {panelizer_entity} SET did = :def WHERE entity_id = :nid AND revision_id = :vid",
        array(":def" => "0", ":nid" => $entity->nid, ":vid" => $entity->vid));
      drupal_goto($_GET['q']);
    }
  }
}
brantwynn’s picture

Status:Needs work» Needs review
StatusFileSize
new5.21 KB
PASSED: [[SimpleTest]]: [MySQL] 137 pass(es).
[ View ]

One minor change to #45 here:

On the first time around, e.g. the first revision, the form_alter that looks for form_state['entity']->panelizer['page_manager']->entity_type will come up empty. It seems to be a nuance with Panelizer that you'll instead see form_state['entity']->panelizer['page_manager']->panelizer_type for this instead.

Strangely, once this form has been saved, the panelizer_type property ceases to exist and is replaced with entity_type.

We may need to be checking for both of these properties. At the very least, we should check of the property exists before we attempt to move forward. This patch does the latter, assuming whatever defaults are in place should remain while panelizer_type is set.

skek’s picture

Status:Needs review» Needs work

The path handling for IPE doesn't support base path. If the Drupal installation has base path different from "/" e.g. "/drupal", it will not work. The base path should be stripped from $path variable.
I will provide patch soon but also looking the case when I'm on /node/ID/draft path, because it seems the patch doesn't handle the vid in this case correctly.

DamienMcKenna’s picture

Status:Needs work» Needs review
StatusFileSize
new5.49 KB
PASSED: [[SimpleTest]]: [MySQL] 137 pass(es).
[ View ]

Does this solve the path problems?

skek’s picture

Hi @DamienMcKenna,

I need to check it. However this patch will not solve the problem with the other issue I've described.
For example, when I'm on /node/ID/draft, this code will always get the current vid instead of the vid of the draft.
We need addtional elseif cause where we handle also this case.
Thinking deeply into this issue, to me the more logical case is to change the IPE to handle the revisions, instead of this patch. For example, send the vid in the ajax URL and load the correct node. We can make an url alter in the IPE so modules like workbench changes the vid if needed.
However I'm currently shooting into dark because I've recently start digging into this but on first look this patch looks very ugly but maybe it is the only way of handling this. I will be more convenient soon, after get better understanding of workbench module in combination with panelizer and IPE.

BR

colan’s picture

Status:Needs review» Needs work

It sounds like there's still more to do than #61 provides, but even if not, it needs a re-roll.

DamienMcKenna’s picture

FYI we've been working on a lot of updates on the Panelizer side too. I'll try to do a reroll next week, if someone doesn't beat me to it first.

skek’s picture

I also spend some time digging on the panelizer side only and it seems it will be possible to handle the versioning only on panelizer side. The approach I checked is to send also the content version ind into edit link and changing the entity load after that to use the version id also.

geerlingguy’s picture

I seem to have run into this issue today, and will report back with any additional findings in my testing. The latest beta1 release (not published on the main module page) seems to be causing me other errors (alongside Panels 7.x-3.5), but I don't have time to test the latest dev version right now.

joseph.olstad’s picture

Status:Needs work» Needs review
StatusFileSize
new4.7 KB
PASSED: [[SimpleTest]]: [MySQL] 186 pass(es).
[ View ]

Here's a reroll of patch 39 which we've been successfully using since comment #39 was posted.

Tested on MySQL and PostgreSQL. (Rob_Johnston commented about MSSQL, sorry I don't have an MSSQL environment to test with)

joseph.olstad’s picture

StatusFileSize
new5.06 KB
PASSED: [[SimpleTest]]: [MySQL] 186 pass(es).
[ View ]
new4.18 KB

Here's a reroll of #61 with an interdiff comparing 67 to 68

patches 67 and 68 from 7.x-1.4 (commit 5c2769ac58b4fc2) rather than the latest dev which at the time is one commit ahead.

asgorobets’s picture

StatusFileSize
new5.56 KB
PASSED: [[SimpleTest]]: [MySQL] 186 pass(es).
[ View ]
new5.56 KB

Here is a reroll of #61 against current dev and a fix for #61 for the actual path parsing.

asgorobets’s picture

StatusFileSize
new1.08 KB

Sorry, wrong interdiff attached, here it is

DamienMcKenna’s picture

What versions of Panelizer and CTools are people using to run into this problem? I'm not currently seeing this behavior when using WM 7.x-1.4+1-dev, CTools 1.6 and Panelizer 7.x-3.2-beta1+11-dev.

DamienMcKenna’s picture

Forgot to mention it - CTools 1.6 includes #1820882: Make node revisions use the node_view display which will allow the system to display the current revision using the configured Page Manager-controlled display, as appropriate.

DamienMcKenna’s picture

StatusFileSize
new5.56 KB
PASSED: [[SimpleTest]]: [MySQL] 186 pass(es).
[ View ]

This updates the spelling of "Panelizer" in comments :) No interdiff because I changed four "p"s ;-)

brantwynn’s picture

@damienmckenna Some folks may be using the panelizer version from Lightning. I believe that commit was c8fb90b
--http://cgit.drupalcode.org/lightning_features/tree/lightning_features.ma...

I believe the version is specific to a) The last time I touched Panelizer for Lightning was Sept or August when you and I were working together and b) We had built some sites on that version and attempts to update to the latest dev HEAD of Panelizer caused regressions.

DamienMcKenna’s picture

Status:Needs review» Postponed (maintainer needs more info)

@brantwynn: In that case I'm changing this to be "postponed" until someone can show a scenario where the current dev releases of the modules involved still have documented problems.

brantwynn’s picture

Status:Postponed (maintainer needs more info)» Active

@DamienMcKenna - we are closer to this working. I have just tested against the latest HEAD of both Workbench Moderation and Panelizer without any patches applied.

The results are promising but we're not 100% there. I see two issues with Workbench Moderation DEV HEAD and Panelizer DEV HEAD.

#1. Creating a new draft of a panelized node works. I can add panes to the page. I can click "Save as Custom" and it works. However, when I try to leave the page or refresh I see a javascript alert warning about there being unsaved changes. I can safefly dismiss the alert and the page changes persist. I can then publish the draft and everything looks fine.

#2. The second issues comes after publishing the first draft. I can create a new draft and I see all the panes in place. However, once I try to edit using the IPE, for example, remove a pane. Removing a pane causes the entire layout of panes to be emptied. I am left with a blank slate and I would be forced to completely rebuild the page from scratch again.

So it looks like a patch still may need to be written. However, I cannot determine at this time what is causing either of these errors.

joelpittet’s picture

@brantwynn FYI, I've needed this entity patch to get panels to work with workbench_moderation. #2020325-1: node_view pages not showing draft node in node/%node/draft page. The context entity wasn't passed along and it didn't know what state the node was in. Just in case you run into anything like that.

DamienMcKenna’s picture

@brantwynn: Could you please test if #2363275: IPE needs to allow for more nuanced save commands helps with the first issue?

DamienMcKenna’s picture

@brantwynn: The problem with getting a JS error after saving is that IPE should do a full page reload, but I don't believe there's a way of doing this right now in IPE; the patch referenced above adds a drupal_alter() to the commands list so that Panelizer can jump in and change the destination (the Panelizer side of this is already in 3.2-beta1). If there's another way of handling this I'm all ears.

As for the second issue, could you provide more clear steps on triggering this issue? I'm happy to work on it but haven't been able to replicate it yet. Hit me up on IRC tomorrow?

DamienMcKenna’s picture

joseph.olstad’s picture

Looks like several test requests still have not been processed.

**EDIT**
@brantwynn and @joelpittet , joelpittet is correct, we're also requiring the same patch to entity that joelpittet is using (patch 24 of #2020325) in addition to patch 39 from this issue for an older version of workbench_moderation (7.x-1.3+6-dev) along with a dozen other patches. I am following up with this issue because at some point we will likely want to use a recent release of workbench_moderation that has rolled up most of the patches we're currently using and that is why I was checking patch 67 and 68.
**EDIT**

Placinta’s picture

StatusFileSize
new1.52 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

@brantwynn @DamienMcKenna Regarding issue #1 in #76, the patch in #2363275 does not help.

So the symptom is the javascript alert that says "changes will be lost" after clicking save button with "create new revision" checked.

I've found the cause, and a quick-fix but I'm not sure if it's the right one.
In panels_ipe.js there is the following code, which gets executed from an AJAX response, when clicking save button.

Drupal.ajax.prototype.commands.endIPE = function(ajax, data, status) {
    if (Drupal.PanelsIPE.editors[data.key]) {
      Drupal.PanelsIPE.editors[data.key].endEditing();
    }
  };

The endEditing() method is not being called because the data.key is not found in the editors property.
The cause is that the editor property contains an object (e.g.) "panelizer-node-4961-default-5591" whereas the response key (data.key) is "panelizer-node-4961-default-5592".
So the response key contains the new revision id, whereas the editor has an old key, thus endEditing() is not executed, the editing flag is still true, and when beforeUnload event is called, it shows the javascript warning dialog.

The data.key is being returned in panels_renderer_ipe::ajax_save_form

<?php
 $this
->commands[] = array(
     
'command' => 'endIPE',
     
'key' => $this->clean_key,
    );
?>

This clean key is already the updated one. It gets updated in

<?php
 
$this
->commands[] = ajax_command_replace("#panels-ipe-display-{$this->clean_key}", panels_render_display($this->display, $this))
?>

when panels_render_display() is called, which modifies $this->clean_key.

My solution is simple, save the old key before it is lost, and return it for the endIPE command.
Attaching patch for the described issue.

Sean P. Clark’s picture

Tested #82 and it appears to address the issue I was experiencing. With this patch, the ipe now works when revisions are enabled.

EclipseGc’s picture

Project:Workbench Moderation» Panels
Version:7.x-1.x-dev» 7.x-3.x-dev
Category:Feature request» Bug report
Status:Active» Needs review

This is clearly a panels ipe bug. Panelizer and others (CPS) are beginning to change the cache_key out from under IPE and this can cause all sorts of havoc. This patch appears to be working great from my testing and is clearly a panels issue and not a workbench_moderation issue as I can reproduce this with native panelizer with its new revisioning features.

NR because we should start moving to get this committed.

Eclipse

gremy’s picture

Status:Needs review» Needs work

@damienmckenna I have applied the workbench_moderation-n1402860-73.patch and it does not solve the problem of displaying the information from the latest revision in the "View Draft" tab. The same problem is with viewing a specific revision.

I am using the latest version of panopoly, which is using panelizer 7.x-3.1.

Also I think the scope of the issue digressed from the original problem and the most recent patch does not treat displaying of revision information in the "View Draft" tab. The IPE discussion should be moved to a more specific issue.

DamienMcKenna’s picture

Status:Needs work» Needs review

The patch in #82 is the one to use, not older ones.