[drupal-devel] [bug] node revisions should only be viewable by admins
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. killes@www.drop.org
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: jakeg Status: patch (code needs review) +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. jakeg Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Morbus Iff Status: patch (code needs review) I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... Morbus Iff Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Morbus Iff Status: patch (code needs review) Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. Morbus Iff Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=...
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: jakeg Status: patch (code needs review) good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions jakeg Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: robertDouglass Status: patch (code needs review) Please don't lump it in with administer nodes, please make a new permission. robertDouglass Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Boris Mann Status: patch (code needs review) Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. Boris Mann Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Boris Mann Status: patch (code needs review) Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. Boris Mann Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module?
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Dries Status: patch (code needs review) +1 for the permission(s), -1 for moving the revisions to a new module at this point. Dries Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision).
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Morbus Iff Status: patch (code needs review) I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. Morbus Iff Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: Morbus Iff Status: patch (code needs review) Sounds good to me! Morbus Iff Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes".
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: mathias Status: patch (code needs review) Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? mathias Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me!
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc?
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_3.module (73.36 KB) And the module killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:22:34 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: cel4145 Status: patch (code needs review) Using the latest node.module listed above and CVS as of yesterday, when an authenticated user is given view and revert permissions for stories, I get the following error when attempting to review the revisions tab for a story created by that user or by another user: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 2) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in /home/cyber/public_html/drupal-cvs/includes/database.mysql.inc on line 99. When I try to set one as active, I get warning: Cannot modify header information - headers already sent by (output started at /home/cyber/public_html/drupal-cvs/includes/common.inc:460) in /home/cyber/public_html/drupal-cvs/includes/common.inc on line 292. And the Drupal page display is doubled with that same error. one page with "Access denied" and the other with "Page not found." cel4145 Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:22:34 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting. ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:24:52 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_3.module (73.36 KB) And the module
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: bonobo Status: patch (code needs review) I also get some similar errors. I am using cvs from yesterday and the latest node.module (node_3, I believe) from this thread. Here is my set up: three users 1. first user = site admin 2. authenticated user -- has view/revert permissions on all node types 3. contrib user (in created role "contrib") -- has administer node permission, no view/revert privileges The site admin and contrib user can use view/edit/revert between various revisions with no problems. When user 2 (authenticated user) attempts to view a post that has been revised, the following error occurs: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 6) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in C:\apachefriends\xampp\htdocs\drupalcvs\includes\database.mysql.inc on line 99. When the same user attempts to revert to an earlier version, the same error occurs. bonobo Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:22:34 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting. ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:24:52 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_3.module (73.36 KB) And the module ------------------------------------------------------------------------ Tue, 06 Sep 2005 14:57:52 +0000 : cel4145 Using the latest node.module listed above and CVS as of yesterday, when an authenticated user is given view and revert permissions for stories, I get the following error when attempting to review the revisions tab for a story created by that user or by another user: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 2) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in /home/cyber/public_html/drupal-cvs/includes/database.mysql.inc on line 99. When I try to set one as active, I get warning: Cannot modify header information - headers already sent by (output started at /home/cyber/public_html/drupal-cvs/includes/common.inc:460) in /home/cyber/public_html/drupal-cvs/includes/common.inc on line 292. And the Drupal page display is doubled with that same error. one page with "Access denied" and the other with "Page not found."
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: bonobo Status: patch (code needs review) And, I also get the same doubled display as Charlie -- Access denied on top, Page not found on bottom. bonobo Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:22:34 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting. ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:24:52 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_3.module (73.36 KB) And the module ------------------------------------------------------------------------ Tue, 06 Sep 2005 14:57:52 +0000 : cel4145 Using the latest node.module listed above and CVS as of yesterday, when an authenticated user is given view and revert permissions for stories, I get the following error when attempting to review the revisions tab for a story created by that user or by another user: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 2) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in /home/cyber/public_html/drupal-cvs/includes/database.mysql.inc on line 99. When I try to set one as active, I get warning: Cannot modify header information - headers already sent by (output started at /home/cyber/public_html/drupal-cvs/includes/common.inc:460) in /home/cyber/public_html/drupal-cvs/includes/common.inc on line 292. And the Drupal page display is doubled with that same error. one page with "Access denied" and the other with "Page not found." ------------------------------------------------------------------------ Wed, 07 Sep 2005 07:06:15 +0000 : bonobo I also get some similar errors. I am using cvs from yesterday and the latest node.module (node_3, I believe) from this thread. Here is my set up: three users 1. first user = site admin 2. authenticated user -- has view/revert permissions on all node types 3. contrib user (in created role "contrib") -- has administer node permission, no view/revert privileges The site admin and contrib user can use view/edit/revert between various revisions with no problems. When user 2 (authenticated user) attempts to view a post that has been revised, the following error occurs: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 6) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in C:\apachefriends\xampp\htdocs\drupalcvs\includes\database.mysql.inc on line 99. When the same user attempts to revert to an earlier version, the same error occurs.
Issue status update for http://drupal.org/node/30098 Post a follow up: http://drupal.org/project/comments/add/30098 Project: Drupal Version: cvs Component: node system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: killes@www.drop.org Updated by: killes@www.drop.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/node_rev_4.patch (11.32 KB) Oops, I confused a permission. it needs to be node_access('update', $node), note 'edit'. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:36:19 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev.patch (1.27 KB) With or without the revisions patch, every user who can access content can access old revisions. I think this is a bug because why would you need to see old revisions if you cannot change them or make them the current revision? The attached patch fixes this and lets only users with "administer nodes" permission see old revs as you need this permission to change revisiosn to be the current one. One might make a case for introducing new "view revisions" and "set revisions" perms. You woud need those if you wanted to mimic a wiki with Drupal. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:14 +0000 : jakeg +1 I would also like to see the extra permissions added if possible: 'view node revisions' - just for viewing node revisions 'administer node revisions' - to delete node revisions or set them as default revision (i.e. current revision) for a node I think this patch is important because e.g. if a revision is made because sensitive information was included in a past revision, such as a phone number, plain text email address (someone stupid didn't know about spammers or the contact form) then these should definitely NOT be accessible, and it would often be desirable to create a new revision to a node rather than to edit the current copy without creating a revision. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:55:28 +0000 : Morbus Iff I disagree. I see revisions as a means of updating a document for the users. If a user has read the document, then sees that a revision has been made, I'd want to provide him with some sort of diff that shows him exactly what, as opposed to making him read the whole blasted thing over and over again. In essence, I want a a wiki diff between revisions (for example) [1]. Putting this permission in place would restrict my ability to do that. [1] http://gamegrene.com/wiki/?title=WhereIsWhere&curid=927&diff=0&oldid=0&rcid=... ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:57:15 +0000 : Morbus Iff Note to self: actually read the whole report before commenting. -1 to JUST this patch. +1 to view/set revisions. ------------------------------------------------------------------------ Wed, 31 Aug 2005 13:59:13 +0000 : jakeg good point #2 morbis, but the revisions tab isn't accessible to normal users anyway... they only get to see the revisions if they know the URL to get there (?revision=x in 4.6; /revisions/x in head) but i agree that its definitely better with the extra permissions ------------------------------------------------------------------------ Thu, 01 Sep 2005 19:39:54 +0000 : robertDouglass Please don't lump it in with administer nodes, please make a new permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 21:02:15 +0000 : Boris Mann Yes, please make this a separate permission. In actuality, the use cases for viewing revisions probably needs a permission *per node type*. E.g. a site uses "story" for front page content. Editors some times revise stories, but don't want those revisions public. The same site uses book pages like a wiki. It gives all users create/edit/maintain book privileges, and additionally wants them to be able to see revisions. So, if we don't enable viewing revisions for node type, I would hope for a way to override this at the node type/module level -- so a custom wiki.module could automatically add a "view revisions" permission. ------------------------------------------------------------------------ Thu, 01 Sep 2005 23:55:46 +0000 : killes@www.drop.org Boris, what you propose makes sense to me. I am however waiting for feedback from Dries before I re-roll this patch. if we introduce this new permissions, maybe we should factor the revisions out into their own module? ------------------------------------------------------------------------ Fri, 02 Sep 2005 01:37:34 +0000 : Boris Mann Revisions as a separate module might very well make more sense. Sites can choose to run with them enabled or not. Might make it easier to eventually do file versioning if we can hook into a module. ------------------------------------------------------------------------ Fri, 02 Sep 2005 05:46:30 +0000 : Dries +1 for the permission(s), -1 for moving the revisions to a new module at this point. ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:08:21 +0000 : killes@www.drop.org Dries, thanks for the feedback. I will prepare such a patch. Thanks for saving my time, too. ;) Boris, we already do some sort of file revisions. A file that is attached to one revision can be deleted from another one, but will still be available with the original revision. Only if we delete a node completely, the file will actually deleted (or you delete it from every revision). ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:18:03 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_0.patch (7.69 KB) Ok, here is a patch which implements edit and view permissions per node type. "admin nodes" is still valid as well. Needs testing. ------------------------------------------------------------------------ Fri, 02 Sep 2005 15:23:42 +0000 : killes@www.drop.org NB: I'd appreciate a review from wiki users. I've implemented "view" as access to the revision overview page and the individual revs and ""edit" as "change current revision" + "delete revision". I guess the latter is a bit dangerous. ------------------------------------------------------------------------ Fri, 02 Sep 2005 22:20:01 +0000 : Morbus Iff I haven't tested the patch, but "delete revision" concerns me - it's the style of a wiki to NEVER delete a previous version of a document - to always have a permanent record of its manifestations. Annoyingly enough, I woudn't +1 these new permissions until "delete" became its own perm as well, separate from edit. ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:20:00 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_1.patch (7.76 KB) Expected this. ;) I've renamed "edit revisions" to "revert revisions" because that is what you can do with that permission. I did not introduce a "delete revs" permission but moved that permission back to "administer nodes". ------------------------------------------------------------------------ Sat, 03 Sep 2005 16:45:16 +0000 : Morbus Iff Sounds good to me! ------------------------------------------------------------------------ Sat, 03 Sep 2005 19:50:24 +0000 : mathias Having node module set revision permissions for all other node modules contradicts the interface of the permissions screen. The current interface suggests that if I want to set revision permissions for books, I would look underneath book heading, not node. Interface issues aside, if node module is going to set revision permissions for all other node mods why stop there? Why not "create $type", "edit own $type", etc? ------------------------------------------------------------------------ Sat, 03 Sep 2005 22:38:43 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_1.module (72.61 KB) @matt: Yeah, you are right abiut that screen. No idea what to do, though. I don't think that we shoudl create permissions by default if we cannot enforce their use. For the rev perms we can that. I've attached a patched node module for testers coming from http://drupal.org/node/30329 ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:49:06 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_2.patch (10.69 KB) Here's a new patch that adds some node_access checks and reorganizes the code a bit. node_page was already too spaghetti so I made a new menu callback for revisions related stuff. ------------------------------------------------------------------------ Mon, 05 Sep 2005 00:50:17 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_2.module (73.1 KB) and the patched module ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:22:34 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_rev_3.patch (11.31 KB) Ok, here is a new patch whihc fixes the permission bug and also creates new revisions when reverting. ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:24:52 +0000 : killes@www.drop.org Attachment: http://drupal.org/files/issues/node_3.module (73.36 KB) And the module ------------------------------------------------------------------------ Tue, 06 Sep 2005 14:57:52 +0000 : cel4145 Using the latest node.module listed above and CVS as of yesterday, when an authenticated user is given view and revert permissions for stories, I get the following error when attempting to review the revisions tab for a story created by that user or by another user: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 2) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in /home/cyber/public_html/drupal-cvs/includes/database.mysql.inc on line 99. When I try to set one as active, I get warning: Cannot modify header information - headers already sent by (output started at /home/cyber/public_html/drupal-cvs/includes/common.inc:460) in /home/cyber/public_html/drupal-cvs/includes/common.inc on line 292. And the Drupal page display is doubled with that same error. one page with "Access denied" and the other with "Page not found." ------------------------------------------------------------------------ Wed, 07 Sep 2005 07:06:15 +0000 : bonobo I also get some similar errors. I am using cvs from yesterday and the latest node.module (node_3, I believe) from this thread. Here is my set up: three users 1. first user = site admin 2. authenticated user -- has view/revert permissions on all node types 3. contrib user (in created role "contrib") -- has administer node permission, no view/revert privileges The site admin and contrib user can use view/edit/revert between various revisions with no problems. When user 2 (authenticated user) attempts to view a post that has been revised, the following error occurs: user error: Unknown column 'grant_edit' in 'where clause' query: SELECT COUNT(*) FROM node_access WHERE (nid = 0 OR nid = 6) AND CONCAT(realm, gid) IN ('all0') AND grant_edit >= 1 in C:\apachefriends\xampp\htdocs\drupalcvs\includes\database.mysql.inc on line 99. When the same user attempts to revert to an earlier version, the same error occurs. ------------------------------------------------------------------------ Wed, 07 Sep 2005 07:09:39 +0000 : bonobo And, I also get the same doubled display as Charlie -- Access denied on top, Page not found on bottom.
participants (9)
-
bonobo -
Boris Mann
-
cel4145
-
Dries
-
jakeg
-
killes
-
mathias
-
Morbus Iff
-
robertDouglass