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

ENH Add permissions for Content Editor #120

Conversation

sabina-talipova
Copy link
Contributor

@sabina-talipova sabina-talipova commented Aug 29, 2022

Description

Add $required_permission_codes for 'CMS_ACCESS_CMSMain' in ElementalBehatTestAdmin class to support new changes in behat tests for Elemental module.

Parent issue


public function canView($member = null)
{
return Permission::check('ELEMENTAL_EDIT', 'any', $member);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new permission, just use ElementalBehatTestAdmin::getRequiredPermissions() instead

Suggested change
return Permission::check('ELEMENTAL_EDIT', 'any', $member);
return Permission::check(ElementalBehatTestAdmin::getRequiredPermissions(), 'any', $member);

You'll need to update the others and remove providePermissions() as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE

@@ -14,4 +14,7 @@ class ElementalBehatTestAdmin extends ModelAdmin
private static $managed_models = [
ElementalBehatTestObject::class,
];

private static $required_permission_codes = 'ELEMENTAL_EDIT';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for this - LeftAndMain::providePermissions() creates a new permission for each ModelAdmin subclass automagically.
The correct permission code to use in behat tests for this admin section is CMS_ACCESS_SilverStripe\FrameworkTest\Elemental\Admin\ElementalBehatTestAdmin

Using this means we're also indirectly testing that the LeftAndMain::providePermissions() and LeftAndMain::getRequiredPermission() logic works as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave private static $required_permission_codes = 'ELEMENTAL_EDIT' instead of using CMS_ACCESS_SilverStripe\FrameworkTest\Elemental\Admin\ElementalBehatTestAdmin, since it's more readable in the behat test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, I'll rename it to give it more obvious meaning.

code/Company.php Show resolved Hide resolved
@sabina-talipova sabina-talipova force-pushed the pulls/4/review-behat-tests branch from 5bb9f5b to d070360 Compare September 1, 2022 02:37
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing locally, I can't create a new elemental block on an ElementalBehatTestObject if I only have ELEMENTAL_DATAOBJECT_EDIT permissions. Is that expected?

@sabina-talipova sabina-talipova force-pushed the pulls/4/review-behat-tests branch from d070360 to 69fcc2f Compare September 6, 2022 22:55
@GuySartorelli GuySartorelli merged commit dbf6dfe into silverstripe:0.4 Sep 6, 2022
@GuySartorelli GuySartorelli deleted the pulls/4/review-behat-tests branch September 6, 2022 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants