New setting - Enable inheritance of permission to a book when assigning to a shelf #1726
Conversation
When ever you assign a book to a shelve and have the "Inherit book permission from shelve" setting enabled, the permissions of the shelve will be copied to the book. "
philjak
changed the title
|
Cant wait for this Setting to become available! :D |
There was a problem hiding this comment.
Thank you once again for this pull request @philjak. I can see why this functionality could be handy but overall I'm not sure this is the right kind of implementation to go after.
By having this as a system setting, I think it could lead people to think the inheritance is live rather than a one-time copy. I'm also generally apprehensive about adding any more system settings without there being a clear reason or benefit to them being a system setting.
I was maybe thinking about having a checkbox for this when creating a book, if creating from a Shelf, since that moves the option closer to the action and re-enforces the idea this is a one-time thing but I think that implementation would also be confusing since only people with permission to "Manage Permissions" will be able to enact such an option.
We have #1091 targeted for the next release which should help in regards to shelf permissions at some level.
I'm tempted to say we don't add anything, upon #1091, for now in regards to shelf permissions. There's a "Permission System Review" item on the road-map. I intend to switch up the permission system a fair bit for that item, so roles (and potentially users) are selected to be overridden instead of having to override permissions for all roles on a shelf/book/chapter/page. That will probably be a good opportunity to look at real shelf permission inheritance.
| $book->permissions()->createMany($shelfPermissions); | ||
| $book->save(); | ||
| $book->rebuildPermissions(); | ||
| } |
There was a problem hiding this comment.
I think this action should be in a different method on the Bookshelf.php model, called from the controller when needed since it adds quite a bit more than just "Appending a book" as the function name implies.
| @@ -40,6 +40,9 @@ | |||
| 'app_disable_comments' => 'Disable Comments', | |||
| 'app_disable_comments_toggle' => 'Disable comments', | |||
| 'app_disable_comments_desc' => 'Disables comments across all pages in the application. <br> Existing comments are not shown.', | |||
| 'app_inherit_from_shelf' => 'Inherit book permission from shelf', | |||
| 'app_inherit_from_shelf_desc' => 'The permission of the shelf will be copied to the book, when assigning it to a shelf. All other permissions of the book will be overwritten.', | |||
| 'app_inherit_from_shelf_toggle' => 'Enable inheritance', | |||
There was a problem hiding this comment.
I think the use of the word "Inheritance" does not communicate that this is a one-time deal and will lead people to think it's fullly active "Live" permission inheritance, which could lead to further confusion.
|
Hey @ssddanbrown - sure. Understand this! In our use case, BookStack will be used for a three-digit number of users. We therefore restrict the creation of shelves to a small number of users anyway in order to maintain clarity. I absolutely understand that a more fine-grained setting, e.g. at shelf or book level, makes more sense. Unfortunately, I'm not yet deep enough in the BookStack code, so I can't offer such a pull request at this time. But I hope to be able to help you at a later time. So please feel free to close this pull request. Best, |
|
As per the discussion above, will go ahead and close this. |
This PR adds a new setting to the settings page. After enabling this setting, a book that will be assigned to a shelf will automatically inherit the permissions of the shelf.