GitHub Access Token became invalid

It seems like the GitHub access token used for retrieving details about this repository from GitHub became invalid. This might prevent certain types of inspections from being run (in particular, everything related to pull requests).
Please ask an admin of your repository to re-new the access token on this website.
Completed
Push — master ( 99ec42...de979d )
by Robbie
9s
created

WorkflowTransition::getCMSFields()   C

Complexity

Conditions 8
Paths 48

Size

Total Lines 66
Code Lines 44

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 66
rs 6.7081
c 0
b 0
f 0
cc 8
eloc 44
nc 48
nop 0

How to fix   Long Method   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace Symbiote\AdvancedWorkflow\DataObjects;
4
5
use SilverStripe\Forms\CheckboxSetField;
6
use SilverStripe\Forms\DropdownField;
7
use SilverStripe\Forms\FieldList;
8
use SilverStripe\Forms\TabSet;
9
use SilverStripe\Forms\TextField;
10
use SilverStripe\Forms\TreeMultiselectField;
11
use SilverStripe\ORM\ArrayList;
12
use SilverStripe\ORM\DataObject;
13
use SilverStripe\ORM\DB;
14
use SilverStripe\Security\Group;
15
use SilverStripe\Security\Member;
16
use SilverStripe\Security\Permission;
17
use SilverStripe\Security\Security;
18
use Symbiote\AdvancedWorkflow\Forms\AWRequiredFields;
19
20
/**
21
 * A workflow transition.
22
 *
23
 * When used within the context of a workflow, the transition will have its
24
 * "isValid()" method call. This must return true or false to indicate whether
25
 * this transition is valid for the state of the workflow that it a part of.
26
 *
27
 * Therefore, any logic around whether the workflow can proceed should be
28
 * managed within this method.
29
 *
30
 * @method WorkflowAction Action()
31
 * @method WorkflowAction NextAction()
32
 * @author  [email protected]
33
 * @license BSD License (http://silverstripe.org/bsd-license/)
34
 * @package advancedworkflow
35
 */
36
class WorkflowTransition extends DataObject
37
{
38
    private static $db = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
Unused Code introduced by
The property $db is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
39
        'Title'     => 'Varchar(128)',
40
        'Sort'      => 'Int',
41
        'Type'      => "Enum('Active, Passive', 'Active')"
42
    );
43
44
    private static $default_sort = 'Sort';
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
Unused Code introduced by
The property $default_sort is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
45
46
    private static $has_one = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
Unused Code introduced by
The property $has_one is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
47
        'Action' => WorkflowAction::class,
48
        'NextAction' => WorkflowAction::class,
49
    );
50
51
    private static $many_many = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
Unused Code introduced by
The property $many_many is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
52
        'Users'  => Member::class,
53
        'Groups' => Group::class,
54
    );
55
56
    private static $icon = 'symbiote/silverstripe-advancedworkflow:images/transition.png';
0 ignored issues
show
Unused Code introduced by
The property $icon is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
57
58
    private static $table_name = 'WorkflowTransition';
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
Unused Code introduced by
The property $table_name is not used and could be removed.

This check marks private properties in classes that are never used. Those properties can be removed.

Loading history...
59
60
    /**
61
     *
62
     * @var array $extendedMethodReturn A basic extended validation routine method return format
63
     */
64
    public static $extendedMethodReturn = array(
65
        'fieldName'  => null,
66
        'fieldField' => null,
67
        'fieldMsg'   => null,
68
        'fieldValid' => true,
69
    );
70
71
    /**
72
     * Returns true if it is valid for this transition to be followed given the
73
     * current state of a workflow.
74
     *
75
     * @param  WorkflowInstance $workflow
76
     * @return bool
77
     */
78
    public function isValid(WorkflowInstance $workflow)
0 ignored issues
show
Unused Code introduced by
The parameter $workflow is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
79
    {
80
        return true;
81
    }
82
83
    /**
84
     * Before saving, make sure we're not in an infinite loop
85
     */
86
    public function onBeforeWrite()
87
    {
88
        if (!$this->Sort) {
0 ignored issues
show
Documentation introduced by
The property Sort does not exist on object<Symbiote\Advanced...cts\WorkflowTransition>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
89
            $this->Sort = DB::query('SELECT MAX("Sort") + 1 FROM "WorkflowTransition"')->value();
0 ignored issues
show
Documentation introduced by
The property Sort does not exist on object<Symbiote\Advanced...cts\WorkflowTransition>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
90
        }
91
92
        parent::onBeforeWrite();
93
    }
94
95
    /* CMS FUNCTIONS */
96
97
    public function getCMSFields()
0 ignored issues
show
Coding Style introduced by
getCMSFields uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
98
    {
99
        $fields = new FieldList(new TabSet('Root'));
100
        $fields->addFieldToTab('Root.Main', new TextField('Title', $this->fieldLabel('Title')));
101
102
        $filter = '';
103
104
        $reqParent = isset($_REQUEST['ParentID']) ? (int) $_REQUEST['ParentID'] : 0;
105
        $attachTo = $this->ActionID ? $this->ActionID : $reqParent;
0 ignored issues
show
Documentation introduced by
The property ActionID does not exist on object<Symbiote\Advanced...cts\WorkflowTransition>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
106
107
        if ($attachTo) {
108
            $action = DataObject::get_by_id(WorkflowAction::class, $attachTo);
109
            if ($action && $action->ID) {
110
                $filter = '"WorkflowDefID" = '.((int) $action->WorkflowDefID);
111
            }
112
        }
113
114
        $actions = DataObject::get(WorkflowAction::class, $filter);
115
        $options = array();
116
        if ($actions) {
117
            $options = $actions->map();
118
        }
119
120
        $defaultAction = $action ? $action->ID : "";
0 ignored issues
show
Bug introduced by
The variable $action does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
121
122
        $typeOptions = array(
123
            'Active' => _t('WorkflowTransition.Active', 'Active'),
124
            'Passive' => _t('WorkflowTransition.Passive', 'Passive'),
125
        );
126
127
        $fields->addFieldToTab('Root.Main', new DropdownField(
128
            'ActionID',
129
            $this->fieldLabel('ActionID'),
130
            $options,
131
            $defaultAction
132
        ));
133
        $fields->addFieldToTab('Root.Main', $nextActionDropdownField = new DropdownField(
134
            'NextActionID',
135
            $this->fieldLabel('NextActionID'),
136
            $options
137
        ));
138
        $nextActionDropdownField->setEmptyString(_t('WorkflowTransition.SELECTONE', '(Select one)'));
139
        $fields->addFieldToTab('Root.Main', new DropdownField(
140
            'Type',
141
            _t('WorkflowTransition.TYPE', 'Type'),
142
            $typeOptions
143
        ));
144
145
        $members = Member::get();
146
        $fields->findOrMakeTab(
147
            'Root.RestrictToUsers',
148
            _t('WorkflowTransition.TabTitle', 'Restrict to users')
149
        );
150
        $fields->addFieldToTab(
151
            'Root.RestrictToUsers',
152
            new CheckboxSetField('Users', _t('WorkflowDefinition.USERS', 'Restrict to Users'), $members)
153
        );
154
        $fields->addFieldToTab(
155
            'Root.RestrictToUsers',
156
            new TreeMultiselectField('Groups', _t('WorkflowDefinition.GROUPS', 'Restrict to Groups'), Group::class)
157
        );
158
159
        $this->extend('updateCMSFields', $fields);
160
161
        return $fields;
162
    }
163
164
    public function fieldLabels($includerelations = true)
165
    {
166
        $labels = parent::fieldLabels($includerelations);
167
        $labels['Title'] = _t('WorkflowAction.TITLE', 'Title');
168
        $labels['ActionID'] = _t('WorkflowTransition.ACTION', 'Action');
169
        $labels['NextActionID'] = _t('WorkflowTransition.NEXT_ACTION', 'Next Action');
170
171
        return $labels;
172
    }
173
174
    public function getValidator()
175
    {
176
        $required = new AWRequiredFields('Title', 'ActionID', 'NextActionID');
177
        $required->setCaller($this);
178
        return $required;
179
    }
180
181
    public function numChildren()
182
    {
183
        return 0;
184
    }
185
186
    public function summaryFields()
187
    {
188
        return array(
189
            'Title' => $this->fieldLabel('Title')
190
        );
191
    }
192
193
194
    /**
195
     * Check if the current user can execute this transition
196
     *
197
     * @return bool
198
     **/
199
    public function canExecute(WorkflowInstance $workflow)
200
    {
201
        $return = true;
202
        $members = $this->getAssignedMembers();
203
204
        // If not admin, check if the member is in the list of assigned members
205
        if (!Permission::check('ADMIN') && $members->exists()) {
206
            if (!$members->find('ID', Security::getCurrentUser()->ID)) {
207
                $return = false;
208
            }
209
        }
210
211
        if ($return) {
212
            $extended = $this->extend('extendCanExecute', $workflow);
213
            if ($extended) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $extended of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
214
                $return = min($extended);
215
            }
216
        }
217
218
        return $return !== false;
219
    }
220
221
    /**
222
     * Allows users who have permission to create a WorkflowDefinition, to create actions on it too.
223
     *
224
     * @param  Member $member
225
     * @param array $context
226
     * @return bool
227
     */
228
    public function canCreate($member = null, $context = array())
229
    {
230
        return $this->Action()->WorkflowDef()->canCreate($member, $context);
231
    }
232
233
    /**
234
     * @param  Member $member
235
     * @return bool
236
     */
237
    public function canEdit($member = null)
238
    {
239
        return $this->canCreate($member);
240
    }
241
242
    /**
243
     * @param  Member $member
244
     * @return bool
245
     */
246
    public function canDelete($member = null)
247
    {
248
        return $this->canCreate($member);
249
    }
250
251
    /**
252
     * Returns a set of all Members that are assigned to this transition, either directly or via a group.
253
     *
254
     * @return ArrayList
255
     */
256 View Code Duplication
    public function getAssignedMembers()
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
257
    {
258
        $members = ArrayList::create($this->Users()->toArray());
0 ignored issues
show
Documentation Bug introduced by
The method Users does not exist on object<Symbiote\Advanced...cts\WorkflowTransition>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
259
        $groups  = $this->Groups();
0 ignored issues
show
Documentation Bug introduced by
The method Groups does not exist on object<Symbiote\Advanced...cts\WorkflowTransition>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
260
261
        foreach ($groups as $group) {
262
            $members->merge($group->Members());
263
        }
264
265
        $members->removeDuplicates();
266
        return $members;
267
    }
268
269
    /*
270
     * A simple field same-value checker.
271
     *
272
     * @param array $data
273
     * @return array
274
     * @see {@link AWRequiredFields}
275
     */
276
    public function extendedRequiredFieldsNotSame($data = null)
277
    {
278
        $check = array('ActionID','NextActionID');
279
        foreach ($check as $fieldName) {
280
            if (!isset($data[$fieldName])) {
281
                return self::$extendedMethodReturn;
282
            }
283
        }
284
        // Have we found some identical values?
285
        if ($data[$check[0]] == $data[$check[1]]) {
286
            // Used to display to the user, so the first of the array is fine
287
            self::$extendedMethodReturn['fieldName'] = $check[0];
288
            self::$extendedMethodReturn['fieldValid'] = false;
289
            self::$extendedMethodReturn['fieldMsg'] = _t(
290
                'WorkflowTransition.TRANSITIONLOOP',
291
                'A transition cannot lead back to its parent action.'
292
            );
293
        }
294
        return self::$extendedMethodReturn;
295
    }
296
}
297