I'd like to allow users in acertain role view diffs, is this possible without a huge rewrite of the code?

CommentFileSizeAuthor
#7 diff-880916.patch6.43 KBrealityloop
#4 diff-880916.patch3.53 KBrealityloop
#2 per-content-type-revision-inline-diffd6.patch1.95 KBrealityloop

Comments

realityloop’s picture

realityloop commented

Only for Inline diffs, not the revisions tab.

realityloop’s picture

realityloop commented
Status: Active » Needs review
StatusFileSize
new per-content-type-revision-inline-diffd6.patch1.95 KB

Putting this here for reference:
http://drupal.org/node/880940

drupalninja99’s picture

drupalninja99 commented

+1 and we need a permission asap (please) for the 'view changes' field as well, it auto-turns on for every content type and any user has access to the view changes button, we really need a 'view changes' permission attached to that.

realityloop’s picture

realityloop commented
StatusFileSize
new diff-880916.patch3.53 KB

@jaykali could you please test this patch against dev from 2010-Aug-13, it has perm for 'view changes button' as you requested as well.

I've added support here for people that want to use my patch at #2 as well.

I fear that 'view inline diff' perm may be somewhat redundant now that the inline diff uses a block, meaning you can apply role based access to it there.

I'd like expand these perms per content type in line with my patch at #2 if people think it would be useful. I will need to discuss it with other maintainers of Diff as well though.

yhahn’s picture

yhahn commented
Status: Needs review » Needs work

Some feedback:

  • A clearer sense of how the different permissions interact with each other would be good. For example, the new check for inline diffs appears to be this:

    (user_access('view revisions') || user_access('view ' . $type . ' revisions')) && user_access('view inline diff')

    Where the view inline diff perm becomes mostly a negative check - e.g. you must have it in addition to the view revisions permission. I think this makes sense but I wonder if there are use cases where site builders might want the opposite - e.g. users who can view diffs inline without being pestered by the revisions tab.

    The next action here is to spell out explicitly (perhaps in the comment block above hook_perm()) what each permission does and how it relates to permissions not provided by diff (ie. view revisions).

  • More consistent permission naming. I've never been a fan of the phrase view inline diff and now that we are adding additional perms for other aspects of diff I wonder if a good set might be:

    • view diff
      Allows users to see side-by-side diffs for revisions.
    • view diff inline
      Allows users to see inline diffs.
    • view diff preview
      Allows users to see diff preview when editing nodes.
realityloop’s picture

realityloop commented

Yhahn, good points.

My aim was to create a way to enable inline diff without the revisions tab eventually, I just wasn't sure how, but you've given me something to try.

I'll do some work and reroll a patch early next week.

realityloop’s picture

realityloop commented
StatusFileSize
new diff-880916.patch6.43 KB

yhahn: I've done some further work on this, unfortunately I'm stuck on how to remove the requirement for 'view revisions'.

line 112 of diff.module calls:

return $may_revision_this_type && _node_revision_access($node, $op);

_node_revision_access() requires the users to have at least 'view revisions' perm, any ideas?

Once I can figure this out I'll do clean up and submit a final patch.

Taxoman’s picture

Taxoman commented
Component: Miscellaneous » Code

Is this development currently done on 6.x or 7.x?

alan d.’s picture

alan d. commented

And is this still relevant as per the comment in #1013146: Need More Granular Permissions?

mitchell’s picture

mitchell commented
Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Needs work » Active

Bumping to 7.x-3.x.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Issue summary: View changes
Status: Active » Closed (outdated)

Given the proximity of EOL for Drupal 7, marking this as won't fix. The likely final Drupal 7 release of diff has landed in https://www.drupal.org/project/diff/releases/7.x-3.5. If this is something still applicable to a more modern version of the module, feel free to re-open and post additional notes.