Integrate Role and Permission Management from `solidus_user_roles` (solidus_core step) #5129

rainerdema · 2023-06-07T11:52:03Z

Summary

This PR integrates role and permission management capabilities from the solidus_user_roles gem directly into Solidus, providing greater flexibility and control over user roles and permissions.

Major changes include:

Permission Sets: A new Spree::PermissionSet model has been added to the database schema. This model represents sets of permissions that can be assigned to roles. Each permission set has a name and a set identifier.
Role Permissions: A new Spree::RolePermission model has been added to the database schema. This model represents the association between roles and permission sets.
Data Migration: Pull Request
This rake task creates Spree::PermissionSet records for each existing permission set subclass in Spree::PermissionSets::*, and Spree::RolePermission records for each role's associated permission sets.
(It is designed to work with both new installations and existing Solidus stores that have custom roles and permissions).

bin/rails solidus:import_existing_permission_sets

Backend interface: closed
Admin interface: Will only be merged into nebulab/admin branch

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

kennyadsl

Thanks @rainerdema! I know it's a draft but I was reading and I thought it was worth commenting. Other than that, what about moving the migration part (which seems quite complex) to a subsequent PR?

core/db/migrate/20230607100229_import_existing_permission_sets.rb

waiting-for-dev

Thanks, @the-krg! 🙌 I left some comments. Besides, it looks like a merge commit slipped in the changeset.

waiting-for-dev · 2023-08-30T03:48:17Z

core/app/models/spree/permission_set.rb

+    scope :display_permissions, -> { where('name LIKE ?', '%Display') }
+    scope :management_permissions, -> { where('name LIKE ?', '%Management') }
+
+    scope :custom_permissions, -> {
+      where.not(id: display_permissions)
+      .where.not(id: management_permissions)
+    }


What do you think about testing these scopes?

waiting-for-dev · 2023-08-30T03:48:38Z

core/app/models/spree/role.rb

    has_many :users, through: :role_users

+    scope :non_base_roles, -> { where.not(name: ['admin']) }


Same as above, should we add tests here?

core/app/models/spree/role.rb

waiting-for-dev · 2023-08-30T03:53:39Z

core/db/migrate/20230607100048_create_spree_permission_sets.rb

+      t.string :name
+      t.string :set


I still fail to understand why we need both name and set. Don't they represent the same information? 🤔

Update:

I'm changing the set attribute to name and adding a new attribute named group (or label...? WDYT?).

name will now take place as set was before; (eg.: name: "Spree::PermissionSets::UserDisplay")

group will be used for grouping the permissions; (eg.: Spree::PermissionSet.where(group: "Users") will return all permissions related to Users);

when presenting the permissions for the user when creating a role, we'll displaying the groups with their permissions such as Edit, View or Custom.

To be sure I understand, would different permission sets share the same group? If so, would it be possible to assign an individual permission set to a role instead of the whole group it belongs to?

Yes and yes.

A group may contain multiple permission sets related to one subject.

eg.: <set 1 - group: Products, name: Spree::PermissionSets::ProductDisplay> <set 2 - group: Products, name: Spree::PermissionSets::ProductManagement> <set 3 - group: Products, name: Spree::PermissionSets::CustomPermission>

On the role creation page you should select only one, as stated on the layout. (Also because Management includes the Display permissions, so it's redundant to have both)

Thanks, it makes sense

This commit introduces database schema changes to extend the role and permission management capabilities of Solidus. These changes are derived from the solidus_user_roles gem and are designed to provide more granular control over user roles and permissions. Changes include: * Create Spree Permission Sets: Introduced a new Spree::PermissionSet table in the database. This table will store sets of permissions, each with a name and set identifier, that can be assigned to user roles. * Create Spree Roles Permissions: Introduced a new Spree::RolePermission table in the database. This table will store the associations between roles and permission sets. It includes foreign key references to the Spree::Role and Spree::PermissionSet tables.

rainerdema

Great work! 👏 Thanks, @the-krg! ✅

rainerdema · 2023-09-08T09:26:02Z

core/app/models/spree/role.rb

    has_many :users, through: :role_users

+    scope :non_base_roles, -> { where.not(name: ['admin', 'default']) }


What do you think about storing the reserved role names like 'admin' and 'default' in a constant? 🤔 like RESERVED_ROLES or STATIC_ROLES something like that.

This commit expands the functionality of the `Spree::Role` model to provide more granular and efficient control over user roles and permissions. * Spree::RolePermission * Spree::Role enhancements * Spree::PermissionSet These enhancements are a part of our broader initiative to improve the flexibility and extensibility of role and permission management in Solidus, adapting functionality from the `solidus_user_roles` gem.

This commit introduces a simple rake task that imports the Permission Sets to the DB, then iterates over the defined permissions on AppConfiguration to create the RolePermissions.

elia · 2023-09-12T08:24:26Z

I'm putting this on hold until we figure out a few things on how it will be used in the new admin dashboard.

rainerdema added type:enhancement Proposed or newly added feature changelog:solidus_core Changes to the solidus_core gem labels Jun 7, 2023

rainerdema self-assigned this Jun 7, 2023

kennyadsl reviewed Jun 7, 2023

View reviewed changes

core/db/migrate/20230607100229_import_existing_permission_sets.rb Outdated Show resolved Hide resolved

the-krg force-pushed the rainerd/incorporate-roles-management-code-into-the-core branch 3 times, most recently from 3bbbf0c to 81d1dee Compare August 22, 2023 13:43

the-krg marked this pull request as ready for review August 29, 2023 13:46

the-krg requested a review from a team as a code owner August 29, 2023 13:46

the-krg requested a review from waiting-for-dev August 29, 2023 13:48

the-krg self-assigned this Aug 29, 2023

waiting-for-dev reviewed Aug 30, 2023

View reviewed changes

the-krg force-pushed the rainerd/incorporate-roles-management-code-into-the-core branch 2 times, most recently from fd82a50 to 8d8cbb5 Compare September 4, 2023 21:47

the-krg force-pushed the rainerd/incorporate-roles-management-code-into-the-core branch from 8d8cbb5 to 9348fe7 Compare September 5, 2023 14:24

the-krg requested a review from elia September 7, 2023 06:13

rainerdema commented Sep 8, 2023

View reviewed changes

rainerdema and others added 3 commits September 12, 2023 01:46

Add task to import current permissions

cc9eb18

This commit introduces a simple rake task that imports the Permission Sets to the DB, then iterates over the defined permissions on AppConfiguration to create the RolePermissions.

Assign roles from database on app configuration

f4e0fd2

the-krg force-pushed the rainerd/incorporate-roles-management-code-into-the-core branch from 9348fe7 to f4e0fd2 Compare September 12, 2023 04:48

elia added the hold On hold for a reason different from a breaking change label Sep 12, 2023

elia removed their request for review January 22, 2024 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Role and Permission Management from `solidus_user_roles` (solidus_core step) #5129

Integrate Role and Permission Management from `solidus_user_roles` (solidus_core step) #5129

rainerdema commented Jun 7, 2023 •

edited by the-krg

kennyadsl left a comment •

edited

waiting-for-dev left a comment

waiting-for-dev Aug 30, 2023

waiting-for-dev Aug 30, 2023

waiting-for-dev Aug 30, 2023

the-krg Aug 31, 2023 •

edited

waiting-for-dev Aug 31, 2023

the-krg Aug 31, 2023

waiting-for-dev Sep 1, 2023

rainerdema left a comment

rainerdema Sep 8, 2023

elia commented Sep 12, 2023

		has_many :users, through: :role_users

		scope :non_base_roles, -> { where.not(name: ['admin']) }

Integrate Role and Permission Management from solidus_user_roles (solidus_core step) #5129

Are you sure you want to change the base?

Integrate Role and Permission Management from solidus_user_roles (solidus_core step) #5129

Conversation

rainerdema commented Jun 7, 2023 • edited by the-krg

Summary

Checklist

kennyadsl left a comment • edited

Choose a reason for hiding this comment

waiting-for-dev left a comment

Choose a reason for hiding this comment

waiting-for-dev Aug 30, 2023

Choose a reason for hiding this comment

waiting-for-dev Aug 30, 2023

Choose a reason for hiding this comment

waiting-for-dev Aug 30, 2023

Choose a reason for hiding this comment

the-krg Aug 31, 2023 • edited

Choose a reason for hiding this comment

waiting-for-dev Aug 31, 2023

Choose a reason for hiding this comment

the-krg Aug 31, 2023

Choose a reason for hiding this comment

waiting-for-dev Sep 1, 2023

Choose a reason for hiding this comment

rainerdema left a comment

Choose a reason for hiding this comment

rainerdema Sep 8, 2023

Choose a reason for hiding this comment

elia commented Sep 12, 2023

Integrate Role and Permission Management from `solidus_user_roles` (solidus_core step) #5129

Integrate Role and Permission Management from `solidus_user_roles` (solidus_core step) #5129

rainerdema commented Jun 7, 2023 •

edited by the-krg

kennyadsl left a comment •

edited

the-krg Aug 31, 2023 •

edited