Hello,
Your module is a life-saver for me. I was afraid I'd have to program something similar from scratch. I work for a University, and we needed a way for people to submit nodes which would then be moderated on a per-revision basis.
The hitch was that we didn't want to give the moderator the full "administer nodes" permission. So, I started making changes to your module to suit our needs here. I thought I would post my changes, in case anyone else wanted to check them out or comment on them (or, if you want to include them in your next release).
I marked ALL of my changes with the text: RP EDIT. So if you want, you can just search through the code for "RP EDIT" to see what I did.
Below is a list of what I changed (starting from 6.x-1.0-alpha2's .module file):
#1: Created a new permission, called "approve revisions." The "administer nodes" permission is no longer required. Instead, give the "approve revisions" permission to the user(s) you want to be able to moderate revisions.
#2: Added to the settings form, to let you set what message is displayed to the user upon submission (instead of simply "Your changes have been submitted for moderation").
#3: Fixed a bug with the function revision_moderation_get_all_pending_revisions() function. As it was, if you had 2 revisions waiting for the same node, the earliest revision would be returned-- but logically you would want the latest revision returned instead. That is what I changed.
I might make more changes or additions during the course of my project here at work. If I do, I will post updated files in response to this message.
Thanks again for this great and useful module!
Richard
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | revision_moderation-919828.patch | 10.21 KB | weseze |
| #4 | patch.diff | 13.79 KB | jec006 |
| revision_moderation.module.txt | 18.77 KB | richardp |
Comments
Comment #2
jbrauer commentedHi Richard,
Thanks for your work on this. I'll take a look but the changes sound like a great start. If you get a chance to take a look at creating patches as in http://drupal.org/patch/create it's generally easier to work through the issue queue in patches vs. full copies of the files.
Josh
Comment #3
richardp commentedSure thing. I will read up on patches. I will confess that I have very little experience with patches, but if in the course of my project I do any more work, i will try to submit it as a patch instead of the whole .module file.
Richard
Comment #4
jec006 commentedI have made a patch file in the hopes that they changes can be rolled into the module.
Comment #5
jbrauer commentedThanks for rolling this into a patch I very much like a number of the ideas here. I looked through it and noticed a few things. I moved this to a 'feature request' since it's mostly features. This isn't comprehensive just what stood out on a quick run-through:
1) Inline documentation should follow the Doxygen standard as partially documented at http://drupal.org/node/1354
2) Handling of revision_moderation_msg in this patch is insecure
3) Why does the patch remove the ability to have revision moderation exempt authors? I can understand not using it but some people will want to use it.
4) What is the benefit of sorting the list by timestamp instead of vid? Since vids are auto-generated they follow with timestamps and are indexed in the database so a bit more performant in doing the queries.
Thanks for the great work here it seems like something that will help make the module better.
Comment #6
richardp commentedThanks, jec006, for making the patch. My patching skills are still woefully inadequate :(
@jbrauer, I'm sorry about the possible sloppiness (the documentation, for example). How was the handling of revision_moderation_msg insecure?
I agree that the revision_moderation_exempt checkbox is probably a good one to keep in. What would be really handy, which I did not program, is if that would include people with the "approve revisions" permissions as well as ones with "administer nodes."
I believe you are correct about the vid. I think I did that while I was trying to track down the bug of it not showing the most recent revision, and it is no longer needed. Good point about vid being indexed.
Richard
Comment #7
weseze commentedI have reworked the patch a bit. Really good stuff to commit when ready!
Comment #8
Alex Andrascu commentedI'm keen to see this one on the dev release. Subscribing.
Comment #9
Alex Andrascu commented