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?

CommentFileSizeAuthor
#121 1402860_121_reroll_36.patch3.84 KBMile23
#112 interdiff-1402860-73-112.txt740 bytesdbyers55
#112 workbench_moderation-n1402860-112.patch6.11 KBdbyers55
#82 panelizer_is-1402860-82-fix-ipe-end-js-alert.patch1.52 KBalcroito
#73 workbench_moderation-n1402860-73.patch5.56 KBDamienMcKenna
#70 interdiff-1402860-61-69.txt1.08 KBasgorobets
#69 interdiff-1402860-61-69.txt5.56 KBasgorobets
#69 workbench_moderation-1402860-69.patch5.56 KBasgorobets
#68 interdiff_patch67_and_patch68.txt4.18 KBjoseph.olstad
#68 workbench_moderation_panelizer_compatible_reroll_61_1402860-68.patch5.06 KBjoseph.olstad
#67 workbench_moderation-panelizer_revisions-1402860-67.patch4.7 KBjoseph.olstad
#61 workbench_moderation-1402860-61.patch5.49 KBDamienMcKenna
#59 workbench_moderation-panelizer_revisions-1402860-59.patch5.21 KBsaltednut
#49 workbench_moderation-panelizer_revisions-1402860-49.patch5.62 KBeugene.ilyin
#47 workbench_moderation-panelizer_revisions-1402860-47.patch5.16 KBhernani
#45 workbench_moderation-panelizer_revisions-1402860-44.patch5.5 KBgrasmash
#39 workbench_moderation-panelizer_revisions-1402860-39.patch5.12 KBsylus
#35 workbench_moderation-panelizer_revisions-1402860-35.patch5.28 KBjedihe
#36 workbench_moderation-panelizer_revisions-1402860-36.patch5.1 KBjedihe
#29 workbench_moderation-panelizer_revisions-1402860-29.patch6.65 KBgrndlvl
#28 workbench_moderation-panelizer_revisions-1402860-28.patch3.38 KBfrakke
#24 moderation-state-inconsistency.png26.02 KBacbramley
#22 workbench_moderation-panelizer_hack.patch2.12 KBXen
#14 panelizer-n1402860-14-d7_2.patch474 bytesDamienMcKenna
Support from Acquia helps fund testing for Drupal Acquia logo

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.)

13rac1’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 » 13rac1
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.

13rac1’s picture

Assigned: 13rac1 » 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
FileSize
474 bytes

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.:

    $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

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

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

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

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:

 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.

  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):

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

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

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:

// 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:

// 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.

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

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:

    $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

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:

/**
 * 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

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

#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

saltednut’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?

saltednut’s picture

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

saltednut’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.

saltednut’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.

saltednut’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']);
    }
  }
}
saltednut’s picture

Status: Needs work » Needs review
FileSize
5.21 KB

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.

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
FileSize
4.7 KB

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

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

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

asgorobets’s picture

FileSize
1.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

FileSize
5.56 KB

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

saltednut’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.

saltednut’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: Entity view CTools plugin doesn't load the correct revision 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**

alcroito’s picture

@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

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

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

$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.

seanpclark’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.

thtas’s picture

I have some custom ctools defined content types which are based on entities in the site.
These are selectable as content to add to panels pages.

If i go to the default panelizer edit config page for a content type
e.g. admin/structure/panelizer/node/event.default/content

It loads all my custom content and then applies work bench moderation to them and re-saves the content which is causing problems.

Other entites (nodes) may be loaded and hence resaved.

I think the path check is not specific enough and needs to ignore this adminstration pages

strpos($_GET['q'], 'panelizer') === FALSE

What path is this really looking for? panels/ajax/ipe/save_form/panelizer*

Anyway, i've added an extra check to makes sure this stuff doesnt happen in admin.

If this logic is sound I can tidy things up and re-roll the patch.

//An extra check to return early on admin pages
if( preg_match('/^admin\/structure\/panelizer.*/', current_path()) ) {
    return;
}

chrisgross’s picture

There is a lot going on here and I cannot tell which, if any of these patches actually solves the problem of panelizer not creating new revisions of nodes. Applying patch #82 does not seem to do the trick. Do I also need to apply a patch from https://www.drupal.org/node/2363275?

chrisgross’s picture

Has there been any movement on this? Any chance the working patch can get re-rolled for testing?

DamienMcKenna’s picture

For what it's worth, I'll be getting back into this later this month.

bcyde’s picture

Sorry I just want to make sure that I understand this issue (and see if other people are having the same issues that I am). Is it that when using workbench moderation and panels and creating a new revision, the UI is not updating after saving an update to a panel or adding panel content?

If so I believe patch #82 resolves this issue for me when saving the page once, however, if I make subsequent updates to the panels on the page the UI continues to reflect these changes but saving doesn't actually save what is in the UI. Basically I am only seeing the initial save of the page contents working but unless I reload the page any subsequent edits will not be saved even though the UI is updating correctly.

What I am seeing is:
1) (in browser tab #1) go to a page click on customize this page
2) (in browser tab #1) Click settings for a pane to edit its content
3) (in browser tab #1)Click finish when done editing the pane
4) (in browser tab #1) See that the UI is updated as expected
5) (in browser tab #1)Click save
6) (in browser tab #2) Confirm that the content is saved by opening up a new tab for the current url
7) (in browser tab #1) Attempt to make further edits to the pane I just edited by clicking customize this page
8) (in browser tab #1) Click settings for a pane to edit its content
9) (in browser tab #1)Click finish when done editing the pane
10) (in browser tab #1) See that the UI is updated as expected
11) (in browser tab #1)Click save
12) (in browser tab #2) See that the content is not updated/saved from the last updates when I open up the tab for the current url

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

We have been using the patch in #82 for 6+ months in production with no issues.

chrisgross’s picture

So it looks like I need the latest dev version of panelizer. But the #82 patch an the patch linked in #80 cannot both be applied. What is the solution here?

Also, it looks like new revisions to a published notde automatically get published and you cannot pick the moderation state from the IPE, so am I correct in stating that you must first manually edit the node to create a new draft, then edit the actual content from the "view draft" page?

Anonymous’s picture

chrisgross: moderations/revisions with Panels/FPP/Panelizer/Workbench Moderation has always been quite confusing and pretty buggy out of the box. Our site uses Workbench Moderation and this patch allows it to create Panelizer revisions properly with IPE I believe.

We have Panels, Panelizer, Fieldable Panels Panes, and Workbench Moderation installed with a variety of patches right now:
https://www.drupal.org/node/1986334 (fixed since adding to our site last year)
https://www.drupal.org/node/2477421 (closed, but it looks like it isn't an issue anymore)
https://www.drupal.org/node/1986334 (fixed since adding to our site last year)
https://www.drupal.org/node/2462331 (RTBC)
https://www.drupal.org/node/2465193 (RTBC)
https://www.drupal.org/node/1402860 (RTBC)
https://www.drupal.org/node/2485713 (RTBC)
https://www.drupal.org/node/2462453 (RTBC)
https://www.drupal.org/node/2360973 (RTBC)
https://www.drupal.org/node/2457113 (needs work)
https://www.drupal.org/node/2485837 (needs work - depends on some WM patches)

With these patches applied, it allows a panelized site with an IPE workflow to have revisions on panelized nodes nicely with Workbench Moderation. Without the patches it doesn't work well at all. My goal is to be able to have all of these modules in an unpatched state with all of it working (pipe dream maybe, most of these big D7 modules are very slow to issue releases these days).

Anonymous’s picture

I should mention I'm currently unboxing our large number of patches so I can apply updates to those modules as context :)

chrisgross’s picture

@vilepickle: wow, that's a lot of patches. definitely a lot of moving parts here. I will try applying some patches to my build and see what happens. thanks for shedding some light on the situation. we desperately need moderation on our panelized pages, so hopefully this will help. Thanks!

Anonymous’s picture

Yeah, pretty much have to use the dev version in most cases for these. I think Fieldable Panels Panes might be usable in its current release though since it looks like all of the issues with it are fixed for revisioning.

chrisgross’s picture

are you using the latest patch from https://www.drupal.org/node/2457113 ? That seems to be the only one you're not 100% confident in.

With the rest of the patches, this seems to work pretty well, other than the moderation block not appearing at the top of panelized nodes.

Anonymous’s picture

We're actually using #33 in that 2457113 issue, I may re-patch it to a later one if that isn't working on HEAD.

chrisgross’s picture

Actually, even with all of the above patches, this is still not good enough to use on a production site. The biggest problem is that revisions appear to work just fine for all users, but there is no way to allow a role to use the IPE ONLY for drafts and not for published revisions. Either the user can use the IPE everywhere or he can't, regardless of moderation state. Using the IPE on a published revision caused the published revision to change, even if that user does not have permission to edit published revisions.

There is one situation where this sort of works, which is when a user manually creates a new draft. In this case, when the user attempts to use the IPE on a published revision, those changes get made on a new draft, but you have to refresh the page to see that, and that will definitely be confusing to content editors. And of course, there is no way to force them to manually create a draft, because all it takes to be able to use the IPE and change the published revision is be able to view the published revision.

Anonymous’s picture

Interesting, that does seem like an oversight. We use the "new draft" tab on nodes for drafts and edit them with IPE, with a stricter workflow it seems like it breaks down as you illustrate.

EclipseGc’s picture

Chris,

Yes, I am well aware of that. Check the lightning install profile, specifically lightning_features module (which is a separate repo on d.o) and hit up the 7.x make file for that and make sure you have all the same patches for panels, panelizer and workbench/moderation. That should yield a workflow which prevents IPE from working on the currently published version of the node EXCEPT when the node has been created for the first time in a draft state and has no forward revisions.

Eclipse

TL:DR; Not an oversight, probably just a lack of docs on what the required patches are.

saltednut’s picture

EclipseGc’s picture

Ahh, I forgot Lightning had some special sauce for that feature.

chrisgross’s picture

@brantwynn: I'll have to give that alter hook a try. I'm already using all of the patches lighting uses, except for the last two workbench moderation patches. Those no longer apply to the latest dev version of workbench moderation.

EDIT: The alter hooks don't seem to do anything for me, but maybe that's because of the patches that I am not able to apply.

In the meantime, if you want to only allow the IPE on drafts (in order to avoid the problems I mentioned in #100) it's far more effective to just disable the IPE when viewing the published revisions.

function yourmodule_page_alter(&$page) {
  if (module_exists('workbench_moderation')) {
    $item = menu_get_item();
    if ($item['path'] == 'node/%') {
      $node = menu_get_object();
      if (workbench_moderation_node_moderated($node) && isset($node->workbench_moderation['published'])) {
        $page['page_bottom']['panels_ipe']['#access'] = FALSE;
      }
    }
  }
}

It's not ideal and I hope such a workaround won't be needed soon, but I can't think of anyway to keep lower roles from accessing the IPE on publshed revisions, so I have to force all users to manually create a new draft before they can see the IPE.

If you use workbench moderation with a basic page, users cannot directly edit the published revision, they have to use the "new draft" tab (from which they can set the revision to published if they have permission to do so). So shouldn't this behavior be the same for changes in the IPE? Since there is no 'moderation state' block on the IPE toolbar, with which you can control what types of panelizer revisions, if any, users can make, the above is the only way I figure I can do that. It takes a couple more clicks to create a new draft than I would like, but at least it works.

chrisgross’s picture

Status: Reviewed & tested by the community » Needs work
saltednut’s picture

What's the actionable task now? I see a use case described, but doesn't necessarily mean the patch does not work. Unsure how one moves forward from #105 I would think that marking an issue from RTBC to Needs Work then we'd need to describe what needs to change and what needs to actually be done here.

chrisgross’s picture

There is still inconsistency in the behavior of creating new drafts of panelized nodes. If you edit a published revision and there is no pending draft, it does not automatically create a draft, even though it does create a new revision. If you edit a published revision when there IS a pending draft, the pending draft becomes identical to the published revision. I think this might be due to fact that you can't choose a moderation state when creating a new revision in the IPE.

There are also still no permissions to prevent certain roles from being able to edit published revisions via the IPE, making permission for workbench's transitions meaningless, since access to the IPE means you can change the published revision, even if you only have permission to save drafts. I assumed that my workaround in #105 is not a sufficient solution, but if it is, let me know, and I will submit a patch, but this will hide the IPE on all published revisions. This wouldn't be a workflow rule so much as a blanket rule that new drafts are required before using the IPE, which I believe would defeat the purpose of giving the user the option create a new revision, as it would be mandatory. I could theoretically also change that access check to look at whether a user has permission to use the IPE on a published revision, but I don't know if that would work with the existing transition permissions. Am I wrong in thinking access to edit and save a node via IPE should be tied to said permissions, much like editing a non-panelized node?

whthat’s picture

Agreed @chrisgross a new draft must be created if the display is customized either by IPE or Panelizer's "Customize Display" tab. It is too easy editors to change the display and bypass moderation.

As a workaround, the magic done in lightning #103 is nice for the removal IPE on published version of the node, maybe similar magic is possible with the Panelizer Tab.

The patch from #36 along with workbench_moderation 1.3 is what I have been using for some time, by removing access to IPE for panelized nodes this patch provides us with panelizer creating new drafts for every save. While not ideal its something we must continue using with until the new draft is added to the latest patch.

Thanks all for your continued success on this, it has come a long way.
I will stay tuned

chrisgross’s picture

Revisions were working relatively well until some time recently. In the latest panels dev, I am having an issue where any time an unpublished revision is saved via the IPE, the "Customize this page/Change layout" bar never reappears and all the panels on the page are still editable. The page can no longer be saved beyond this point without a refresh. Anybody else seeing the same thing?

UPDATE: This actually happens on published revisions as well. Basically, any time a new revision is created, this happens now.

brockfanning’s picture

@chrisgross, if you were using the patch in this issue, have you tried removing it? I was using #82, and experienced the same as you. Right now I'm testing without the patch, and removing it seems to solve the problem of the missing buttons. (It once again allows endEditing() to fire.) However I'm not totally sure what #82 was doing for me (it was added before my time on this project) so I'm still testing.

dbyers55’s picture

@chrisgross, in regards to #108, I think I've found a solution. The problem was in hook_node_update. In previous versions of workbench_moderation it appeared to simply reset the state to it's default state (which in most cases was draft). Now it seems to keep it set to whatever state it is currently editing (published when published, draft when draft) which is why it worked when you manually created a new draft.

The change I've made makes it so the behavior is as follows:

editing the live (published) version of a node without creating a new revision will publish your panelized changes immediately.
editing the live (published) version of a node WHILE creating a new revision will create the new revision in it's default state (draft)

editing a draft version without creating a new revision will edit the current draft and keep it as a draft (no publishing).
editing a draft version WHILE creating a new revision will edit the create a new revision and keep it as a draft.

this means that now you have to manually publish any time you have a draft version of your node (which I believe is intended as this behavior is a little more conservative on what get's published and what doesn't).

whthat’s picture

Awesome, #112 with Workbench Moderation 7.x-1.4, Panels 7.x-3.7, Panelizer 7.x-3.4 the customize display tab is properly creating a new draft when no draft currently exist.

Unfortunately IPE is not creating a new draft when edited from published revision. So to stop unmoderated panelizer edits the workaround done in lightning #103 would be necessary to remove IPE buttons on published revisions of the node.

DamienMcKenna’s picture

@whthat: That's also how Panelizer in D8 works... we may just have to go down that road after all. Would you mind trying the patch in #2457113: Better Revision Handling inside and outside of Workbench Moderation to see if it helps with the workflow? It would also be worth testing with the Workbench Moderation 7.x-3.x branch and the #2376839: Rewrite to use the new Drafty module patch applied.

Ben Buske’s picture

Unfortunately I can't get #112 to work with Workbench Moderation 7.x-3.x. Hunk 2 and 3 won't apply. Has someone managed to get it to work and can give me some hints what I can do?

It appears to me that the "shutdown mode" is now handled by drafty, but I don't understand enough of this module and issue to fully get what is happening.

DamienMcKenna’s picture

Issue tags: +Workbench Moderation
Ben Buske’s picture

For anyone else how has this problem. We traced our error down to a ctools plugin of entity api that doesn't load a revision. Instead of a patch from this issue we used the following patch, wich solved our problem. https://www.drupal.org/node/2057751#comment-11504577

davewilly’s picture

Due to the recent security advisory, we are updating to workbench moderation 7.x-3.x.
I am no longer seeing this issue in Workbench moderation 7.x-3.x. Can anyone else confirm?

Ben Buske’s picture

Yes, we have this problem on multiple systems even after we updated to workbench moderation 7.x-3.x. As mentioned in #117 we now use a patch for entity api.

drumm’s picture

Issue tags: +affects drupal.org

We’re using #88 on Drupal.org for some reason, I’m not really sure why. It was added along with some OG-related stuff https://bitbucket.org/drupalorg-infrastructure/drupal.org/commits/ef7fdf....

Mile23’s picture

FileSize
3.84 KB

I encountered a site that needs #36, but that patch no longer applies against 7.x-3.x.

I can't pretend I understand all the intricacies here. :-)

So here's a re-roll. The difference is that it does not register a new shutdown function.

EDIT: Please don't waste time using this patch. Sorry. :-)