Completed
Branch develop (a31570)
by Mohamed
08:09 queued 04:45
created

IssueController::postNew()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 9
Code Lines 6

Duplication

Lines 9
Ratio 100 %

Code Coverage

Tests 6
CRAP Score 1

Importance

Changes 3
Bugs 0 Features 1
Metric Value
c 3
b 0
f 1
dl 9
loc 9
ccs 6
cts 6
cp 1
rs 9.6667
cc 1
eloc 6
nc 1
nop 3
crap 1
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\Http\Controllers\Project;
13
14
use Illuminate\Http\Request;
15
use Illuminate\Http\Response;
16
use Tinyissue\Form\Comment as CommentForm;
17
use Tinyissue\Form\Issue as IssueForm;
18
use Tinyissue\Http\Controllers\Controller;
19
use Tinyissue\Http\Requests\FormRequest;
20
use Tinyissue\Model\Project;
21
use Tinyissue\Model\Project\Issue;
22
use Tinyissue\Model\Project\Issue\Attachment;
23
use Tinyissue\Model\Project\Issue\Comment;
24
use Tinyissue\Model\User\Activity as UserActivity;
25
26
/**
27
 * IssueController is the controller class for managing request related to projects issues
28
 *
29
 * @author Mohamed Alsharaf <[email protected]>
30
 */
31
class IssueController extends Controller
32
{
33
    /**
34
     * Project issue index page (List project issues)
35
     *
36
     * @param Project     $project
37
     * @param Issue       $issue
38
     * @param CommentForm $form
39
     *
40
     * @return \Illuminate\View\View
41
     */
42 17
    public function getIndex(Project $project, Issue $issue, CommentForm $form)
43
    {
44
        $issue->attachments->each(function (Attachment $attachment) use ($issue) {
0 ignored issues
show
Documentation introduced by
The property attachments does not exist on object<Tinyissue\Model\Project\Issue>. 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...
45 1
            $attachment->setRelation('issue', $issue);
46 17
        });
47 17
        $activities = $issue->activities()->with('activity', 'user', 'comment', 'assignTo',
48 17
            'comment.attachments')->get();
49 17
        $activities->each(function (UserActivity $activity) use ($issue) {
50 17
            $activity->setRelation('issue', $issue);
51 17
        });
52
53
        // Projects should be limited to issue-modify
54 17
        $projects = null;
55 17
        if (!$this->auth->guest() && $this->auth->user()->permission('issue-modify')) {
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...
56 14
            $projects = $this->auth->user()->projects()->get();
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 projects() 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...
57
        }
58
59 17
        return view('project.issue.index', [
60 17
            'issue'       => $issue,
61 17
            'project'     => $project,
62 17
            'commentForm' => $form,
63 17
            'activities'  => $activities,
64 17
            'sidebar'     => 'project',
65 17
            'projects'    => $projects,
66
        ]);
67
    }
68
69
    /**
70
     * Ajax: Assign new user to an issue
71
     *
72
     * @param Issue   $issue
73
     * @param Request $request
74
     *
75
     * @return \Symfony\Component\HttpFoundation\Response
76
     */
77 1 View Code Duplication
    public function postAssign(Issue $issue, Request $request)
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...
78
    {
79 1
        $response = ['status' => false];
80 1
        if ($issue->reassign((int) $request->input('user_id'), $this->auth->user()->id)) {
0 ignored issues
show
Bug introduced by
Accessing id on the interface Illuminate\Contracts\Auth\Authenticatable suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
81 1
            $response['status'] = true;
82
        }
83
84 1
        return response()->json($response);
85
    }
86
87
    /**
88
     * Ajax: save comment
89
     *
90
     * @param Comment $comment
91
     * @param Request $request
92
     *
93
     * @return \Symfony\Component\HttpFoundation\Response
94
     */
95 1
    public function postEditComment(Comment $comment, Request $request)
96
    {
97 1
        $body = '';
98 1
        if ($request->has('body')) {
99 1
            $comment->fill(['comment' => $request->input('body')])->save();
100 1
            $body = \Html::format($comment->comment);
101
        }
102
103 1
        return response()->json(['text' => $body]);
104
    }
105
106
    /**
107
     * To add new comment to an issue
108
     *
109
     * @param Project             $project
110
     * @param Issue               $issue
111
     * @param Comment             $comment
112
     * @param FormRequest\Comment $request
113
     *
114
     * @return \Illuminate\Http\RedirectResponse
115
     */
116 4
    public function getAddComment(Project $project, Issue $issue, Comment $comment, FormRequest\Comment $request)
117
    {
118 4
        $comment->setRelation('project', $project);
119 4
        $comment->setRelation('issue', $issue);
120 4
        $comment->setRelation('user', $this->auth->user());
121 4
        $comment->createComment($request->all());
122
123 4
        return redirect($issue->to() . '#comment' . $comment->id)
124 4
            ->with('notice', trans('tinyissue.your_comment_added'));
125
    }
126
127
    /**
128
     * Ajax: to delete a comment
129
     *
130
     * @param Comment $comment
131
     *
132
     * @return \Symfony\Component\HttpFoundation\Response
133
     */
134 1
    public function getDeleteComment(Comment $comment)
135
    {
136 1
        $comment->deleteComment();
137
138 1
        return response()->json(['status' => true]);
139
    }
140
141
    /**
142
     * New issue form
143
     *
144
     * @param Project   $project
145
     * @param IssueForm $form
146
     *
147
     * @return \Illuminate\View\View
148
     */
149 5
    public function getNew(Project $project, IssueForm $form)
150
    {
151 5
        return view('project.issue.new', [
152 5
            'project' => $project,
153 5
            'form'    => $form,
154 5
            'sidebar' => 'project',
155
        ]);
156
    }
157
158
    /**
159
     * To create a new issue
160
     *
161
     * @param Project           $project
162
     * @param Issue             $issue
163
     * @param FormRequest\Issue $request
164
     *
165
     * @return \Illuminate\Http\RedirectResponse
166
     */
167 2 View Code Duplication
    public function postNew(Project $project, Issue $issue, FormRequest\Issue $request)
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...
168
    {
169 2
        $issue->setRelation('project', $project);
170 2
        $issue->setRelation('user', $this->auth->user());
171 2
        $issue->createIssue($request->all());
172
173 2
        return redirect($issue->to())
174 2
            ->with('notice', trans('tinyissue.issue_has_been_created'));
175
    }
176
177
    /**
178
     * Edit an existing issue form
179
     *
180
     * @param Project   $project
181
     * @param Issue     $issue
182
     * @param IssueForm $form
183
     *
184
     * @return \Illuminate\View\View
185
     */
186 2
    public function getEdit(Project $project, Issue $issue, IssueForm $form)
187
    {
188
        // Cannot edit closed issue
189 2
        if ($issue->status == Issue::STATUS_CLOSED) {
190 1
            return redirect($issue->to())
0 ignored issues
show
Bug Best Practice introduced by
The return type of return redirect($issue->...t_edit_closed_issue')); (Illuminate\Http\RedirectResponse) is incompatible with the return type documented by Tinyissue\Http\Controlle...ssueController::getEdit of type Illuminate\View\View.

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...
191 1
                ->with('notice', trans('tinyissue.cant_edit_closed_issue'));
192
        }
193
194 2
        return view('project.issue.edit', [
195 2
            'issue'   => $issue,
196 2
            'project' => $project,
197 2
            'form'    => $form,
198 2
            'sidebar' => 'project',
199
        ]);
200
    }
201
202
    /**
203
     * To update an existing issue details
204
     *
205
     * @param Project           $project
206
     * @param Issue             $issue
207
     * @param FormRequest\Issue $request
208
     *
209
     * @return \Illuminate\Http\RedirectResponse
210
     */
211 1 View Code Duplication
    public function postEdit(Project $project, Issue $issue, FormRequest\Issue $request)
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...
212
    {
213 1
        $issue->setRelation('project', $project);
214 1
        $issue->setRelation('updatedBy', $this->auth->user());
215 1
        $issue->updateIssue($request->all());
216
217 1
        return redirect($issue->to())
218 1
            ->with('notice', trans('tinyissue.issue_has_been_updated'));
219
    }
220
221
    /**
222
     * To close or reopen an issue
223
     *
224
     * @param Project $project
225
     * @param Issue   $issue
226
     * @param int     $status
227
     *
228
     * @return \Illuminate\Http\RedirectResponse
229
     */
230 2
    public function getClose(Project $project, Issue $issue, $status = 0)
231
    {
232 2
        if ($status == 0) {
233 2
            $message = trans('tinyissue.issue_has_been_closed');
234
        } else {
235 1
            $message = trans('tinyissue.issue_has_been_reopened');
236
        }
237
238 2
        $issue->setRelation('project', $project);
239 2
        $issue->changeStatus($status, $this->auth->user()->id);
0 ignored issues
show
Bug introduced by
Accessing id on the interface Illuminate\Contracts\Auth\Authenticatable suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
240
241 2
        return redirect($issue->to())
242 2
            ->with('notice', $message);
243
    }
244
245
    /**
246
     * To upload an attachment file
247
     *
248
     * @param Project    $project
249
     * @param Attachment $attachment
250
     * @param Request    $request
251
     *
252
     * @return \Symfony\Component\HttpFoundation\Response
253
     */
254 3
    public function postUploadAttachment(Project $project, Attachment $attachment, Request $request)
255
    {
256
        try {
257 3
            if (!$this->auth->user()->permission('project-all')) {
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...
258
                abort(404);
259
            }
260
261 3
            $attachment->upload($request->all(), $project, $this->auth->user());
0 ignored issues
show
Documentation introduced by
$this->auth->user() is of type object<Illuminate\Contra...h\Authenticatable>|null, but the function expects a object<Tinyissue\Model\User>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
262
263
            $response = [
264
                'upload' => [
265
                    [
266 3
                        'name'   => $attachment->filename,
267 3
                        'size'   => $attachment->filesize,
268 3
                        'fileId' => $attachment->id,
269
                    ],
270
                ],
271
            ];
272
        } catch (\Exception $exception) {
273
            $file = $request->file('upload');
274
275
            $response = array(
276
                'status' => false,
277
                'name'   => $file->getClientOriginalName(),
278
                'error'  => $exception->getMessage(),
279
                'trace'  => $exception->getTraceAsString(),
280
            );
281
        }
282
283 3
        return response()->json($response);
284
    }
285
286
    /**
287
     * Ajax: to remove an attachment file
288
     *
289
     * @param Project    $project
290
     * @param Attachment $attachment
291
     * @param Request    $request
292
     *
293
     * @return \Symfony\Component\HttpFoundation\Response
294
     */
295 1
    public function postRemoveAttachment(Project $project, Attachment $attachment, Request $request)
296
    {
297 1
        $attachment->remove($request->all(), $project, $this->auth->user());
0 ignored issues
show
Documentation introduced by
$this->auth->user() is of type object<Illuminate\Contra...h\Authenticatable>|null, but the function expects a object<Tinyissue\Model\User>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
298
299 1
        return response()->json(['status' => true]);
300
    }
301
302
    /**
303
     * Display an attachment file such as image
304
     *
305
     * @param Project    $project
306
     * @param Issue      $issue
307
     * @param Attachment $attachment
308
     * @param Request    $request
309
     *
310
     * @return Response
311
     */
312 2
    public function getDisplayAttachment(Project $project, Issue $issue, Attachment $attachment, Request $request)
313
    {
314 2
        $issue->setRelation('project', $project);
315 2
        $attachment->setRelation('issue', $issue);
316
317 2
        $path = config('tinyissue.uploads_dir') . '/' . $issue->project_id . '/' . $attachment->upload_token . '/' . $attachment->filename;
318 2
        $storage = \Storage::disk('local');
319 2
        $length = $storage->size($path);
320 2
        $time = $storage->lastModified($path);
321 2
        $type = $storage->getDriver()->getMimetype($path);
322
323 2
        $response = new Response();
324 2
        $response->setEtag(md5($time . $path));
325 2
        $response->setExpires(new \DateTime('@' . ($time + 60)));
326 2
        $response->setLastModified(new \DateTime('@' . $time));
327 2
        $response->setPublic();
328 2
        $response->setStatusCode(200);
329
330 2
        $response->header('Content-Type', $type);
331 2
        $response->header('Content-Length', $length);
332 2
        $response->header('Content-Disposition', 'inline; filename="' . $attachment->filename . '"');
333 2
        $response->header('Cache-Control', 'must-revalidate');
334
335 2
        if ($response->isNotModified($request)) {
336
            // Return empty response if not modified
337
            return $response;
338
        }
339
340
        // Return file if first request / modified
341 2
        $response->setContent($storage->get($path));
342
343 2
        return $response;
344
    }
345
346
    /**
347
     * Download an attachment file
348
     *
349
     * @param Project    $project
350
     * @param Issue      $issue
351
     * @param Attachment $attachment
352
     *
353
     * @return \Symfony\Component\HttpFoundation\BinaryFileResponse
354
     */
355 1
    public function getDownloadAttachment(Project $project, Issue $issue, Attachment $attachment)
356
    {
357 1
        $issue->setRelation('project', $project);
358 1
        $attachment->setRelation('issue', $issue);
359
360 1
        $path = config('filesystems.disks.local.root') . '/' . config('tinyissue.uploads_dir') . '/' . $issue->project_id . '/' . $attachment->upload_token . '/' . $attachment->filename;
361
362 1
        return response()->download($path, $attachment->filename);
363
    }
364
365
    /**
366
     * Ajax: move an issue to another project
367
     *
368
     * @param Issue   $issue
369
     * @param Request $request
370
     *
371
     * @return \Symfony\Component\HttpFoundation\Response
372
     */
373 1
    public function postChangeProject(Issue $issue, Request $request)
374
    {
375 1
        $issue->changeProject($request->input('project_id'));
0 ignored issues
show
Documentation introduced by
$request->input('project_id') is of type string|array, but the function expects a integer.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
376
377 1
        return response()->json(['status' => true, 'url' => $issue->to()]);
378
    }
379
}
380