Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Admin] Introduce role creation #5831

Merged

Conversation

MadelineCollier
Copy link
Contributor

This is basically just a repost of this PR: #5826 with a new base branch.

This PR is for #5823. The update/edit functionality will follow in a second PR to keep things small and easy to review. Currently this is just the index, new, create, and delete methods.

This PR creates a new role management page in the new admin interface, following the existing pattern used for tax categories and refund reasons.

The form is rendered via a modal dialog on the roles list by leveraging Turbo frames. Successful creation leads to a turbo stream page refresh, which updates the existing list preserving the query params and the scroll position, for a consistent UX.

The attached video shows the functionality visually:

New tab nesting, scoping, and role creation:

Screen.Recording.2024-08-14.at.7.05.11.PM.mov

Deletion:

Screen.Recording.2024-08-14.at.7.08.38.PM.mov

Additional Questions

This is the first piece for the new [Admin][Settings] Introduce role creation and modification capability ticket.

Example designs:
Screenshot 2024-08-14 at 7 43 40 PM

Spree::Role currently lacks:

  • Types
  • Descriptions
  • Any validation on "Name" except that it be unique (it is still allowed to be blank).

I am assuming that to support the example designs, I will be adding those attributes to the Spree::Role model in a future PR, but I am unsure whether that should go in solidus_admin or whether that should be added to core.

Additionally, the example designs seem to differentiate between "custom" and "standard" roles. Will we be creating new stock roles with default permissions sets and adding them to core/db/default/spree/roles.rb? If so, what will those default roles and permissions be?

Summary

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

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.

Now instead of a top level Users component, the main landing page is
"Users and Roles" (with the users page being the pre-selected tab, so
the only visual change is a new page header and a new tab component to
swap between "Users" and "Roles".
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.12%. Comparing base (b766363) to head (1fbe6d5).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5831      +/-   ##
==========================================
+ Coverage   89.08%   89.12%   +0.04%     
==========================================
  Files         739      743       +4     
  Lines       17246    17317      +71     
==========================================
+ Hits        15363    15434      +71     
  Misses       1883     1883              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This includes the index, the new/create logic, and the singular or bulk
deletion logic as well as the associated components and specs.
@MadelineCollier MadelineCollier merged commit 02d6092 into solidusio:main Aug 27, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants