Forward revisions break publishing audit trail

+1 to be addressed by Drafty or Workbench moderation

To save this in a general approach I'd suggest do see if something like this can be implemented by drafty since it's responsible to handle the saving operation.

Here is a patch to repair the audit trail.

To get the best result, apply #2579205-15: Delete redundant revisions to Drafty and set the following variables:
* drafty_delete_old_revisions TRUE
* drafty_delete_with_cron FALSE (best for testing)

A batch will be started after every save to delete the previous published revision.

ToDo: Implement hook_drafty_queue_before_delete_revision() if #2579205: Delete redundant revisions is fixed to cleanup workbench history.

Are these test fails caused by the current patch? Do they disappear when #2579205: Delete redundant revisions is applied as well?

After using this patch for a few days, I noticed some strange things, some record were not updated or displaying correct. But that was a long time ago.

Conclusion: More testing needed.

ToDo: Implement hook_drafty_queue_before_delete_revision() if #2579205: Delete old revisions is fixed to cleanup workbench history.

On #2579205-85: Delete redundant revisions for Drafty, I added hook_drafty_predelete_revision() which could be used to implement this in potentially more stable way! I'll make a new patch that depends on that soon-ish

Here's a new patch that depends on #2579205-85: Delete redundant revisions for Drafty! When deleting revisions is enabled (it has to be turned on in a configuration form), it'll move the uid, timestamp, and all the node history from the old revision to the new revision. Additionally, it changes the sort order on the moderation tab to be by timestamp rather than vid, so the history even makes chronological sense :-)

Here's a screenshot:

IMHO, this brings the usability of that page back in line with what we had in workbench_moderation 1.x

Just wanted to chip in that I applied both patches (WBM and Drafty) without any problem and it works like a charm. Thanks for all your effort and I am looking forward to the next stable releases including these patches.

Hi, this is the exact issue I have had on 2 out of 5 similar sites, one site is brand new. I have the latest versions of drafty and workbench_moderation. I am trying the patches. Thank you!

Is there any progress towards getting this patch rolled into any upcoming releases in the near future? We have a few clients with similar issues, and would greatly appreciate it if the patch to be part of an updated release.

Thanks dsnopek !

RTBC +1 for patch #14 - - in combo with #2579205-94: Delete old revisions

RTBC +1 for patch #14, been using it for a while.

I have applied this patch, however it also appears that when using the diff module to compare revisions the /moderation/diff (Compare Revisions) page is still showing order by revision ID rather than the timestamp as this patch #14 addresses.

Taking a closer look at this:

it looks like diff uses the node_revision_list() function to create the list of revised nodes.
https://api.drupal.org/api/drupal/modules%21node%21node.module/function/...

this function has an order by ORDER BY r.vid function.

it seems like this is called several times in diff as all the calculations are based on vid and not timestamp.

I found this related issue with the Diff module:
https://www.drupal.org/project/diff/issues/2881771

I was able to resolve the issue with Workbench Moderation revisions and Diff comparisons.
First I applied this patch and the diff patch found here:
https://www.drupal.org/project/diff/issues/2881771

After applying the patch, when comparing revisions, the comparison was comparing NEW to OLD instead of the expected OLD to NEW

the issue appears to be related to the following workbench moderation function:
function workbench_moderation_diff_node_revisions_submit($form, &$form_state) {

I was able to re-write this function as follows: (based off the diff patch referenced above)

function workbench_moderation_diff_node_revisions_submit($form, &$form_state) {

  // the ids are ordered so the old revision is always on the left
  $revisions = $form_state['revisions'];
  if ($revisions[$form_state['values']['old']]->timestamp > $revisions[$form_state['values']['new']]->timestamp) {
    $old_vid = $form_state['values']['new'];
    $new_vid = $form_state['values']['old'];
  }
  else {
    $old_vid = $form_state['values']['old'];
    $new_vid = $form_state['values']['new'];
  }
  $form_state['redirect'] =  'node/'. $form_state['values']['nid'] .'/moderation/diff/view/'. $old_vid .'/'. $new_vid;
}

I found another issue in this patch. When you hit the revision limit on a page, there is a function in diff.pages.inc that splits the revision list based on the 'REVISION_LIST_SIZE' parameter (set in diff.module line 11)
When the revision list gets split, the revision object identifier remains in a 0-49 sequential order. The action in 'diff_node_revision_list()' adds the vid as the revision identifier but the pager action removes this association.

My solution:
Update diff.pages.inc
Line 56 (add)

	foreach ($revisions as $revision) {
        $revisions[$revision->vid] = $revision;
    }	

This goes back to the chunk and adds the associated vid so that the previous 'workbench_moderation_diff_node_revisions_submit()' updates will correctly order the revisions by timestamp.

I'll get a patch out for this shortly.

I have been successful with using this patch along with the patch I created for diff to sort the revision history:
https://www.drupal.org/project/diff/issues/2881771

diff-sort-revision-timestamp-2881771-15.patch

The patch seems to be working as expected when applied along with:
workbench_moderation-preserve-history-2824798-14.patch

however I had to make an additional update to the #14 patch:

Update Line: 161-174:

function workbench_moderation_diff_node_revisions_submit($form, &$form_state) {

  // the ids are ordered so the old revision is always on the left
  $revisions = $form_state['revisions'];
  if ($revisions[$form_state['values']['old']]->timestamp > $revisions[$form_state['values']['new']]->timestamp) {
    $old_vid = $form_state['values']['new'];
    $new_vid = $form_state['values']['old'];
  }
  else {
    $old_vid = $form_state['values']['old'];
    $new_vid = $form_state['values']['new'];
  }
  $form_state['redirect'] =  'node/'. $form_state['values']['nid'] .'/moderation/diff/view/'. $old_vid .'/'. $new_vid;
}​

I have included an updated patch for testing.

@mrgoodfellow Your changes are basically the same as in #2881777: Allow Diff to compare revisions in timestamp order (follow-up to Diff #2881771), however, that patch also includes some code that'll allow that function to continue to work if a site owner hasn't already applied the Diff patch. Personally, I'd recommend using the patch here from #14 plus the patch from #2881777: Allow Diff to compare revisions in timestamp order (follow-up to Diff #2881771), rather than the patch on #25.

To prevent confusion, I'm going to re-upload the patch from #14 so it's the latest, and set the status back to RTBC because that patch was RTBC'd in #15. Please see the previous comment (#27) for an explanation of why!

Hm, I guess it needs a re-roll! Here's an updated patch.

A number of sites using this patch started getting an error message every time cron ran. It appears that somehow the revision is getting deleted before the queued operation can take place. This will, of course, prevent the full history from being preserved, because we can't get those bits from the old revision any more. But here's a patch that at least makes this case not be a fatal error!

I messed up that last patch! Here's the real fix for the problem I was trying to address in #30.