Completed
Push — develop ( b16730...136b9c )
by Mohamed
07:35
created

Issue::setup()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 8
Code Lines 5

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 6
CRAP Score 2

Importance

Changes 1
Bugs 0 Features 0
Metric Value
c 1
b 0
f 0
dl 0
loc 8
ccs 6
cts 6
cp 1
rs 9.4285
cc 2
eloc 5
nc 2
nop 1
crap 2
1
<?php
2
3
/*
4
 * This file is part of the Tinyissue package.
5
 *
6
 * (c) Mohamed Alsharaf <[email protected]>
7
 *
8
 * For the full copyright and license information, please view the LICENSE
9
 * file that was distributed with this source code.
10
 */
11
12
namespace Tinyissue\Form;
13
14
use Tinyissue\Model;
15
16
/**
17
 * Issue is a class to defines fields & rules for add/edit issue form.
18
 *
19
 * @author Mohamed Alsharaf <[email protected]>
20
 */
21
class Issue extends FormAbstract
22
{
23
    /**
24
     * An instance of project model.
25
     *
26
     * @var Model\Project
27
     */
28
    protected $project;
29
30
    /**
31
     * Collection of all tags.
32
     *
33
     * @var \Illuminate\Database\Eloquent\Collection
34
     */
35
    protected $tags = null;
36
37
    /**
38
     * Is issue readonly.
39
     *
40
     * @var bool
41
     */
42
    protected $readOnly = false;
43
44
    /**
45
     * @param string $type
46
     *
47
     * @return \Illuminate\Database\Eloquent\Collection|null
48
     */
49 7 View Code Duplication
    protected function getTags($type = null)
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...
50
    {
51 7
        if ($this->tags === null) {
52 7
            $this->tags = (new Model\Tag())->getGroupTags();
53
        }
54
55 7
        if ($type) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $type of type string|null is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
56 7
            return $this->tags->where('name', $type)->first()->tags;
57
        }
58
59
        return $this->tags;
60
    }
61
62
    /**
63
     * Get issue tag for specific type/group.
64
     *
65
     * @param string $type
66
     *
67
     * @return int
68
     */
69 7
    protected function getIssueTag($type)
70
    {
71 7
        if ($this->isEditing()) {
72 2
            $groupId     = $this->getTags($type)->first()->parent_id;
73 2
            $selectedTag = $this->getModel()->tags->where('parent_id', $groupId);
74
75 2
            if ($selectedTag->count() > 0) {
76
                return $selectedTag->last();
77
            }
78
        }
79
80 7
        return new Model\Tag();
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Tinyissue\Model\Tag(); (Tinyissue\Model\Tag) is incompatible with the return type documented by Tinyissue\Form\Issue::getIssueTag of type integer.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

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

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
81
    }
82
83
    /**
84
     * @param array $params
85
     *
86
     * @return void
87
     */
88 6
    public function setup(array $params)
89
    {
90 6
        $this->project = $params['project'];
91 6
        if (!empty($params['issue'])) {
92 2
            $this->editingModel($params['issue']);
93 2
            $this->readOnly = $this->getModel()->hasReadOnlyTag(auth()->user());
94
        }
95 6
    }
96
97
    /**
98
     * @return array
99
     */
100 6
    public function actions()
101
    {
102 6
        $actions = [];
103
104
        // Check if issue is in readonly tag
105 6
        if (!$this->readOnly) {
106
            $actions = [
107 6
                'submit' => $this->isEditing() ? 'update_issue' : 'create_issue',
108
            ];
109
110 6
            if ($this->isEditing() && auth()->user(Model\Permission::PERM_ISSUE_MODIFY)) {
0 ignored issues
show
Unused Code introduced by
The call to Guard::user() has too many arguments starting with \Tinyissue\Model\Permission::PERM_ISSUE_MODIFY.

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
111 2
                $actions['delete'] = [
112 2
                    'type'         => 'danger_submit',
113 2
                    'label'        => trans('tinyissue.delete_something', ['name' => '#' . $this->getModel()->id]),
114 2
                    'class'        => 'close-issue',
115 2
                    'name'         => 'delete-issue',
116 2
                    'data-message' => trans('tinyissue.delete_issue_confirm'),
117
                ];
118
            }
119
        }
120
121 6
        return $actions;
122
    }
123
124
    /**
125
     * @return array
126
     */
127 6
    public function fields()
128
    {
129 6
        $issueModify = \Auth::user()->permission('issue-modify');
130
131 6
        $fields = [];
132 6
        $fields += $this->readOnlyMessage();
133 6
        $fields += $this->fieldTitle();
134 6
        $fields += $this->fieldBody();
135 6
        $fields += $this->fieldTypeTags();
136
137
        // Only on creating new issue
138 6
        if (!$this->isEditing()) {
139 5
            $fields += $this->fieldUpload();
140
        }
141
142
        // Show fields for users with issue modify permission
143 6
        if ($issueModify) {
144 6
            $fields += $this->issueModifyFields();
145
        }
146
147 6
        return $fields;
148
    }
149
150
    /**
151
     * Return a list of fields for users with issue modify permission.
152
     *
153
     * @return array
154
     */
155 6
    protected function issueModifyFields()
156
    {
157 6
        $fields = [];
158
159 6
        $fields['internal_status'] = [
160
            'type' => 'legend',
161
        ];
162
163
        // Status tags
164 6
        $fields += $this->fieldStatusTags();
165
166
        // Assign users
167 6
        $fields += $this->fieldAssignedTo();
168
169
        // Quotes
170 6
        $fields += $this->fieldTimeQuote();
171
172
        // Resolution tags
173 6
        $fields += $this->fieldResolutionTags();
174
175 6
        return $fields;
176
    }
177
178
    /**
179
     * Returns message about read only issue.
180
     *
181
     * @return array
182
     */
183 6
    protected function readOnlyMessage()
184
    {
185
        return [
186
            'readonly' => [
187 6
                'type'  => 'plaintext',
188 6
                'label' => ' ',
189 6
                'value' => '<div class="alert alert-warning">' . trans('tinyissue.readonly_issue_message') . '</div>',
190
            ],
191
        ];
192
    }
193
194
    /**
195
     * Returns title field.
196
     *
197
     * @return array
198
     */
199 7
    protected function fieldTitle()
200
    {
201
        return [
202
            'title' => [
203
                'type'  => 'text',
204
                'label' => 'title',
205 7
            ],
206
        ];
207
    }
208
209
    /**
210
     * Returns body field.
211
     *
212
     * @return array
213
     */
214 7
    protected function fieldBody()
215
    {
216
        return [
217
            'body' => [
218
                'type'  => 'textarea',
219
                'label' => 'issue',
220 7
            ],
221
        ];
222
    }
223
224
    /**
225
     * Returns status tag field.
226
     *
227
     * @return array
228
     */
229 6
    protected function fieldStatusTags()
230
    {
231 6
        $currentTag = $this->getIssueTag('status');
232
233 6
        if ($currentTag && !$currentTag->canView()) {
0 ignored issues
show
Bug introduced by
The method canView cannot be called on $currentTag (of type integer).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
234
            $tags = [$currentTag];
235
        } else {
236 6
            $tags = $this->getTags('status');
237
        }
238
239 6
        $options = [];
240 6
        foreach ($tags as $tag) {
0 ignored issues
show
Bug introduced by
The expression $tags of type object<Illuminate\Databa...nteger,{"0":"integer"}> is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
241 6
            $options[ucwords($tag->name)] = [
242 6
                'name'      => 'tag_status',
243 6
                'value'     => $tag->id,
244 6
                'data-tags' => $tag->id,
245 6
                'color'     => $tag->bgcolor,
246
            ];
247
        }
248
249 6
        $fields['tag_status'] = [
0 ignored issues
show
Coding Style Comprehensibility introduced by
$fields was never initialized. Although not strictly required by PHP, it is generally a good practice to add $fields = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
250 6
            'label'  => 'status',
251 6
            'type'   => 'radioButton',
252 6
            'radios' => $options,
253 6
            'check'  => $this->getIssueTag('status')->id,
254
        ];
255
256 6
        return $fields;
257
    }
258
259
    /**
260
     * Returns tags field.
261
     *
262
     * @return array
263
     */
264 7
    protected function fieldTypeTags()
265
    {
266 7
        $currentTag = $this->getIssueTag('type');
267
268 7
        if ($currentTag && !$currentTag->canView()) {
0 ignored issues
show
Bug introduced by
The method canView cannot be called on $currentTag (of type integer).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
269
            $tags = [$currentTag];
270
        } else {
271 7
            $tags = $this->getTags('type');
272
        }
273
274 7
        $options = [];
275 7
        foreach ($tags as $tag) {
0 ignored issues
show
Bug introduced by
The expression $tags of type object<Illuminate\Databa...nteger,{"0":"integer"}> is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
276 7
            $options[ucwords($tag->name)] = [
277 7
                'name'      => 'tag_type',
278 7
                'value'     => $tag->id,
279 7
                'data-tags' => $tag->id,
280 7
                'color'     => $tag->bgcolor,
281
            ];
282
        }
283
284 7
        $fields['tag_type'] = [
0 ignored issues
show
Coding Style Comprehensibility introduced by
$fields was never initialized. Although not strictly required by PHP, it is generally a good practice to add $fields = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
285 7
            'label'  => 'type',
286 7
            'type'   => 'radioButton',
287 7
            'radios' => $options,
288 7
            'check'  => $this->getIssueTag('type')->id,
289
        ];
290
291 7
        return $fields;
292
    }
293
294
    /**
295
     * Returns tags field.
296
     *
297
     * @return array
298
     */
299 6
    protected function fieldResolutionTags()
300
    {
301 6
        $currentTag = $this->getIssueTag('resolution');
302
303 6
        if ($currentTag && !$currentTag->canView()) {
0 ignored issues
show
Bug introduced by
The method canView cannot be called on $currentTag (of type integer).

Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects.

Loading history...
304
            $tags = [$currentTag];
305
        } else {
306 6
            $tags = $this->getTags('resolution');
307
        }
308
309
        $options = [
310 6
            trans('tinyissue.none') => [
311
                'name'      => 'tag_resolution',
312
                'value'     => 0,
313
                'data-tags' => 0,
314
                'color'     => '#62CFFC',
315 6
            ],
316
        ];
317 6
        foreach ($tags as $tag) {
0 ignored issues
show
Bug introduced by
The expression $tags of type object<Illuminate\Databa...nteger,{"0":"integer"}> is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
318 6
            $options[ucwords($tag->name)] = [
319 6
                'name'      => 'tag_resolution',
320 6
                'value'     => $tag->id,
321 6
                'data-tags' => $tag->id,
322 6
                'color'     => $tag->bgcolor,
323
            ];
324
        }
325
326 6
        $fields['tag_resolution'] = [
0 ignored issues
show
Coding Style Comprehensibility introduced by
$fields was never initialized. Although not strictly required by PHP, it is generally a good practice to add $fields = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
327 6
            'label'  => 'resolution',
328 6
            'type'   => 'radioButton',
329 6
            'radios' => $options,
330 6
            'check'  => $this->getIssueTag('resolution')->id,
331
        ];
332
333 6
        return $fields;
334
    }
335
336
    /**
337
     * Returns assigned to field.
338
     *
339
     * @return array
340
     */
341 6
    protected function fieldAssignedTo()
342
    {
343
        return [
344
            'assigned_to' => [
345 6
                'type'    => 'select',
346 6
                'label'   => 'assigned_to',
347 6
                'options' => [0 => ''] + $this->project->usersCanFixIssue()->get()->lists('fullname', 'id')->all(),
348 6
                'value'   => (int) $this->project->default_assignee,
349
            ],
350
        ];
351
    }
352
353
    /**
354
     * Returns upload field.
355
     *
356
     * @return array
357
     */
358 6
    protected function fieldUpload()
359
    {
360 6
        $user                      = \Auth::guest() ? new Model\User() : \Auth::user();
361 6
        $fields                    = $this->projectUploadFields('upload', $this->project, $user);
362 6
        $fields['upload']['label'] = 'attachments';
363
364 6
        return $fields;
365
    }
366
367
    /**
368
     * Returns time quote field.
369
     *
370
     * @return array
371
     */
372 6
    protected function fieldTimeQuote()
373
    {
374
        $fields = [
375
            'time_quote' => [
376 6
                'type'     => 'groupField',
377 6
                'label'    => 'quote',
378
                'fields'   => [
379
                    'h'    => [
380 6
                        'type'          => 'number',
381 6
                        'append'        => trans('tinyissue.hours'),
382 6
                        'value'         => $this->extractQuoteValue('h'),
383 6
                        'addGroupClass' => 'col-sm-5 col-md-5 col-lg-4',
384
                    ],
385
                    'm'    => [
386 6
                        'type'          => 'number',
387 6
                        'append'        => trans('tinyissue.minutes'),
388 6
                        'value'         => $this->extractQuoteValue('m'),
389 6
                        'addGroupClass' => 'col-sm-5 col-md-5 col-lg-4',
390
                    ],
391
                    'lock' => [
392 6
                        'type'          => 'checkboxButton',
393 6
                        'label'         => '',
394
                        'noLabel'       => true,
395 6
                        'class'         => 'eee',
396 6
                        'addGroupClass' => 'sss col-sm-12 col-md-12 col-lg-4',
397
                        'checkboxes'    => [
398
                            'Lock Quote' => [
399 6
                                'value'     => 1,
400 6
                                'data-tags' => 1,
401 6
                                'color'     => 'red',
402 6
                                'checked'   => $this->isEditing() && $this->getModel()->isQuoteLocked(),
403
                            ],
404
                        ],
405
                        'grouped'       => true,
406
                    ],
407
                ],
408 6
                'addClass' => 'row issue-quote',
409 6
            ],
410
        ];
411
412
        // If user does not have access to lock quote, then remove the field
413 6
        if (!auth()->user()->permission(Model\Permission::PERM_ISSUE_LOCK_QUOTE)) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Illuminate\Contracts\Auth\Authenticatable as the method permission() does only exist in the following implementations of said interface: Tinyissue\Model\User.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
414 1
            unset($fields['time_quote']['fields']['lock']);
415
416
            // If quote is locked then remove quote fields
417 1
            if ($this->isEditing() && $this->getModel()->isQuoteLocked()) {
418
                return [];
419
            }
420
        }
421
422 6
        return $fields;
423
    }
424
425
    /**
426
     * @return array
427
     */
428 7
    public function rules()
429
    {
430
        $rules = [
431 7
            'title' => 'required|max:200',
432
            'body'  => 'required',
433
        ];
434
435 7
        return $rules;
436
    }
437
438
    /**
439
     * @return string
440
     */
441
    public function getRedirectUrl()
442
    {
443
        if ($this->isEditing()) {
444
            return $this->getModel()->to('edit');
445
        }
446
447
        return 'project/' . $this->project->id . '/issue/new';
448
    }
449
450
    /**
451
     * Extract number of hours, or minutes, or seconds from a quote.
452
     *
453
     * @param string $part
454
     *
455
     * @return float|int
456
     */
457 6
    protected function extractQuoteValue($part)
458
    {
459 6
        if ($this->getModel() instanceof Model\Project\Issue) {
460 2
            $seconds = $this->getModel()->time_quote;
461 2
            if ($part === 'h') {
462 2
                return floor($seconds / 3600);
463
            }
464
465 2
            if ($part === 'm') {
466 2
                return ($seconds / 60) % 60;
467
            }
468
        }
469
470 5
        return 0;
471
    }
472
}
473