Wiki pages are mostly useless, because we don't allow users to see and/or revert revisions on those pages.

The main argument against allowing users to see revisions is that this permissions is not fine-grained enough. I believe we can easily solve that easily by adding a simple hook_menu_alter() in the groupsorg module.

Comments

moshe weitzman’s picture

This is a frequent request. Due to lack of hook_menu_alter() which I won't get into now, I am unwilling to do this in D5. It will be in D6 for sure. Coming very soon.

moshe weitzman’s picture

Actually, someone needs to port the "show diff tab by content type" feature from #120955: Integrate Diff into Core to the module. Help wanted.

markus_petrux’s picture

Is there any feature request or something in the diff queue for that?

By "show diff tab by content type" do you mean the new 'node_show_view_changes' option in the content type settings page, and the 'view_changes' option in the node edit form? If so, I can try the port to diff 6.x-2.x. Is that ok to install diff at g.d.o?

moshe weitzman’s picture

Title: Display revisions for wiki content on groups.drupal.org » Display revisions by content type
Project: Drupal.org site moderators » Diff
Version: » 6.x-2.x-dev
Component: Groups.drupal.org » Code

I mean, show the revisions tab only on certain content types. yes, there needs to be admin pref added to content type form.

i moved this issue to diff queue.

groups.drupal.org shows revsiions tab currently to editors. once this patch hits, we will start showing to everyone for wiki type.

markus_petrux’s picture

Well, I've been looking at the latest patch in #120955: Integrate Diff into Core and it seems to me it is not complete, unless I'm missing something.

I would like to try to work out this feature here, so I need to ask.

- We have 'view revision' permissions, and this is checked in node module by the access callback _node_revision_access().
- We want to add an option to the content type settings form to enable the 'View changes' button in the node edit form.
- This 'View changes' in the node edit form seems to allow previewing the diffs when editing.

Since latest patch doesn't seem to alter _node_revision_access(), I have these doubts:

- This 'View changes' in the node edit form is not affected by 'view revision' permission?

- I'm not sure how this affects the tab to view revisions. So, do we need variants of the 'view revision' permission the same way we have these variants for edit permissions, etc.? If so, I guess we need to create here a new access callback that implements this checks, right? wrong?

Edited to clarify my doubts as I was initially confused. Sorry.

markus_petrux’s picture

Sorry, I'm confused because you asked to backport something that I've been unable to see implemented in the patch. :-/

...or is it as simple as adding the option to the content type settings form, and then only show the diff tab to users with 'view revision' permission AND only for the content types that have this option enabled?

If this is the case, then the menu items need to use a different access callback that checks users permissions, but also if the feature is enabled for the given content type. Right?

markus_petrux’s picture

Status: Active » Needs review
StatusFileSize
new2.04 KB

Well, here's a patch that implements 'Enable revisions page' at content type level, and then uses a new access callback to check for this option, in addition to regular user permissions and existence of revisions for the given node.

It also introduces a hook_uninstall() so that site admins can clean up module variables when uninstalling the module.

moshe weitzman’s picture

Yes thats what I had in mind. I really thought that was in the big D7 patch.

- the uninstall change does not look right. the preview changes button is unrelated, no? that button also got renamed in the big patch but thats not really relevant here.
- i never saw #prefix on a checkbox. Is that prettier?

I did a code review and all looks good. yhahn is the new maintainer here so lets wait for him to chime in or commit this.

markus_petrux’s picture

The preview changes button is also implemented as an option by diff. And I have built the new checkbox using the same exact technique (#prefix et al).

The hook_uninstall() implementation should have been there just for the 'preview' changes setting. But now we are adding a new option, which means more variables per content type, so I thought we could cover this clean up stuff in this patch.

emok’s picture

StatusFileSize
new6.97 KB
new2.75 KB

I've tested the patch and think there is a glitch: we need to use the new access callback on all kinds of revision pages. Use it on 'node/%node/revisions/view/%/%' and 'node/%node/revisions/view/latest' which are defined in diff_menu, plus in diff_menu_alter you need to add the line
$callbacks['node/%node/revisions/%/view']['access callback'] = 'diff_node_revision_access';.

Then, I'd vote for a bit more flexibility. If I have a wiki content type and disable revision viewing for all other types, then even the administrators are constrained. I'd suggest either a new permission or reusing 'administer nodes' that overrides the restrictions set by this patch. By that I mean to change diff_node_revision_access into:

function diff_node_revision_access($node) {
  $may_revision_this_type = variable_get('enable_revisions_page_'. $node->type, TRUE) || user_access('administer nodes');
  return $may_revision_this_type && _node_revision_access($node);
}

With these changes it works well to me.

Also, I made another version that incorporates the renaming of 'Preview changes' to 'View changes' that seemed to be agreed upon in issue 120955.

emok’s picture

StatusFileSize
new7.16 KB
new2.94 KB

Sorry, I missed 'node/%node/revisions/%/revert' and 'node/%node/revisions/%/delete'.

I may also add that the rename-version of the patch is not fully compatible with the patches found in the other issue, particularly I did not rename the variables show_preview_changes_<type> to node_show_view_changes_<type>. I just changed the UI name of the button.

BenK’s picture

Hi everyone,

What is the status of the latest patch (#11 by emok)? Are there plans to commit this to 6.x-2.x-dev of the main Diff module?

I, too, need this functionality.... I need users to be able to view revisions for some content types, but not view revisions for other ones.

If someone could let me know whether this patch is likely to be committed, I'd appreciate it.

Cheers,
Ben

kate’s picture

Hi,

I am also very interested in the status of this issue. I would like to be able to have group members view revisions for their group wiki pages.

Thanks,
Kate

markus_petrux’s picture

Both patches in #11 (with or without s/Preview/View) look good. Is there anything else we can do to help get this in?

samchok’s picture

subscribe

greggles’s picture

There is now a module - http://drupal.org/project/view_revisions_by_content_type - which seems to provide a similar feature. (also subscribe)

yhahn’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

jmlane’s picture

I was also wondering about that, greggles. Would http://drupal.org/project/view_revisions_by_content_type be depreciated with the above commit, once it makes it into a stable diff release? There isn't a 5.x version of that module, so the functionality is essentially reproduced here, but diff also includes some basic revision tools that should be standard in most Drupal sites exposing revisions.

Seems like it might be worth bringing this to Wim's attention so he can choose to redirect people looking at view_revisions_by_content_type to diff, if he thinks that is the best course of action.

Thanks for this patch, this is a great feature for diff that I look forward to using in production.

greggles’s picture

@jmlane - seems reasonable to me. Please open an issue in the queue for view_revisions_by_content_type.

jyee’s picture

Fwiw, "view revisions by content type" is significantly different because it integrates into role permissions. Diff disables revisions tabs on specific content types for everyone (except "administer nodes"). View revisions by content type allows you to disable the revisions tab on content types for specific user roles, which is far more flexible, especially if you have a number of specific content type author and editor roles.... and of course, it's handled in permissions, so it's easy to administrate it from one page.

jmlane’s picture

Ah, I see that you are right, jyee. I will update #672654: Diff module duplicates module functionality to reflect this. I believe the View revisions by content type module's method of managing these permissions is superior when I think about this issue again.

Perhaps some of the contributors that worked on the D7 Diff integration and the code that was committed in this issue can comment on the advantages/disadvantages of putting this access check into a new permission per content type by individual user role?