Revision message repeated

Thanks Rik, I appreciate your help. It is annoying when these subtle bugs happen, and they can be the hardest to get rid of. I got lost trying to follow the logic of how the message is generated, so I couldn't attempt to solve the problem myself.

I don't know if it is helpful, but the other permission-related modules I have are: module grants, nodeaccess. I do not have workflow.

Ron.

Hi again, I've tried an interesting experiment, using two static variables to count how many times functions are executed. I did:

function revisioning_nodeapi(&$node, $op, $teaser = NULL, $page = NULL) {
$args = arg();
static $count = 0; $count++;
...
drupal_set_message(_get_node_details($node, $count));
...
function _get_node_details($node, $count) {
static $count2 = 0; $count2++;
...
if ($is_current) {
$message = t('Displaying %current revision of %title, last modified by !author on @date ' . $count2 . ' ' . $count, $placeholder_data);
}

and I got this display on the page:

* Displaying current, published revision of Welcome, last modified by Ron House on 05/11/2009 - 15:48 1 3
* Displaying current, published revision of Welcome, last modified by Ron House on 05/11/2009 - 15:48 2 5

So revisioning_nodeapi is being called twice with op=alter, and that is causing two calls to _get_node_details.

I tried the following very crude fix, but it seems to work:

function revisioning_nodeapi(&$node, $op, $teaser = NULL, $page = NULL) {
$args = arg();
static $count = 0;
...
if (module_grants_node_revision_access('view revisions', $node) && !$count++) {

This also possibly skips a second call to _theme_content_menu, and I don't know what the consequences of that might be. Hope this helps, Ron.

Hi Rik,
The content types on the site are standard: book, story, page. But a closer look reveals that it is only story, not book or page, that shows duplicate messages. Strange! I'll try the new versions before I explore your other suggestions, as it may be the problem is now fixed.
Thanks for your help!
Ron.

Hi again Rik,
Tried the new HEAD version of revisioning. The duplicate messages have gone, but unfortunately two worse bugs have appeared:

1) Published content edited by my guest author role (which has edit revisions permission, but not publish or unpublish) when saved, immediately becomes the published version - i.e., without moderation.

2) The moderator's PENDING tab has vanished.

I am afraid the code is beyond me, so I can't help out to find the problem. Thanks for your help!
Ron.

Thanks midkemia. That fixed the missing PENDING tab, but the problem that the guest author's changes are automatically published still remains.

The user interface of the changes is really brilliant though!

Also, although this is probably what it intended, I have lost a feature I need. I want the guest author to be able to see but not edit all unpublished content. The official current versions allow this, but the HEAD versions don't. We really need an "access unpublished content" permission, which, if granted should simply funnel all permission checks through the normal set of checks performed for published nodes. I don't know which module should solve this problem.

Cheers, Ron.

Hi John, Thank you so much for taking the time out to try to get to the bottom of my problem!

I am setting up a site that will have guest authors writing posts, which I want to verify for content, html sanity, etc., before it goes live. The guest authors are friends whom I trust completely, so I want them to be able to see the whole site, but they are not as drupal-savvy as I, and I don't want them to accidentally do terrible things, which they could do with administer nodes permission. Essentially I want the system to give them the same access to unpublished content that it would give them to the same item if it were published, so they can get advance warning of upcoming articles etc. In other words, the idea is that with an "access unpublished content" permission, the system would do all the normal checks, except that it would ignore the (un)published status for these people. I do envisage that I might in future have guests whom I trust a little bit less, so if there were such a permission, I could invent another role that differed only in lacking that one permission.

I want to check articles before they go live, so when a guest edits or creates an article, the new version should remain unpublished until I vet it and publish it. (Guest author does not have publish or unpublish revisions permission, so it seems odd that the HEAD version of revisions lets their edited version go public, as that is an implicit publication of the edited copy and unpublication of the previous version.)

I think I see why we are on slightly different wavelengths: you are seeing the permissions as determined by the type of node, whereas I don't have content categories that mean anything much as far as security goes (although I do see that they could be created if necessary). The security here amounts basically to only two things: guests can edit only their own articles, and they cannot publish or unpublish. Everything else is open to them. Normal anonymous and authenticated users should only see published stuff, of course.

With your "adults only" scenario, yes, I think your assumptions are correct. The nodeaccess module seems to handle that scenario by allowing one to set up an adult node with special restrictions.

Hope that all makes sense, and thanks again.
Ron.

Thanks Rik, I had somehow failed to notice your #9 (need new glasses I guess!). I have checked everything you mentioned there and in #12. I had granted guest "revert revisions" permission, but have unchecked it - problem still persists. I don't understand the difference between that and unpublish though.

"I don't think I quite follow you with this "access unpublished content" permission.
The content access rights (view, edit, delete) are already independent of the publication status, aren't they?"

I don't think so. Guest still cannot see unpublished content from other users. (But they can in the current stable release version.) Since other sites will not want what I want, I suspect this new permission is the only way to satisfy everyone.

So still two problems: (1) Guest edits own current published version, and the new edit becomes the current published, instead of waiting for moderation, and (2) Guest cannot see other users' unpublished content. If Guest creates a fresh story, it correctly remains unpublished and appears in moderator's pending tab.

If you are not getting this behaviour, could it be some conflict with nodeaccess, which is the only other permission-related module I have?

If I might suggest a usability improvement: Users withOUT (un)publish permission are most likely to be editing their own works, and "I created" seems the right default in Administer content, but users with (un)publish are likely site moderators who will want to see all pending content by default and are most likely to want "I can access". Even in my testing, I have been tripped up by that, spending time wondering where the guest article has gone, only to realise some time later that I am only looking at my own articles.

Again, many thanks for the great module,
Ron.

Thanks John - already disabled, sadly.

One thought that occurred to me: after installing the HEAD versions, I ran update.php. Was that the right thing to do?

Ron.

PS [CORRECTED] More testing: It seems that if Guest edits her own UNpublished original, a new revision is created, also unpublished. But if Guest edits her own original after being published by moderator, the publication status (published) transfers to the new revision and her changes go into publication. I am intrigued enough to delve into the code again and see what more I can learn (every time I do it I am more impressed). I must say you guys sure have done a magnificent job thinking through all this stuff!

I believe I have found the bug causing my main trouble. In _handle_edit, the code has:

       case 'presave': // called from start of node_save()
...
       else {
          // Save the vid for subsequent restore during 'update' op
          $node->original_vid = _get_current_revision_id($node->nid);
        }
        break;
      case 'update': // called from end of node_save(), after _node_save_revision()
        $node->revision = $node->original_revision;
        if (isset($node->original_vid) && $node->original_vid != $node->vid) {
          // Resetting node vid back to its originial value, thus creating pending revision
          db_query("UPDATE {node} SET vid=%d WHERE nid=%d", $node->original_vid, $node->nid);
        }

But case presave and case update are executed in two different calls to _handle_edit, so changes are not propagated through the $node argument, which has been called by value. I changed the call and formal parameters from:

    _handle_edit($node, $op); // 'prepare', 'presave', 'update'
...
function _handle_edit($node, $op) {

to:

    _handle_edit(&$node, $op); // 'prepare', 'presave', 'update'
...
function _handle_edit(&$node, $op) {

and it stopped automatically publishing Guest's edits to a published version. I think it has also stopped making gratuitous versions, but I need to do a thorough check.

Ron

Hi Rik, thank you.

The gratuitous revisions stopped also because of this change, as I never had that set in the first place.

[UPDATE: inserting your missing FROM fixes a second bug I had noticed - thanks!]

Ron

Hi again Rik,

I have made another change that seems to make sense in the case where the "Every time content is saved" option is selected. At present, if Guest creates a node and then edits it, the revisions display says:

Saved 05/29/2009 - 17:16 by Karen pending moderation
Saved 05/29/2009 - 17:10 by Karen current revision (unpublished)

and the earlier version is flagged as current - which makes sense if current is published, but not if it isn't, as here, since the naive Guest is likely to start editing the "current" version, expecting to be editing their latest save. It is better in this case to make the latest version current, so the display reads:

Saved 05/29/2009 - 17:40 by Karen current revision (unpublished)
Saved 05/29/2009 - 17:10 by Karen archived

It turns out to be relatively easy to do this. Change:

      case 'presave': // called from start of node_save()
        $node->original_revision = $node->revision;
...
      case 'update': // called from end of node_save(), after _node_save_revision()
        $node->revision = $node->original_revision;
        if (isset($node->original_vid) && $node->original_vid != $node->vid) {

to:

      case 'presave': // called from start of node_save()
        $node->original_revision = $node->revision;
        $node->original_status = $node->status;
...
      case 'update': // called from end of node_save(), after _node_save_revision()
        $node->revision = $node->original_revision;
        if ($node->original_status && isset($node->original_vid) && $node->original_vid != $node->vid) {

Of course I am merely futzing around here, very grateful for the opportunity to learn to understand this snazzy code; you may well see some more elegant way to do it.

Ron.

Hi Rik,

"I see where you're coming from, but it would interfere with a second scenario: that of a previously published node as opposed to a node pending publication."

I wondered about that and tested it. AFAICT, that case continues to work in the present manner. Here is the revisions after two edits by Guest:

Saved 05/29/2009 - 19:23 by Karen pending moderation
Saved 05/29/2009 - 19:22 by Karen pending moderation
Saved 05/29/2009 - 17:43 by Karen current revision (published)

"So your patch effectively turns a node pending publication into a previously published node, so it will no longer show up under "pending"."

I don't think so. The extra check only prevents shifting the 'current' marker when there is no published node in the trail, it doesn't alter any tests for pending. I have entered three stories, one never published with only one edit (the original), one never published with two edits, and one which has edits after the published version, and all three show up in Pending. I can't find a case that the change changes, except the one it is meant to change.

Of course I don't really understand the subtleties of the various fields in $node, so I could be way off beam.

Ron.

Hi Rik,

Yes, I see what you mean. However, it raises a new question, how does the moderator proceed if he reads the revision, decides not to publish it as it stands, and tells the author to revise it? How does he remove it from Pending until it has been edited again? It seems that 'current' status is used to flag which revision has been checked by moderator (a newer one flags pending again) but with unpublished work, there doesn't seem to be a way for moderator to shift the current flag onto the one he has rejected. Publishing does it, but he doesn't want to do that. Also, by using current status in this way, when only a single revision (the original) exists, it must be current, so it is taken as pending. Even with a way to shift the 'current' flag, moderator can never get that article out of the Pending tray until a second revision is made. This is likely the most common case.

I apologise for letting this thread get so long and take so much of your time, but the module has subtlety and a fascination about it that just drags one in!

Also I think I forgot to answer your question about the duplicated messages - they seem to be fixed since the other bug got put right - thanks. And apologies for not sticking to one point in one thread!

Ron.