Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stevector’s picture

Project: Workbench » Workbench Moderation

Moving to Workbench Moderation.

robeano’s picture

Any time there is a to node_save(), the node and the node_revision tables are updated with the current timestamp (see http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...).

@stevector and I went back on forth on this issue. We are not sure how to proceed just yet. In fact, it's not clear that we can even fix this issue.

A fundamental issue is that the creation of a revision is something that affects the node object itself. It appears this is how Core approaches it. This is perhaps a philosophical issue i.e. the creation of new revision should affect the updated timestamp of the node, because it is a fundamental change to the node.

Continuing to investigate.

rnyberg’s picture

Pretty weird to be honest -- I can't see why an older revision would get updated when a new one is created - it's supposed to be a revision, so losing the original timestamp data is annoying (and makes us need to do more ordering and filtering in SQL query to retrieve tha actual latest revision). I'm quite sure this didn't happend in Drupal 6 either. Haven't tested without workbench tho, so might well be a core issue instead.

stevector’s picture

Also I think this is the same fundamental problem as #1240850: Publishing/Unpublishing a revision changes the author of that revision.

I'm curious if/how State Machine handles this case. If they got around the core bug/feature, we might be able move the same technique into Workbench Moderation's 1.x and/or wait until we get to a 2.x that depends on State Machine. State Machine/ Workbench is getting discussed here: #1316314: Workflow Architecture for Workbench Moderation

stevector’s picture

Status: Active » Postponed

Part of the 2.x plans involve removing Workbench Moderation's direct interactions with the node table. State Flow will likely become the mechanism for that. State Flow has its own way of juggling node revisions. I don't anticipate making time to change this behavior in 1.x before 2.x is ready. I'm marking this as postponed instead of closed because I'm open to a patch from someone who needs to change this behavior in 1.x.

rnyberg’s picture

That plan sounds good, we've definately seen odd things happend with our data using Workbench. :) Will the 2.x be directly compatible with 1.x?

stevector’s picture

There will be an upgrade path.

JvE’s picture

Status: Postponed » Needs review
FileSize
427 bytes

The node table has a "created" and "changed" column to keep track of both.
The node_revision table only has a "timestamp" column with the description "A Unix timestamp indicating when this version was created."
Since this value is obviously meant to hold the creation timestamp of the revision and there is no value in updating this timestamp I have created the attached patch.

Status: Needs review » Needs work

The last submitted patch, workbench_moderation-timestamp-1307256-8.patch, failed testing.

JvE’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, workbench_moderation-timestamp-1307256-8.patch, failed testing.

JvE’s picture

Status: Needs work » Needs review
FileSize
463 bytes
hass’s picture

hass’s picture

I'm attaching a new patch that uses $node->workbench_moderation['my_revision']->stamp. It looks like this contains the timestamp when the published node was last saved. Therefore - over time we fix all broken dates in published versions automatically. This seems to work better than #12 for me.

Status: Needs review » Needs work
hass’s picture

Priority: Normal » Major
Status: Needs work » Needs review
hass’s picture

hass’s picture

BrockBoland’s picture

Status: Needs review » Reviewed & tested by the community

Have been able to replicate this problem locally:

  • Find a node with a published revision, dated before today, on the Manage Revisions tab.
  • Edit the node and re-save without changing anything.
  • New revision will appear on the Manage Revisions tab, and the date on the published revision will be updated to today.

This patch fixes that problem. Now, if I edit and re-save that same node—the one where the date was incorrectly changed—the date on the published revision reverts back to what it should be.

pacproduct’s picture

Issue summary: View changes

I confirm patch #18 fixes the bug for me too.
Any chance it will be included in a release anytime soon?

acbramley’s picture

This is great and fixes the issue of the node revision timestamp being incorrect. However the node table changed stamp still updates, I have a view which I want to order by Updated date but I don't want nodes that have had their drafts edited to show up at the top. Is this something that should also be fixed in this patch? It seems odd to say the Published version of a node has changed (i.e the version in the node table) when it hasn't.

Note: adding $node->changed = $node->workbench_moderation['my_revision']->stamp; underneath the $node->timestamp assignment makes the node table preserve the changed date until the node is republished.

acbramley’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
599 bytes

Patch attached for preserving the node table's changed date too.

hass’s picture

Status: Needs review » Reviewed & tested by the community

Looks codewise good to me.

jlincct’s picture

The attached patch is the way I fix this issue.

jlincct’s picture

Sorry, wrong file name on last post.

Re-attached the file again.

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

  • colan committed 3c5363b on 7.x-1.x authored by acbramley
    Issue #1307256 by hass, JvE, acbramley: Set appropriate timestamp on new...
colan’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed #22.

das-peter’s picture

Status: Patch (to be ported) » Fixed

Does not apply to the 2.x branch, it uses State Machine / State Flow for those operations.

hass’s picture

Are you really sure? You no longer save the forward revision with node_save() in 2.x?

das-peter’s picture

Yes I'm sure. Feel free to take a look in the code, no such save / revision handling there. The only thing we do is this:

/**
 * Implements hook_node_presave().
 */
function workbench_moderation_node_presave($node) {
  // Enforce a revision for new nodes, even when programmatically created.
  if ($node->is_new) {
    $node->revision = 1;
  }
}

The idea of the 2.x branch was and is to avoid "re-inventing the wheel" and in this case this means exactly a lot of the revision handling / saving stuff.

hass’s picture

I know how complicated the save process works in 1.x and just wonder how you solved this. Maybe easier to understand after a 2.x review. What I really asking me is the migration to D8. I guess the maintenance of D7 2.x becomes highly difficult as D8 changes everything or is state machine a functional backport of D8? Just interested...

das-peter’s picture

@hass As I'm not the initial developer of 2.x and also just have limited knowledge about the other related modules I can hardly deliver qualified answers. But here's what I know :)
State Machine / State Flow is the tool at hand for handling the revisioning and the different states on entity level.
The submodule workbench_workflows of Workbench Moderation integrates with State Flow to be able to create configurable workflows.
So Workbench Moderation is basically "degraded" to a tool for configuring the Moderation Workflow, provide some UI and act as glue to Workbench.

I think the idea is to have the really complex handling of revisions in a central location API as State Machine tries to provide.
Btw. "recently" Drafty was created by catch which also aims to be a bulletproof revision / publish / draft handling tool.
We really should try to focus on one module that does all the hardcore revisioning / drafting / publishing stuff. But I personally have not enough of a bird-view to be able to tell or even guess what's the one module in the future.

I'm idling in IRC if you'd like to have a more direct conversation :)

colan’s picture

It might be a good idea to post those thoughts (and anything else you discuss in IRC) over at #2100109: Port Workbench Moderation to Drupal 8 and carry on the conversation there. I don't think anyone's mentioned Drafty over there yet.

Status: Fixed » Closed (fixed)

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