Replace spree roles #13090
Merged
Replace spree roles #13090
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mkllnk
added
the
technical changes only
dacook
reviewed
Jan 24, 2025
| @@ -235,8 +235,6 @@ class GatewayWithPassword < PaymentMethod | |||
| let(:user) do | |||
| new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', | |||
| password_confirmation: 'blahblah', ) | |||
| # for some reason unbeknown to me, this new user gets admin permissions by default. | |||
| new_user.spree_roles = [] | |||
There was a problem hiding this comment.
It's reassuring to see this is no longer necessary!
| spree_put :update, id: test_user.id, user: { show_api_key_view: true } | ||
| expect(response).to redirect_to spree.edit_admin_user_path(test_user) | ||
| end | ||
|
|
||
| it "re-renders the edit form if error" do | ||
| user.spree_roles << Spree::Role.find_or_create_by(name: 'admin') | ||
| user.update!(admin: true) | ||
| spree_put :update, id: test_user.id, user: { password: "blah", password_confirmation: "" } | ||
|
|
||
| expect(response).to render_template :edit |
There was a problem hiding this comment.
3 out of 4 tests require an admin user, so we could set it up as an admin user in the beginning.
In fact, the fourth test mocks the admin? method anyway..
Maybe I'll submit a commit.
|
Hope you don't mind me joining in! |
dacook
approved these changes
Jan 24, 2025
|
Hey @mkllnk , Tried to access the pages: As a:
Access rights seem to work the same as before. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What? Why?
Since we have only one super admin role, we can replace Spree::Role with a simple boolean flag on the user. This makes two database tables redundant and reduces database queries.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates