Content Review permission logic

With the new `canReviewContent()` permission checker we only need to check the permission is set for the user and `canBeReviewedBy()` will always check if the page object is due for review by its owner.

Thus removed redundant logic in `canUseReviewContent()` and accordingly renamed the class filename for additional context.
This commit is contained in:
Jared Dreyer 2022-06-21 17:41:20 +12:00
parent beacd13b19
commit 04ddb6b206
5 changed files with 31 additions and 36 deletions

View File

@ -6,7 +6,7 @@ use SilverStripe\Admin\LeftAndMain;
use SilverStripe\Admin\LeftAndMainExtension;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\ContentReview\Forms\ReviewContentHandler;
use SilverStripe\ContentReview\Traits\ReviewPermission;
use SilverStripe\ContentReview\Traits\PermissionChecker;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\HTTPResponse_Exception;
@ -20,7 +20,7 @@ use SilverStripe\Security\Security;
*/
class ContentReviewCMSExtension extends LeftAndMainExtension
{
use ReviewPermission;
use PermissionChecker;
private static $allowed_actions = [
'ReviewContentForm',
@ -50,7 +50,7 @@ class ContentReviewCMSExtension extends LeftAndMainExtension
{
$page = $this->findRecord(['ID' => $id]);
$user = Security::getCurrentUser();
if (!$this->canUseReviewContent($page, $user)) {
if (!$this->isContentReviewable($page, $user)) {
$this->owner->httpError(403, _t(
__CLASS__.'.ErrorItemPermissionDenied',
'It seems you don\'t have the necessary permissions to review this content'

View File

@ -512,11 +512,11 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
*/
public function canBeReviewedBy(Member $member = null)
{
if (!$this->owner->obj("NextReviewDate")->exists()) {
if (!$this->owner->obj('NextReviewDate')->exists()) {
return false;
}
if ($this->owner->obj("NextReviewDate")->InFuture()) {
if ($this->owner->obj('NextReviewDate')->InFuture()) {
return false;
}
@ -542,7 +542,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
return true;
}
// Whether or not a user is allowed to review the content of the page.
// Check whether this user is allowed to review the content of the page.
if ($this->owner->hasMethod("canReviewContent") && !$this->owner->canReviewContent($member)) {
return false;
}

View File

@ -3,7 +3,7 @@
namespace SilverStripe\ContentReview\Forms;
use SilverStripe\ContentReview\Extensions\SiteTreeContentReview;
use SilverStripe\ContentReview\Traits\ReviewPermission;
use SilverStripe\ContentReview\Traits\PermissionChecker;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPResponse;
@ -20,7 +20,7 @@ use SilverStripe\Security\Security;
class ReviewContentHandler
{
use Injectable;
use ReviewPermission;
use PermissionChecker;
/**
* Parent controller for this form
@ -127,6 +127,6 @@ class ReviewContentHandler
return false;
}
return $this->canUseReviewContent($record, Security::getCurrentUser());
return $this->isContentReviewable($record, Security::getCurrentUser());
}
}

View File

@ -0,0 +1,22 @@
<?php
namespace SilverStripe\ContentReview\Traits;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\ORM\DataObject;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;
trait PermissionChecker
{
/**
* Checks the user has been granted special permission to review the content of the page
* if not fallback to canEdit() permission.
*/
protected function isContentReviewable(DataObject $record, ?Member $user = null): bool
{
return $record->hasMethod('canReviewContent')
? $record->canReviewContent($user)
: $record->canEdit();
}
}

View File

@ -1,27 +0,0 @@
<?php
namespace SilverStripe\ContentReview\Traits;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\ORM\DataObject;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;
trait ReviewPermission
{
/**
* Whether or not a user is allowed use content review
*/
protected function canUseReviewContent(DataObject $record, ?Member $user = null): bool
{
// Whether or not the user is a reviewer. User must be allowed to view the page
$isReviewer = $record->canView($user) &&
$record->hasMethod('canBeReviewedBy') &&
$record->canBeReviewedBy($user);
// Whether or not the user is allowed to review the content of the page
// Fallback to canEdit as it the original implementation
$canEdit = $record->hasMethod('canReviewContent') ? $record->canReviewContent($user) : $record->canEdit();
return $canEdit || $isReviewer;
}
}