Merge pull request #371 from creative-commoners/pulls/5.0/admins-can-…

…always-edit

FIX Admin users can always edit records that have active workflow transitions

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;
     }
 

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,17 +62,19 @@ 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();
             $current = $active->CurrentAction();
             $wfFields = $active->getWorkflowFields();
 
             $allowed = array_keys($wfFields->saveableFields());
-            $data = array();
+            $data = [];
             foreach ($allowed as $fieldName) {
                 $data[$fieldName] = $current->$fieldName;
             }
@@ -78,16 +87,22 @@ 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();
             }
 
             $this->owner->extend('updateWorkflowEditForm', $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

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()
     {

tests/DataObjects/WorkflowActionTest.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace Symbiote\AdvancedWorkflow\Tests\DataObjects;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\DataObject;
+use Symbiote\AdvancedWorkflow\DataObjects\WorkflowAction;
+
+class WorkflowActionTest extends SapphireTest
+{
+    protected $usesDatabase = true;
+
+    public function testAdminUsersCanAlwaysEdit()
+    {
+        $this->logInWithPermission('ADMIN');
+        $action = new WorkflowAction();
+        $this->assertTrue($action->canEditTarget(new DataObject));
+    }
+}