-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
refactor: expanded product admin permissions #5428
refactor: expanded product admin permissions #5428
Conversation
Signed-off-by: Erik Kieckhafer <[email protected]>
…ct, where needed Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Didn't test, but code 👍 |
@kieckhafer On it |
I'm trying to test this PR locally cos Mia was having trouble. I got up to the part where I made a new Store Manager w/ the updated Product Admin permissions... and then I tried to log out, by clicking the corner menu button and Log Out. But it immediately logged me back in. I tried opening the site in Safari, Firefox, Chrome, even Vivaldi, but same deal. I then manually deleted all my Session cookies. Now I'm stuck in this Zombie mode, where I cannot access the login / log out buttons at all. Sooo @kieckhafer What's the trick to logging into several different Admin w/ different Users? Use different browsers? Use different browsers, in Incognito mode? Manually delete session cookies? |
As @machikoyasuda mention, I was never able to fully test this because any time I try to turn anything on/off, I get the error below. Independent of selecting "Shop Manager" or no. No errors appear on logs, so I have no idea how to debug this. |
I tested the UI on this and it works for me 👍 |
Impact: major
Type: feature
Issue
We currently use the
createProduct
as a catch-all for any product related admin action. All users should not have permission to do all things with a product.Solution
Granulate permissions so that different users can perform different tasks, and not have all permissions on product admin.
createProduct
permission as is, so any existing user groups withcreateRefund
permission are not affected.product/admin
permission in all places wherecreateProduct
is used. This will provide forward capability to use this permission as a catch-all / super-admin with a better descriptive name.product/admin
in places where deemed appropriate:product/archive
,product/clone
,product/create
,product/publish
, andproduct/update
.Thoughts for discussion
product/clone
andproduct/create
probably could be merged, if that's seen as a better fit, however I can imagine instances where a user should only be allowed to clone a variant, not create a whole new product.createProduct
and swap it forproduct/admin
, instead of just addingproduct/admin
in addition, but this would be a breaking change, and should probably be better addressed in an entire permissions overhaul, rather than this.Breaking changes
None. We keep our existing
createProduct
permission as is. All new permissions are In addition tocreateProduct
.Testing
createProduct
permission (calledAdd product
in our UI), which is the main products permission for the entire appProduct Admin
permission in the new sectionProduct Admin
Archive Product
. See that thearchive
button is missing from the actions dropdownClone Product
, and see that Duplicate is missing from the dropdownCreate product
and see that+
is missing from the variant / option listNote: You must have either
product/admin
,product/update
, orcreateProduct
permission in order to even see a product page. If none of those are active, you'll see a blank page / error. This is consistent with the current app.