Posted by mrtoner on February 14, 2007 at 6:07am
Jump to:
| Project: | Revision Moderation |
| Version: | 4.7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (won't fix) |
Issue Summary
In the Pending Revisions block, the list always ends with the "View all pending revisions" link even if the user does not have administer nodes permission. This means a non-admin user can click this link to admin/node/revisions, but will get an "Access Denied" error. IMHO, the link should simply not appear for non-admin users.
I know enough about PHP to add the appropriate if statement:
if (user_access('administer nodes')) {
$output .= '<p>'. l(t('View all pending revisions'), 'admin/node/revisions') .'</p>';
}but I have to leave it to someone else to patch the module or update it. (How do you create a patch, anyway?)
Comments
#1
Oh, another thing about that link that I didn't fix yet: it always shows up when there are no pending revisions. Kinda strange to see "No pending revisions found" followed by "Review all pending revisions."
#2
What's the date at the top of your file? This should have been fixed last night.
While the module's still in flux, you might be better off to grab it from here instead: http://cvs.drupal.org/viewcvs/drupal/contributions/modules/revision_mode...
#3
D'oh! Ignore me. Different error. :)
Sure, I'll get that fixed up. Just a sec.
#4
Actually I can't reproduce this? The block doesn't even show up for non node administrators, because the whole thing is wrapped in an if (user_access('administer nodes')):
elseif ($op == 'view') {
$block = array();
if (user_access('administer nodes')) {
// make the block
}
return $block; // an empty array won't be shown.
}
So now I'm back on the "what version of the module are you using?" ;) At the top of the file, there should be a line like:
// $Id: revision_moderation.module,v 1.20.2.7 2007/02/14 15:22:21 webchick Exp $Make sure that line matches the one at the top of the first file in this list (click "view" -- though NOT the HEAD view):
http://cvs.drupal.org/viewcvs/drupal/contributions/modules/revision_mode...
Once the most recent changes have a bit more testing, then I'll create an official release for the module so there won't be any questions. :P
#5
Oops! Sorry, webchick: I had found it necessary to modify revision_moderation (I'm using v1.19) because
1) The pending revisions block would not, as you point out, appear for users without admininster nodes permission, and
2) A user could not edit revisions if they a) did not have administer nodes permissions or b) did not have both view revisions and edit node permissions.
In my particular situation, I have 30+ users who can edit their own content and two editors who check the pending revisions. However, I don't want to give the editors permission to administer nodes (too many other settings for them to fool with) and instead use the nodeaccess module to allow editors to edit content not their own. I modified revision_moderation_menu() and revision_moderation_block() with additional conditions to allow the display of the menu and block -- and forgot that I had done so before starting this issue.
I think this points up, though, that users with view revisions permission should be able to see the pending revisions block -- not just those with administer nodes permission. I don't know how you might allow the "edit" link to appear -- although I'm using a user access('revert revisions') test, that's probably not appropriate for all uses of this module.
#6
OK, fair point. I'll see what I can do.
#7
Here's a patch.. it's against HEAD but might apply with some offsets to 4.7.
It changes the pending revisions block and admin page to be visible by people with 'view revisions' permissions. Edit and Publish are only visible if you have revert revisions + access to update a given node.
I'm not too sure about this, personally (except the revert + update change should probably be made in any case)... but I leave it for review so we can get some discussion around this.
#8
I don't see the patch webchick. Did you forget to attach it to your last comment ? Or maybe I have to replace my eyes...
#9
Ah, crap. :(
I don't still have that patch, so I'll need to rewrite it at some point.
#10
It would be very practical with the flexibility that mrtoner refers to for this feature request.
So, +1.
However: Would this still require the use of an ACL module for it to work?
If so, this is a pretty "generic" scenario that would be useful to discuss with the maintainers of such ACL modules, to reach a consensus about how to best support this across the board. Perhaps the ACL API module is the only relevant place to ensure support for such a feature.
Trying to adjust the title to a more descriptive one. (Does _this_ one make sense?)
I guess this is desireable for both 4.7.x and 5.x. Should perhaps update the version on this issue.
#11
Since 4.7 is past support closing this. If there is still a need and a desire to write patches for Drupal 6 it will be revisited.