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 #5826

Closed
wants to merge 2 commits into from
Closed

Conversation

MadelineCollier
Copy link
Contributor

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".
This includes the index, the new/create logic, and the singular or bulk
deletion logic as well as the associated components and specs.
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.08%. Comparing base (2ae2bfd) to head (12963d8).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5826      +/-   ##
==========================================
+ Coverage   89.03%   89.08%   +0.04%     
==========================================
  Files         737      741       +4     
  Lines       17188    17259      +71     
==========================================
+ Hits        15304    15375      +71     
  Misses       1884     1884              

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

@MadelineCollier MadelineCollier marked this pull request as ready for review August 14, 2024 18:20
@MadelineCollier MadelineCollier requested a review from a team as a code owner August 14, 2024 18:20
@MadelineCollier MadelineCollier changed the title introduce role creation [Admin] Introduce role creation Aug 15, 2024
@MadelineCollier
Copy link
Contributor Author

Closing in favour of #5831 which has the correct base branch

@MadelineCollier MadelineCollier deleted the introduce-role-creation branch August 19, 2024 10:23
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.

1 participant