Closed MadelineCollier closed 3 weeks ago
Attention: Patch coverage is 98.34711%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 89.24%. Comparing base (
d81043a
) to head (3239325
). Report is 7 commits behind head on main.
Files with missing lines | Patch % | Lines |
---|---|---|
...lib/spree/permission_sets/configuration_display.rb | 60.00% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I'll just fix these failing specs and then it should be good to re-review :)
Updated the screenshots and recordings, thanks for all the time put into many rounds of reviews everyone :) Much appreciated!
Note for Reviewers
This PR is part of this issue: https://github.com/solidusio/solidus/issues/5823
I went with checkboxes instead of the view/edit dropdowns as they were becoming complicated to un-pack on the backend with each select having its own form input and array of
permission_set_ids
.I also added a component for the
CheckboxRow
as that whole chunk of code repeated itself 14 times, and any other methods of handling that repetition (partial, helper) proved awful to work with and messy to implement & maintain.The attached video shows the functionality visually.
https://github.com/user-attachments/assets/07bf8b47-468d-4e0f-ac40-c5b6df23bd51
Screenshots:
Component preview
"All" scope
"Admin" scope
Summary
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: