backdrop-contrib / paragraphs

Paragraphs module to control your content flow
https://backdropcms.org/project/paragraphs
GNU General Public License v2.0
5 stars 11 forks source link

Issue #143 - Default permission on install and save #151

Closed yorkshire-pudding closed 1 year ago

yorkshire-pudding commented 1 year ago

Fixes #143

laryn commented 1 year ago

@yorkshire-pudding Does this PR overwrite permissions for existing Paragraphs types, or just provide defaults for new Paragraphs types? If the former that may be problematic in the case that someone has configured their Paragraphs type differently than these defaults, and then the permissions get overwritten by a default setting. (I haven't tested yet but I'm wondering about the changes in the install file.)

laryn commented 1 year ago

@yorkshire-pudding Two thoughts I'd love to hear your perspective on:

1) I think we need to use something like $admin_role = config_get('system.core', 'user_admin_role'); rather than assuming administrator as the role.

2) We're using the same logic structures and strings multiple times for the conditional messages, etc. Is there any way to simplify that in a reasonable way to reduce redundancy?

yorkshire-pudding commented 1 year ago

@yorkshire-pudding Two thoughts I'd love to hear your perspective on:

  1. I think we need to use something like $admin_role = config_get('system.core', 'user_admin_role'); rather than assuming administrator as the role.
  2. We're using the same logic structures and strings multiple times for the conditional messages, etc. Is there any way to simplify that in a reasonable way to reduce redundancy?

Thanks for these suggestions @laryn - I hadn't considered the first one but of course, people can choose which role gets the new admin permissions. Regarding the second, yes, that component does make sense to put into a shared function. Two points to note:

laryn commented 1 year ago

Thanks for your persistence and patience @yorkshire-pudding! Merged.