Project:Node Hierarchy
Version:6.x-3.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:active

Issue Summary

I had a need a year or so ago to extend out some of the Node Hierarchy permissions. I created a very quick module called nodehierarchy_add_children_only (#811274: Recursive Pathauto for 6-x-2.x and "Add Children Only", github). With the 2.0 release, I was interested in incorporating these changes back into the module, but when I started looking into it, I got a little distracted by the permissions.

I had run into some issues with the permission nomenclature before and thought it would be a good time to try and iron them out. As I didn't want to work isolated, I figured it would also be good to run these by the community first and get some feedback before I took a stab at them. I created a Google Doc detailing all the NH permissions and where they were used. Node Hierarchy Views also has some separate permissions, but after reviewing them, I didn't see any improvements needed.

Anyways, here's a list of what I was thinking about.

  1. Update permission’s names
    1. create child nodes => create child of own parent node
    2. edit all node parents => change the parent of any node
    3. edit own node parents => change the parent of own node
    4. customize nodehierarchy menus => customize nodehierarchy menu items
    5. create child of any parent => create child of any parent node
  2. Delete unnecessary permissions
    1. reorder children
    2. view site outline
    3. administer hierarchy. “administer hierarchy” is currently being used to display helper text on how to enable content types as potential parents on a node’s edit form. As the node hierarchy administration page currently requires “administer site configuration” permission, I propose updating the helper text to also use “administer site configuration.”
  3. Review and, if necessary, rework permission implementations.
    1. During some of my research, I came across instances where the permissions did not behave as expected. It seemed like they were typically fringe cases where I was deliberately throwing curveballs.
    2. Potentially reduce permissions. Would there ever be a scenario where a user could “create a child of own parent node” but not “change the parent of own node?” “Change the parent of own node” but not “create a child of own parent node?” These functionalities seem very similar and seems like they could be combined.
    3. Potential new permission: create child node with no parent. Control if a user can create a child while selecting “-- NONE --” as a parent. This could be used to control a user from populating a menu that might show root items. I believe this would be better accomplished through a previous permissions implementation but I’m not sure if that is possible.
  4. Prevent data loss: create a nodehierarchy_update to update permissions changes.
  5. Document changes in README.txt:
    1. Update with new permission names and descriptions.
    2. Detail what permissions are unnecessary when a content type is sets “Show item in menu” to “Always.”

I know it is a whole lot to take in, but I think tackling these will help reduce some of the unnecessary complexities going on. Thoughts?

Comments

#1

This is great. I totally agree that this needs to be cleaned up and you've done a great job of outlining a new plan.

Update permission’s names
create child nodes => create child of own parent node
edit all node parents => change the parent of any node
edit own node parents => change the parent of own node
customize nodehierarchy menus => customize nodehierarchy menu items
create child of any parent => create child of any parent node

Just a couple of notes: First and third suggestion should probably be plural. Otherwise these do seem clearer.

reorder children
view site outline

Agreed. These are holdovers from the last version and have no use anymore.

administer hierarchy

This one I'm leaning towards keeping (but fixing, obviously). I've always found the 'administer pretty much everything' a little too broad for a lot of use cases. Not sure how practical splitting this out is in the real world, but I don't think having the option hurts much.

During some of my research, I came across instances where the permissions did not behave as expected. It seemed like they were typically fringe cases where I was deliberately throwing curveballs.

Agreed. There are definitely some holes in the logic. Lets try and document some of these edge cases so we can get rid of them.

Potentially reduce permissions. Would there ever be a scenario where a user could “create a child of own parent node” but not “change the parent of own node?” “Change the parent of own node” but not “create a child of own parent node?” These functionalities seem very similar and seems like they could be combined.

Yeah this is where it gets tough. Seems like the two types of permission are trying to either restrict which node's parent's you can edit and which nodes you can add children to. The former solves the need to make sure only certain people can chose where nodes go and if they can be moved (as in: bloggers can edit their posts but they can't move them to a different blog). The latter lets people have control over one branch of the hierarchy without being able to edit the whole tree (as in: Jane can add stuff to the 'about us' section but not to 'products'). Right now, though they're named poorly, there are 2 settings that control parents and two that control children. There is the loophole you point out where you can add a child to any parent you own even if you don't have 'create child node' perms. We can close this up by changing the 'create child of ...' permissions to 'add child to ...' and enforce it for existing nodes as well as new ones. This will be clearer and I don't think it removes any functionality. We still have the issue where if a user is allowed to 'edit the parent' of a node they need either 'add to own' or 'add to any' permission to actually be able to do anything (this, I think, is same issue as the redundancy you mention above). We could take away the 'add to own' permission and make it assumed when you have 'edit own' or 'edit any' permission. That would reduce the four main perms to: 'edit parents of own nodes', 'edit parents of any node' and 'add children to any parent' without losing the most important use cases though I do think we lose some edge case setups (but probably ones that don't work properly anyway). Not sure what you'd get if you had 'add node to any parent' but not 'edit own' or 'edit any'. There's also no way in that set up to allow creation of nodes in the hierarchy but not moving them afterwards, but I'm not sure how useful that really is.

Prevent data loss: create a nodehierarchy_update to update permissions changes

Absolutely agree, I'm glad you're planning for this. If we remove perms though there'll be no way to recreate everybody's setups. Any ideas how we deal with those users? We may have to leave them behind for the sake of simplicity, but it'd be nice to at least put in some flexibility (i'm thinking new hooks, that sort of thing) to allow the more exotic use cases to be recreated if we really need them to be.

Thanks for all your feedback. Because of the potential to remove functionality for some people I'd like to have the work for this done in the 3.x branch. That'll give you the power to simplify where needed without breaking people's sites.

Thanks for your hard work!

#2

I unfortunately don't have the time to write out everything that I think at the moment, but in a sense I already have:

#921198: Rework administration and node create/edit with new features and clean design - includes a NH update path for rewriting the variables, a permissions based wouldn't be that difficult
#809468-6: Redesign children form for usability and permission based management - even though the reorder permission isn't used, it should be.
#953614: Create new permission - "Change Default Parent"

Potentially reduce permissions. Would there ever be a scenario where a user could “create a child of own parent node” but not “change the parent of own node?” “Change the parent of own node” but not “create a child of own parent node?” These functionalities seem very similar and seems like they could be combined.

For me, this is a little tricky. I'd have to say yes there's a need, but in a slightly different way. Often times, I have site webmasters who create the initial structure of the site and then later grant edit capabilities to whomever should maintain the page though nodeaccess. These new people are usually never the "author" of the page, but it would be nice if there was a kind of "create a child of editable parent node". We could then, in a sense, restrict the parent dropdown to only the nodes a user can edit.

I also think a "create top level parent" and "change top level parent" should be viable permissions that we should discuss and possibly create.

#3

Version:6.x-2.x-dev» 6.x-3.x-dev

Moving into 3.x branch.

#4

Title:Permission reworking» Permissions Framework