From edec1d70f1812ad13e71e183e18f303e184484b9 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 29 Jun 2018 15:57:58 +1200 Subject: [PATCH] FIX Admin users can always edit records that have active workflow transitions Also updates some docblocks and syntax for consistency --- src/DataObjects/WorkflowAction.php | 9 ++++ src/Extensions/AdvancedWorkflowExtension.php | 55 ++++++++++++++------ src/Extensions/WorkflowApplicable.php | 14 +++-- tests/DataObjects/WorkflowActionTest.php | 19 +++++++ 4 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 tests/DataObjects/WorkflowActionTest.php diff --git a/src/DataObjects/WorkflowAction.php b/src/DataObjects/WorkflowAction.php index aa6e5025..24c55a2b 100644 --- a/src/DataObjects/WorkflowAction.php +++ b/src/DataObjects/WorkflowAction.php @@ -13,6 +13,8 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\Security\Member; +use SilverStripe\Security\Permission; +use SilverStripe\Security\Security; /** * A workflow action describes a the 'state' a workflow can be in, and @@ -70,11 +72,18 @@ class WorkflowAction extends DataObject * will try and figure out an appropriate value for the actively running workflow * if null is returned from this method. * + * Admin level users can always edit. + * * @param DataObject $target * @return bool */ public function canEditTarget(DataObject $target) { + $currentUser = Security::getCurrentUser(); + if ($currentUser && Permission::checkMember($currentUser, 'ADMIN')) { + return true; + } + return null; } diff --git a/src/Extensions/AdvancedWorkflowExtension.php b/src/Extensions/AdvancedWorkflowExtension.php index 746ba5de..867eb3d0 100644 --- a/src/Extensions/AdvancedWorkflowExtension.php +++ b/src/Extensions/AdvancedWorkflowExtension.php @@ -3,6 +3,7 @@ namespace Symbiote\AdvancedWorkflow\Extensions; use SilverStripe\Control\Controller; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Extension; use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Forms\Form; @@ -23,25 +24,31 @@ */ class AdvancedWorkflowExtension extends Extension { - private static $allowed_actions = array( + private static $allowed_actions = [ 'updateworkflow', 'startworkflow' - ); + ]; + /** + * @param array $data + * @param Form $form + * @param HTTPRequest $request + * @return string|null + */ public function startworkflow($data, $form, $request) { $item = $form->getRecord(); $workflowID = isset($data['TriggeredWorkflowID']) ? intval($data['TriggeredWorkflowID']) : 0; if (!$item || !$item->canEdit()) { - return; + return null; } // Save a draft, if the user forgets to do so $this->saveAsDraftWithAction($form, $item); - $svc = singleton(WorkflowService::class); - $svc->startWorkflow($item, $workflowID); + $service = singleton(WorkflowService::class); + $service->startWorkflow($item, $workflowID); return $this->returnResponse($form); } @@ -55,9 +62,11 @@ public function startworkflow($data, $form, $request) public function updateEditForm(Form $form) { Requirements::javascript('symbiote/silverstripe-advancedworkflow:client/dist/js/advancedworkflow.js'); - $svc = singleton(WorkflowService::class); - $p = $form->getRecord(); - $active = $svc->getWorkflowFor($p); + /** @var WorkflowService $service */ + $service = singleton(WorkflowService::class); + /** @var DataObject|WorkflowApplicable $record */ + $record = $form->getRecord(); + $active = $service->getWorkflowFor($record); if ($active) { $fields = $form->Fields(); @@ -65,7 +74,7 @@ public function updateEditForm(Form $form) $wfFields = $active->getWorkflowFields(); $allowed = array_keys($wfFields->saveableFields()); - $data = array(); + $data = []; foreach ($allowed as $fieldName) { $data[$fieldName] = $current->$fieldName; } @@ -78,7 +87,9 @@ public function updateEditForm(Form $form) $form->loadDataFrom($data); - if (!$p->canEditWorkflow()) { + // Set the form to readonly if the current user doesn't have permission to edit the record, and/or it + // is in a state that requires review + if (!$record->canEditWorkflow()) { $form->makeReadonly(); } @@ -86,8 +97,12 @@ public function updateEditForm(Form $form) } } + /** + * @param Form $form + */ public function updateItemEditForm($form) { + /** @var DataObject $record */ $record = $form->getRecord(); if ($record && $record->hasExtension(WorkflowApplicable::class)) { $actions = $form->Actions(); @@ -104,17 +119,23 @@ public function updateItemEditForm($form) * @param array $data * @param Form $form * @param HTTPRequest $request - * @return string + * @return string|null */ public function updateworkflow($data, Form $form, $request) { - $svc = singleton(WorkflowService::class); - $p = $form->getRecord(); - $workflow = $svc->getWorkflowFor($p); + /** @var WorkflowService $service */ + $service = singleton(WorkflowService::class); + /** @var DataObject $record */ + $record = $form->getRecord(); + $workflow = $service->getWorkflowFor($record); + if (!$workflow) { + return null; + } + $action = $workflow->CurrentAction(); - if (!$p || !$p->canEditWorkflow()) { - return; + if (!$record || !$record->canEditWorkflow()) { + return null; } $allowedFields = $workflow->getWorkflowFields()->saveableFields(); @@ -127,7 +148,7 @@ public function updateworkflow($data, Form $form, $request) } if (isset($data['TransitionID']) && $data['TransitionID']) { - $svc->executeTransition($p, $data['TransitionID']); + $service->executeTransition($record, $data['TransitionID']); } else { // otherwise, just try to execute the current workflow to see if it // can now proceed based on user input diff --git a/src/Extensions/WorkflowApplicable.php b/src/Extensions/WorkflowApplicable.php index 5f363f5c..6d461fb8 100644 --- a/src/Extensions/WorkflowApplicable.php +++ b/src/Extensions/WorkflowApplicable.php @@ -18,12 +18,14 @@ use SilverStripe\Forms\TabSet; use SilverStripe\ORM\CMSPreviewable; use SilverStripe\ORM\DataExtension; +use SilverStripe\ORM\DataList; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; +use Symbiote\AdvancedWorkflow\DataObjects\WorkflowActionInstance; use Symbiote\AdvancedWorkflow\DataObjects\WorkflowDefinition; use Symbiote\AdvancedWorkflow\DataObjects\WorkflowInstance; use Symbiote\AdvancedWorkflow\Services\WorkflowService; -use Symbiote\QueuedJobs\Service\AbstractQueuedJob; +use Symbiote\QueuedJobs\Services\AbstractQueuedJob; /** * DataObjects that have the WorkflowApplicable extension can have a @@ -80,7 +82,7 @@ public function getIsPublishJobRunning() */ public function isPublishJobRunning() { - $propIsSet = $this->getIsPublishJobRunning() ? true : false; + $propIsSet = (bool) $this->getIsPublishJobRunning(); return class_exists(AbstractQueuedJob::class) && $propIsSet; } @@ -351,7 +353,7 @@ public function getWorkflowInstance() /** * Gets the history of a workflow instance * - * @return DataObjectSet + * @return DataList */ public function getWorkflowHistory($limit = null) { @@ -411,6 +413,8 @@ public function canPublish() /** * Can only edit content that's NOT in another person's content changeset + * + * @return bool */ public function canEdit($member) { @@ -420,12 +424,14 @@ public function canEdit($member) } if ($active = $this->getWorkflowInstance()) { - return $active->canEditTarget($this->owner); + return $active->canEditTarget(); } } /** * Can a user edit the current workflow attached to this item? + * + * @return bool */ public function canEditWorkflow() { diff --git a/tests/DataObjects/WorkflowActionTest.php b/tests/DataObjects/WorkflowActionTest.php new file mode 100644 index 00000000..cc1eeff0 --- /dev/null +++ b/tests/DataObjects/WorkflowActionTest.php @@ -0,0 +1,19 @@ +logInWithPermission('ADMIN'); + $action = new WorkflowAction(); + $this->assertTrue($action->canEditTarget(new DataObject)); + } +}