Create ACL for Klaviyo config settings

klaviyo / magento2-klaviyo

37 stars 51 forks source link

Create ACL for Klaviyo config settings #264

Closed snoop0x7b closed 6 months ago

snoop0x7b commented 9 months ago

Description

This pull request creates the ACL that's raised in this issue https://github.com/klaviyo/magento2-klaviyo/issues/160

It creates permissions for each klaviyo configuration area.

Manual Testing Steps

php bin/magento cache:clean
Log into the admin panel
Go to system -> User Roles
Check whether the new klaviyo roles appear (control+f klaviyo)

Pre-Submission Checklist:

[ ] You've updated the CHANGELOG following the steps here
[ ] Internal Only - If this is a release, please confirm the following:
- [ ] The links in the changelog have been updated to point towards the new versions
- [ ] The version has been incremented in the following places: module.xml and composer.json

NOTE: Please use the Changelogger cli tool to manage versioned file upgrades.

cykolln commented 9 months ago

@snoop0x7b Thanks for contributing! This won't work as expected because all sections except General use the same resource id Klaviyo_Reclaim::klaviyo_reclaim when we defined them in system.xml. We should separate these out to match what you have in acl.xml, this should be good to go if you:

update system.xml to reference the resource ids you defined in acl.xml, for example: update the <resource> tag for section klaviyo_reclaim_newsletter to be Klaviyo_Reclaim::klaviyo_reclaim_newsletter as defined in acl.xml

cykolln commented 7 months ago

@snoop0x7b We are looking to release a new version of the module in December and can include these changes once the feedback above has been addressed. Thanks!

snoop0x7b commented 7 months ago

@snoop0x7b We are looking to release a new version of the module in December and can include these changes once the feedback above has been addressed. Thanks!

Thanks! I'll take a look and make those changes! Sorry it took me a while to get to this.

snoop0x7b commented 7 months ago

Thanks for the feedback on this, I have made the requested updates!

cykolln commented 6 months ago

@snoop0x7b Looks good to me. Can you please update the changelog and then I can approve!

cykolln commented 6 months ago

@snoop0x7b We want to get this in the next release so i'm going to go ahead and merge this in and we will update the changelog in a separate PR to reflect. Thanks for contributing!