Panelizer has a lot of node moderation code built in at this point. I'm not entirely sure this is necessary and after quite a bit of digging, I've found a few errors in the existing code. My efforts here are to make more seamless integration with Workbench_moderation simple a straight forward. Long term I'd like to see the existing moderation code in panelizer put into a sub-module that can be enabled per node type, but I've not attempted to do anything along those lines yet.

Within this code are 3 small changes, the first two are populating missing revision variables during various stages of panelizer's normal workflow. Panelizer expects these to be there and they aren't for various minor logic reasons. The 3rd change is a change to entity_save() on the PanelizerEntityNode class, which now checks to see if a.) workbench_moderation is enabled and b.) if this node bundle is moderated. If it is it then loads the current live node, does a node save against the passed revision, and then does a simple drupal_write_record against the live node revision to reset which node revision is referenced in the node table. This steps around workbench_moderation's nasty double node_save process, but still results in the same sort of workflow. This appears to be a perfect workbench_moderation process because I can draft/edit/customize/revert/etc node revisions and corresponding panelizer displays w/o any problems (or extra modules).

That being said, my revision id fixes look like they may have caused issues for the existing revision support in panelizer because new node revisions don't appear to be created by changing page content/layouts/panelizer anythings. I'll dig into this issue further, but I'd still like to seriously consider putting that code path into a submodule anyway.

Eclipse

CommentFileSizeAuthor
#72 panelizer-2457113-workbench-moderation-72.patch9.3 KBmuschpusch
#69 panelizer-n2457113-69.patch9.99 KBjaperry
#68 panelizer-n2457113-68.patch9.99 KBjaperry
#67 2457113.diff9.39 KBdrumm
#64 2457113.diff9.39 KBdrumm
#59 panelizer-n2457113-59.patch9.35 KBDamienMcKenna
#59 panelizer-n2457113-59.interdiff.txt2.93 KBDamienMcKenna
#58 panelizer-n2457113-58.patch8.9 KBDamienMcKenna
#56 panelizer-n2457113-56.patch8.62 KBDamienMcKenna
#48 panelizer-better_revision-2457113-48.patch8.72 KBjaperry
#47 panelizer-better_revision-2457113-47.patch8.45 KBheddn
#47 interdiff_45-47.txt1.46 KBheddn
#45 panelizer-better_revision-2457113-45.patch8.12 KBheddn
#45 interdiff_43-45.txt854 bytesheddn
#43 interdiff_33-43.txt2.41 KBheddn
#43 panelizer-better_revision-2457113-43.patch8.04 KBheddn
#33 panelizer-n2457113-33.patch7.84 KBreescott
#32 panelizer-n2457113-32.patch8.7 KBreescott
#28 panelizer-n2457113-28.patch7.98 KBDamienMcKenna
#26 2457113-26.patch8.31 KBsaltednut
#19 panelizer-2457113-17-18-warning.png38.85 KBsaltednut
#18 2457113.interdiff.txt703 bytesEclipseGc
#18 2457113-18.patch8.37 KBEclipseGc
#17 2457113.interdiff.txt1.06 KBEclipseGc
#17 2457113-17.patch8.25 KBEclipseGc
#12 2457113-12.patch8.5 KBEclipseGc
#10 2457113.interdiff.txt721 bytesEclipseGc
#10 2457113-10.patch8.59 KBEclipseGc
#9 2457113.interdiff.txt4.32 KBEclipseGc
#8 2457113-8.patch7.89 KBEclipseGc
#3 2457113-3.patch4.05 KBEclipseGc
#1 2457113-1.patch1.88 KBEclipseGc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-1.patch1.88 KB

Here's my current patch.

Eclipse

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented

Also, for the record, I'm not thrilled with this workbench_moderation specific logic, but I've not yet come up with a better solution. I'll be looking to see if there's not a better way to tackle this as well in this issue.

Eclipse

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-3.patch4.05 KB

Ok, after sleeping on it for a bit and then looking again it became obvious that we were loading the entity revision we wanted to save during panelizer_panels_cache_save() instead of using the modified one in the $cache that tells us whether or not we should be generating a new revision. I've not checked, but if this works the way I think it works, then dev of panelizer is generating a new revision all the time and never used the log text. Both work appropriately now both inside and outside of workbench_moderation.

Vanilla behavior:

  • Without revision:
    Updates the existing revision & revision time stamp.
  • With revision:
    Generates a new revision and adds a log message.

Workbench Moderation behavior:

  • Without revision on node view:
    Updates the current revision's display.
  • With revision on node view:
    Generates new node revision in default workbench moderation state. (this has a tendency to generate multiple revision splits and make the current revision of the node very difficult to track, but it operates sanely under the circumstances, even if supporting this use case feels crazy.
  • Without revision on draft view:
    Updates the current draft revision.
  • With revision on draft view:
    Generates a new revision split from the current draft at the same state.
  • With revision on draft view (at non-default state):
    Generates a new revision at the current state (I suspect this should revert back to the default state from a "purity" perspective for workbench_moderation).

All in all, I think this is a huge move in the right direction and fixes some bugs that were extraordinarily difficult to track down. I hope others will give this code a try and give us some feedback, but my initial tests are quite promising.

Eclipse

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
Status: Needs work » Needs review 
  1. +++ b/includes/common.inc
@@ -153,14 +153,14 @@ function panelizer_settings_form_submit(&$form, &$form_state) {
-  $entity = $form_state['entity'];
+  $entity = $form_state['display']->context['panelizer']->data;

    Check values in the cache instead of wherever this entity came from. 

  2. +++ b/includes/common.inc
@@ -218,7 +218,7 @@ function panelizer_add_revision_info_form(&$form, &$form_state) {
-  $entity = $form_state['entity'];
+  $entity = $form_state['display']->context['panelizer']->data;

    Previously we updated the $form_state['entity'] but this was not passed back through the panelizer cache save process, so we couldn't check to see if a new revision should be created or not. 

  3. +++ b/panelizer.module
@@ -1183,7 +1183,7 @@ function panelizer_panels_cache_get($argument) {
-  list($entity_id, $view_mode) = explode(':', $key);
+  list($entity_id, $view_mode) = explode(':', $key, 2);

    This could lead to situations where we didn't load the appropriate cache entity because there was never a subsequent ":" in $view_mode. Limiting this fixes that issue. 

  4. +++ b/panelizer.module
@@ -1310,17 +1310,16 @@ function panelizer_panels_cache_save($argument, $cache) {
-    list($entity_id, $view_mode) = explode(':', $key);
-    $conditions = (isset($vid) ? array('vid' => $vid) : array());
-    $entities = entity_load($entity_type, array($entity_id), $conditions);
-    if ($entities[$entity_id] && $entities[$entity_id]->panelizer[$view_mode]) {
-      $entities[$entity_id]->panelizer[$view_mode]->display = $cache->display;
-      $entities[$entity_id]->panelizer[$view_mode]->display_is_modified = TRUE;
-      $handler->entity_save($entities[$entity_id]);
+    list($entity_id, $view_mode, $vid) = explode(':', $key);
+    $entity = $cache->display->context['panelizer']->data;
+    if ($entity->panelizer[$view_mode]) {
+      $entity->panelizer[$view_mode]->display = $cache->display;
+      $entity->panelizer[$view_mode]->display_is_modified = TRUE;
+      $handler->entity_save($entity);
       // The display may have been cloned in the save process, so we need
       // to be sure to put the old display back, and its contexts.
-      $cache->display = $entities[$entity_id]->panelizer[$view_mode]->display;
-      $cache->display->context = $handler->get_contexts($entities[$entity_id]->panelizer[$view_mode], $entities[$entity_id]);
+      $cache->display = $entity->panelizer[$view_mode]->display;
+      $cache->display->context = $handler->get_contexts($entity->panelizer[$view_mode], $entity);

    Instead of reloading the entity from scratch, we're pulling it out of the $cache object which is where our revision/log information resides. 

  5. +++ b/plugins/entity/PanelizerEntityNode.class.php
@@ -25,7 +25,15 @@ class PanelizerEntityNode extends PanelizerEntityDefault {
-    node_save($entity);
+    if (module_exists('workbench_moderation') && workbench_moderation_node_type_moderated($entity->type)) {
+      $live_revision = workbench_moderation_node_live_load($entity);
+      $live_revision->status = 1;
+      node_save($entity);
+      drupal_write_record('node', $live_revision, 'nid');
+    }
+    else {
+      node_save($entity);
+    }

    I'm not super thrilled with this, but at least it properly generates new revisions and doesn't require workbench_moderation's double node_save() awful.

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
Title: Workbench Moderation Revision Handling » Better Revision Handling inside and outside of Workbench Moderation
EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented

Also worth mentioning, reverting seems to work flawlessly in both workbench_moderation and vanilla situations.

Eclipse

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Parent issue: » #2155813: Plan for Panelizer 7.x-3.2 release
EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-8.patch7.89 KB

Ok, I think I'm pretty happy with this. The latest version of this patch adds a new node option for leveraging revisions during panelizer workflows so that the revision handling panelizer supports can be turned on or off as necessary. This included adding a form validation step for the form which I followed what was already in place for form submission, so should be pretty straight forward. Need to get some eyes on this patch and see how it's working for other people, we will also need an update hook to auto-enable this new node option since that's the default for how panelizer is working right now.

Behavior Profile:

Node types without revisioning:
This will only ever have one display per nid.

Node types with revisioning but no panelizer revisioning:
If a new node revision is create somehow (say via workbench_moderation or similar) we should end up with a clone display that can be manipulated per revision.

Node types with panelizer revisioning:
A new revision can be spawned for every atomic panelizer interaction (if the user doing the work wishes it so) or at just each major stage. This happens the same way it does in HEAD and is very granular in how it can be applied.

Eclipse

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113.interdiff.txt4.32 KB

Here the interdiff

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-10.patch8.59 KB
2457113.interdiff.txt721 bytes

Ok, and here's the update hook to keep it all sane.

Eclipse

Status: Needs review » Needs work

The last submitted patch, 10: 2457113-10.patch, failed testing.

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-12.patch8.5 KB

reroll

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/plugins/entity/PanelizerEntityDefault.class.php
--- a/plugins/entity/PanelizerEntityNode.class.php
+++ b/plugins/entity/PanelizerEntityNode.class.php

+++ b/plugins/entity/PanelizerEntityNode.class.php
@@ -25,7 +25,15 @@ class PanelizerEntityNode extends PanelizerEntityDefault {
-    node_save($entity);
+    if (module_exists('workbench_moderation') && workbench_moderation_node_type_moderated($entity->type)) {
+      $live_revision = workbench_moderation_node_live_load($entity);
+      $live_revision->status = 1;
+      node_save($entity);
+      drupal_write_record('node', $live_revision, 'nid');
+    }
+    else {
+      node_save($entity);
+    }

Why don't you just subclass in workbench module itself + probably some plugin alter?

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented

Last I looked there aren't plugin alters, and as best as I know, I can't do a handler per entity bundle, otherwise I'd have done EXACTLY that. :-D Open to other ideas though.

Eclipse

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

Could you please split off the smaller fixes from the grand tamale? I think you might be resolving some other issues as part of it.

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-17.patch8.25 KB
2457113.interdiff.txt1.06 KB

Which fixes? Let me know and I can either justify or split as appropriate.

This patch contains a fix for how I was saving node entities previously as my old process could lead to nodes displaying with field values from new revisions. This new code is simpler and follows workbench_moderation's expected workflow which means a new revision for every panelizer save. This is the same thing wbm does for node edits, and keeps us in line with it's expected workflow. This means new revisions (and thus new displays) every time you save the customize page or layout stuff.

I also spent some time testing without IPE turned on, and due to how workbench_moderation tracks node drafts, it's impossible to customize draft pages without IPE.

Eclipse

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
FileSize
2457113-18.patch8.37 KB
2457113.interdiff.txt703 bytes

Failed to check that we weren't attempting to edit the current live vid before creating a new one. Simple fix.

Eclipse

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut commented
Issue summary: View changes
Status: Needs review » Needs work
FileSize
panelizer-2457113-17-18-warning.png38.85 KB

As of #17 (and also in #18), when editing via IPE, I am seeing one problem where I get a warning before leaving the page. Might have to do with leaving behind an abandoned layout when a new revision for every panelizer save is created.

Everything works correctly - its just an annoyance. The warning is not actually true, I don't lose unsaved changes if I click "leave this page" -- is there something we need to update in javascript to prevent this confusion?

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

@brantwynn: Check the patch in #1402860: Panelizer is incompatible with moderation to see if it resolves that issue, if so it's a bug in Panels.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Related issues: +#1402860: Panelizer is incompatible with moderation
saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut commented

@DamienMcKenna The patch from #1402860-82: Panelizer is incompatible with moderation (which applies to Panels) does not resolve the IPE alert problem that #17 introduced.

Sorry, I had not tested correctly against the latest version of Panels. The patch does seem to work alongside #17 to get rid of the popup.

EclipseGc’s picture

EclipseGc CreditAttribution: EclipseGc at Acquia commented
Status: Needs work » Needs review
Related issues: +#2462331: IPE insufficient for Panelizer (data loss when using revisions)

Moving this back to needs review since #1402860: Panelizer is incompatible with moderation does solve it and is a separate issue from this. Adding another related issues that need to be gotten into panels.

Eclipse

brantwynn queued 18: 2457113-18.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 18: 2457113-18.patch, failed testing.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut commented
Status: Needs work » Needs review
FileSize
2457113-26.patch8.31 KB

Rerolled to fix the .install file.

gremy’s picture

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

@eclipsegc The patch from https://www.drupal.org/node/1402860 does not fix the problem. I just applied it and it does not work for me.
I am still seeing the information of the published node in the "View Draft" tab.

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

I tried upgrading panelizer to it's latest dev version, and then applying the patch from this issue, and the problem still persists.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
FileSize
panelizer-n2457113-28.patch7.98 KB

Rerolled as one minor bug was fixed in #2408691: Undefined offset in panelizer_panels_cache_get() in IPE, and "Invalid pane id" when revision is enabled.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Needs work » Needs review
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Component: Other module integration » Revisions
Status: Needs review » Needs work

I'm... not completely comfortable with changing the way Panelizer works with core's revision handling... I wonder if all of the changes will be necessary given some of the changes going on in WM? I'm going to hold off on the WM side of things but will continue to work on this from the POV of resolving revisions problems with & without IPE.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Related issues: +#2376839: Rewrite to use the new Drafty module
reescott’s picture

reescott CreditAttribution: reescott commented
FileSize
panelizer-n2457113-32.patch8.7 KB

This patch didn't seems to be applying correctly.

reescott’s picture

reescott CreditAttribution: reescott commented
FileSize
panelizer-n2457113-33.patch7.84 KB

@DamienMcKenna Rerolled #28 against latest dev version and also fixed an error on line 150 that mistakenly disabled the "Create New Revision" checkbox on content type Publishing Options tab, making it difficult to enable revisions and moderation for using Workbench Moderation.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

I'm considering pushing out 3.2 Really Soon with this as-is and then doing a quick turn-around on 3.3 that focuses exclusively on revisions. The first thing to do towards that goal is document what works and what doesn't, which will go into #2151837: Update README.txt file.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Related issues: +#2151837: Update README.txt file
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

@DamienMcKenna: I'd be all for a quick 3.2 with what's there now! Last time we ran the latest Panelizer -dev through Panopoly's automated tests it worked great.

joelstein’s picture

joelstein CreditAttribution: joelstein commented

Just a quick note about the content type form.

The patch in #33 prevents me from selecting "Create new revision". Both it and "Enable panelizer revisions" are disabled without any way to select one or the other. Note: I'm not using Workbench Moderation... just Drupal's built-in revisions.

The patch in #28 does allow me to check "Create new revision", FYI.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut commented

I believe this was the intent of @eclipsegc because of the nature of how the IPE would interact with Workbench.

The main idea is that you shouldn't be able to use the IPE unless you are already working on a draft.

Otherwise, you'd be making changes to the published layout.

It is implied that new drafts will be made going forward as long as one works on drafts (even when making changes to the layout in the IPE).

So, that is why this patch disables the functionality that it does in #33.

We do use this patch in the Lightning Features package provided for both Demo Framework and Lightning distros.

joelstein’s picture

joelstein CreditAttribution: joelstein commented

Makes sense, however since the title of the issue is to get revision handling "inside and outside of Workbench Moderation", I think the patch should accomplish both.

It won't make sense to commit something that breaks Panelizer revision handling for sites without Workbench Moderation. :)

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
CreditAttribution: saltednut commented

Yes, that's totally valid ^ - hence my disclosure! :)

It might make sense to add an if statement here where we check for WBM using module_exists() before disabling those checkboxes.

MustangGB’s picture

MustangGB CreditAttribution: MustangGB commented

Is this being pushed back to 3.3 now, so 3.2 can get out the door?

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Forum One commented 
+++ b/plugins/entity/PanelizerEntityNode.class.php
@@ -138,7 +144,28 @@ class PanelizerEntityNode extends PanelizerEntityDefault {
+      // Disable the 'revision' checkbox when the 'moderation' checkbox is checked, so that moderation
+      // can not be enabled unless revisions are enabled.
+      $form['workflow']['node_options']['revision']['#states'] = array(
+        'disabled' => array(':input[name="node_options[panelizer]"]' => array('checked' => FALSE)),
+      );
...
+      // Disable the 'moderation' checkbox when the 'revision' checkbox is not checked, so that
+      // revisions can not be turned off without also turning off moderation.
+      $form['workflow']['node_options']['#options']['panelizer'] = t('Enable panelizer revisions');
+      $form['workflow']['node_options']['panelizer']['#description'] = t('Revisions must be enabled in order to create panelizer revisions.');
+      $form['workflow']['node_options']['panelizer']['#states'] = array(
+        'disabled' => array(':input[name="node_options[revision]"]' => array('checked' => FALSE)),
+      );

re #37/#40, Based on these points, I think this should be wrapped by a module_exists.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Forum One commented
Status: Needs work » Needs review
FileSize
panelizer-better_revision-2457113-43.patch8.04 KB
interdiff_33-43.txt2.41 KB

Status: Needs review » Needs work

The last submitted patch, 43: panelizer-better_revision-2457113-43.patch, failed testing.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Forum One commented
Status: Needs work » Needs review
FileSize
interdiff_43-45.txt854 bytes
panelizer-better_revision-2457113-45.patch8.12 KB
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Forum One commented
Status: Needs review » Needs work 
  1. +++ b/plugins/entity/PanelizerEntityNode.class.php
@@ -47,7 +53,7 @@ class PanelizerEntityNode extends PanelizerEntityDefault {
     $node_options = variable_get('node_options_' . $bundle, array('status', 'promote'));

    Missing the default options value for new site installs that didn't go through an upgrade.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Forum One commented
Status: Needs work » Needs review
FileSize
interdiff_45-47.txt1.46 KB
panelizer-better_revision-2457113-47.patch8.45 KB
japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented
FileSize
panelizer-better_revision-2457113-48.patch8.72 KB

Re-rolled based on all the changes from sept 8th and 9th.

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented 
  1. +++ b/includes/common.inc
@@ -153,14 +153,14 @@ function panelizer_settings_form_submit(&$form, &$form_state) {
-  if (empty($form_state['entity'])) {
+  if (empty($form_state['display']->context['panelizer']->data)) {

    This seems like quite the substantial change. Relying on the form's display state to hold the entity data looks like a monumental change in how entity information is saved.

    If Workbench Moderation is not enabled, is revision information still maintained? Why does Panelizer modify the handling of revisions? This functionality seems way out of scope for Panelizer. 

  2. +++ b/plugins/entity/PanelizerEntityNode.class.php
@@ -25,6 +25,12 @@ class PanelizerEntityNode extends PanelizerEntityDefault {
+        $entity->revision = TRUE;

    Should this change live in Workbench Moderation rather than Panelizer?

gremy’s picture

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

I just tried it in Panopoly with the latest panelizer dev version and applied the panelizer-better_revision-2457113-48.patch patch, and if I create a new Draft revision, and press "View Draft" I still see the content of the Published revision, with the pink styling of the unpublished.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Issue tags: +Needs tests

We need some tests to lock down a) how core works, b) how IPE works and c) how WM works, and also confirm that we're working with WM with Drafty (#2376839: Rewrite to use the new Drafty module).

And yes, we'll be working on expanding Panelizer's base test suite.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

New issues for writing tests: #2579447: Write tests to cover the standard node workflows, including core revisions, excluding IPE, #2579449: Write tests to cover the standard term workflows, excluding IPE. #2579451: Write tests to cover the standard user workflows, excluding IPE, #2579453: Write tests to cover IPE usage on nodes, inc core revisions

The absolute priority is ensuring that core's workflows are not broken by other modules, and only then will we fix Workbench Moderation support.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Related issues: +#2579447: Write tests to cover the standard node workflows, including core revisions, excluding IPE

Anyone who wants to help move this forward should collaborate on #2579447: Write tests to cover the standard node workflows, including core revisions, excluding IPE first. Thanks.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Any update here? I see the linked issue is fixed now.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

@vilepickle: I'm currently working on tests for the node handling and core revisioning, once I have that tied down more I'll be able to look at WM.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
panelizer-n2457113-56.patch8.62 KB

I've added several tests recently around the editorial workflow, lets see what we can do for this.

Lets start with a reroll.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

PS anyone want to help with the new release, please help write tests for this and the other issues marked as being part of the 3.2 release. Thanks!

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
panelizer-n2457113-58.patch8.9 KB

Minor changes to comments.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
panelizer-n2457113-59.interdiff.txt2.93 KB
panelizer-n2457113-59.patch9.35 KB

A minor tweak to the logic, it only allows revisions to be disabled if Workbench Moderation is enabled.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Needs work

There's a minor regression - if the "Enable moderation of revisions" option is enabled the "node/$nid/panelizer/$view_mode/content" path no longer shows the second-level local tasks (Settings, Context, Layout, Content) though the paths still work and they still show on the Overview page.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

A bug against the new intended workflow:

  • Enable Panelizer and revision handling on Panelizer for a content type.
  • Load "node/$nid/panelizer".
  • Click one of the operations for the enabled view modes.
  • Only the "Content" page will show the revisions form, the others do not.
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

I think that, in general, a little more work needs to be done with the UX here. It's unclear why some links & pages are no longer accessible but others are, the UX doesn't make it clear what the intended workflow is and doesn't hide pages that should no longer be part of the workflow.

So this definitely needs more work.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: #2155813: Plan for Panelizer 7.x-3.2 release »

Pulling this for the 3.2 release plan, will come back to it afterwards.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm as a volunteer commented
Status: Needs work » Needs review
FileSize
2457113.diff9.39 KB

Attached is a quick reroll.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm as a volunteer commented

I don't really understand the added update function:

  foreach (node_type_get_types() as $type) {
    $options = variable_get('node_options_' . $type->type, array('status', 'promote', 'panelizer'));
    // Only set the variable if it was previously configured.
    if (!in_array('panelizer', $options)) {
      $options[] = 'panelizer';
      variable_set('node_options_' . $type->type, $options);
    }
  }

I read the code as doing the opposite of what the comment says, if panelizer isn't in the options array, it is added.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

@drumm: yes. The idea is that if the variable doesn't exist already it'll load the default, which will include the new 'panelizer' option, but if the variable was set before it'll enable the 'panelizer' option.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm as a volunteer commented
FileSize
2457113.diff9.39 KB

I see this looks okay now. I initially thought that was for panelizing the content type, but it really means “Enable Panelizer revisions”

This patch updates the comment to Only set the variable if it was*n’t* previously configured.

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry commented
FileSize
panelizer-n2457113-68.patch9.99 KB

Note, if you applied patch #65 or #67, you must go back into your system table and revert your update status to 7301 before installing Panelizer 7.x-3.3.

Here is a re-roll that works with the Security release.

japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry commented
FileSize
panelizer-n2457113-69.patch9.99 KB

oops. in the re-roll missed some syntax. trying again.

The last submitted patch, 68: panelizer-n2457113-68.patch, failed testing.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Needs work

Now that Workbench Moderation v3 is out with some rewritten internals, this needs to be updated to make sure it still works.

muschpusch’s picture

muschpusch CreditAttribution: muschpusch at Factorial GmbH commented
FileSize
panelizer-2457113-workbench-moderation-72.patch9.3 KB

Updated patch with updated panelizer_install_N functions.

cboyden’s picture

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

I've tested this patch with the following recipes, and it's working as expected. Probably needs more review though. Setting to needs review for testbot.

projects[panels][version] = 3.8
projects[panels][subdir] = contrib
projects[panels][patch][2729331] = https://www.drupal.org/files/issues/panels-region-style-2729331-2.patch
projects[panels][patch][2798831] = https://www.drupal.org/files/issues/panels-ipe-change-layout-revision-2798831-2.patch
projects[panels][patch][2841709] = https://www.drupal.org/files/issues/panels-ipe-bar-not-hiding-2841709-2.patch
projects[panels][patch][2790075] = https://www.drupal.org/files/issues/not_authorized_when-2790075-26.patch
projects[panels][patch][2479879] = https://www.drupal.org/files/issues/panels-revert_allowed_types_to_variable-2479879-37.patch

projects[panelizer][version] = 3.4
projects[panelizer][subdir] = contrib
projects[panelizer][patch][1549608] = https://www.drupal.org/files/issues/panelizer-n1549608-26.patch
projects[panelizer][patch][2189477] = https://www.drupal.org/files/issues/panelizer-change-layout-revisions-2189477-12.patch
projects[panelizer][patch][2788633] = https://www.drupal.org/files/issues/panelizer_update_7120-2788633-7.patch
projects[panelizer][patch][2788851] = https://www.drupal.org/files/issues/panelizer-administer-panelizer-2788851-2.patch
projects[panelizer][patch][2457113] = https://www.drupal.org/files/issues/panelizer-2457113-workbench-moderation-72.patch

projects[workbench_moderation][download][type] = git
projects[workbench_moderation][download][revision] = b3e615e
projects[workbench_moderation][download][branch] = 7.x-3.x
projects[workbench_moderation][subdir] = contrib
projects[workbench_moderation][patch][1285090] = https://www.drupal.org/files/issues/workbench_moderation-playnicewithpanels-40.patch
projects[workbench_moderation][patch][1512442] = https://www.drupal.org/files/issues/1512442-20-workbench_moderation-fix_access_check.patch

projects[drafty][version] = 1.0-beta3
projects[drafty][subdir] = contrib

projects[workbench_moderation_state_access][version] = 1.1
projects[workbench_moderation_state_access][subdir] = contrib
projects[workbench_moderation_state_access][patch][2467957] = https://www.drupal.org/files/issues/workbench_moderation_state_access-info_package-2467957-1.patch
cboyden’s picture

cboyden CreditAttribution: cboyden commented
Status: Needs review » Reviewed & tested by the community

We've been using this for quite a while now and it's working as expected.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #2787147: Plan for Panelizer 7.x-3.5 release

That's fantastic, thanks for the update cboyden!

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Reviewed & tested by the community » Fixed

Committed. Finally. Thank you all!

Status: Fixed » Closed (fixed)

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