Test Setup Failed
Push — feature/word_counter ( e4aaca...45e6a8 )
by Yonathan
14:36
created

ApplicationByJobController::submit()   B

Complexity

Conditions 5
Paths 8

Size

Total Lines 55

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 30

Importance

Changes 0
Metric Value
cc 5
nc 8
nop 2
dl 0
loc 55
ccs 0
cts 22
cp 0
crap 30
rs 8.6707
c 0
b 0
f 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 App\Http\Controllers;
4
5
use Illuminate\Support\Facades\Lang;
6
use Illuminate\Http\Request;
7
use App\Models\Lookup\ApplicationStatus;
8
use App\Models\Lookup\VeteranStatus;
9
use App\Models\Lookup\PreferredLanguage;
10
use App\Models\Lookup\CitizenshipDeclaration;
11
use App\Models\Applicant;
12
use App\Models\JobPoster;
13
use App\Models\JobApplication;
14
use App\Models\JobApplicationAnswer;
15
use App\Models\SkillDeclaration;
16
use App\Models\Skill;
17
use App\Models\Lookup\SkillStatus;
18
use App\Models\Degree;
19
use App\Models\Lookup\CriteriaType;
20
use App\Models\Criteria;
21
use App\Models\Course;
22
use App\Models\WorkExperience;
23
use App\Services\Validation\ApplicationValidator;
24
use Illuminate\Support\Facades\Auth;
25
use Illuminate\Support\Facades\Log;
26
use App\Models\Lookup\ReviewStatus;
27
28
29
class ApplicationByJobController extends Controller
30
{
31
    /**
32
    * Display a listing of the applications for given jobPoster.
33
    *
34
    * @return \Illuminate\Http\Response
35
    */
36
    public function index(JobPoster $jobPoster)
37
    {
38
        $applications = $jobPoster->submitted_applications()
39
            ->with([
40
                'veteran_status',
41
                'citizenship_declaration',
42
                'application_review',
43
                "applicant.user",
44
                "job_poster.criteria",
45
            ])
46
            ->get();
47
        return view('manager/review_applications', [
48
            /*Localization Strings*/
49
            'jobs_l10n' => Lang::get('manager/job_index'),
50
51
            /* Data */
52
            'job' => $jobPoster,
53
            'applications' => $applications,
54
            'review_statuses' => ReviewStatus::all()
55
        ]);
56
    }
57
58
    protected function getApplicationFromJob(JobPoster $jobPoster) {
59
        $application = JobApplication::where('applicant_id', Auth::user()->applicant->id)
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
60
        ->where('job_poster_id', $jobPoster->id)->first();
61
        if ($application == null) {
62
            $application = new JobApplication();
63
            $application->job_poster_id = $jobPoster->id;
64
            $application->applicant_id = Auth::user()->applicant->id;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
65
            $application->application_status_id = ApplicationStatus::where('name', 'draft')->firstOrFail()->id;
66
            $application->save();
67
        }
68
        return $application;
69
    }
70
71
    /**
72
    * Show the form for editing Application basics for the specified job.
73
    *
74
    * @param  \App\Models\JobPoster  $jobPoster
75
    * @return \Illuminate\Http\Response
76
    */
77
    public function edit_basics(JobPoster $jobPoster)
78
    {
79
80
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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
82
        $application = $this->getApplicationFromJob($jobPoster);
83
84
        //Ensure user has permissions to view and update application
85
        $this->authorize('view', $application);
86
        $this->authorize('update', $application);
87
88
        return view('applicant/application_post_01', [
89
90
            /* Application Template Data */
91
            "application_step" => 1,
92
            "application_template" => Lang::get("applicant/application_template"),
93
            "language_options" => PreferredLanguage::all(),
94
            "citizenship_options" => CitizenshipDeclaration::all(),
95
            "veteran_options" => VeteranStatus::all(),
96
            "preferred_language_template" => Lang::get('common/preferred_language'),
97
            "citizenship_declaration_template" => Lang::get('common/citizenship_declaration'),
98
            "veteran_status_template" => Lang::get('common/veteran_status'),
99
100
            /* Job Data */
101
            "job" => $jobPoster,
102
103
            /* Applicant Data */
104
            "applicant" => $applicant,
105
            "job_application" => $application,
106
107
            /* Submission */
108
            "form_submit_action" => route('job.application.update.1', $jobPoster)
109
110
        ]);
111
112
    }
113
114
    /**
115
    * Show the form for editing Application Experience for the specified job.
116
    *
117
    * @param  \App\Models\JobPoster  $jobPoster
118
    * @return \Illuminate\Http\Response
119
    */
120
    public function edit_experience(JobPoster $jobPoster)
121
    {
122
123
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
124
125
        $application = $this->getApplicationFromJob($jobPoster);
126
127
        //Ensure user has permissions to view and update application
128
        $this->authorize('view', $application);
129
        $this->authorize('update', $application);
130
131
        return view('applicant/application_post_02', [
132
133
            /* Application Template Data */
134
            "application_step" => 2,
135
            "application_template" => Lang::get("applicant/application_template"),
136
137
            /* Job Data */
138
            "job" => $jobPoster,
139
140
            /* Applicant Data */
141
            "applicant" => $applicant,
142
            "job_application" => $application,
143
144
            /* Submission */
145
            "form_submit_action" => route('job.application.update.2', $jobPoster)
146
147
        ]);
148
149
    }
150
151
    /**
152
    * Show the form for editing Application Essential Skills for the specified job.
153
    *
154
    * @param  \App\Models\JobPoster  $jobPoster
155
    * @return \Illuminate\Http\Response
156
    */
157 View Code Duplication
    public function edit_essential_skills(JobPoster $jobPoster)
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...
158
    {
159
160
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
161
162
        $application = $this->getApplicationFromJob($jobPoster);
163
164
        //Ensure user has permissions to view and update application
165
        $this->authorize('view', $application);
166
        $this->authorize('update', $application);
167
168
        $criteria = [
169
            'essential' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
170
                return $value->criteria_type->name == 'essential';
171
            }),
172
            'asset' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
173
                return $value->criteria_type->name == 'asset';
174
            }),
175
        ];
176
177
        return view('applicant/application_post_03', [
178
179
            /* Application Template Data */
180
            "application_step" => 3,
181
            "application_template" => Lang::get("applicant/application_template"),
182
183
            /* Job Data */
184
            "job" => $jobPoster,
185
186
            /* Skills Data */
187
            "skills" => Skill::all(),
188
            "skill_template" => Lang::get("common/skills"),
189
            "criteria" => $criteria,
190
191
            /* Applicant Data */
192
            "applicant" => $applicant,
193
            "job_application" => $application,
194
195
            /* Submission */
196
            "form_submit_action" => route('job.application.update.3', $jobPoster)
197
198
        ]);
199
200
    }
201
202
    /**
203
    * Show the form for editing Application Asset Skills for the specified job.
204
    *
205
    * @param  \App\Models\JobPoster  $jobPoster
206
    * @return \Illuminate\Http\Response
207
    */
208 View Code Duplication
    public function edit_asset_skills(JobPoster $jobPoster)
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...
209
    {
210
211
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
212
213
        $application = $this->getApplicationFromJob($jobPoster);
214
215
        //Ensure user has permissions to view and update application
216
        $this->authorize('view', $application);
217
        $this->authorize('update', $application);
218
219
        $criteria = [
220
            'essential' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
221
                return $value->criteria_type->name == 'essential';
222
            }),
223
            'asset' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
224
                return $value->criteria_type->name == 'asset';
225
            }),
226
        ];
227
228
        return view('applicant/application_post_04', [
229
230
            /* Application Template Data */
231
            "application_step" => 4,
232
            "application_template" => Lang::get("applicant/application_template"),
233
234
            /* Job Data */
235
            "job" => $jobPoster,
236
237
            /* Skills Data */
238
            "skills" => Skill::all(),
239
            "skill_template" => Lang::get("common/skills"),
240
            "criteria" => $criteria,
241
242
            /* Applicant Data */
243
            "applicant" => $applicant,
244
            "job_application" => $application,
245
246
            /* Submission */
247
            "form_submit_action" => route('job.application.update.4', $jobPoster)
248
249
        ]);
250
    }
251
252
    /**
253
    * Show the Application Preview for the application for the specified job.
254
    *
255
    * @param  \App\Models\JobPoster  $jobPoster
256
    * @return \Illuminate\Http\Response
257
    */
258
    public function preview(JobPoster $jobPoster) {
259
260
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
261
262
        $application = $this->getApplicationFromJob($jobPoster);
263
264
        $this->authorize('view', $application);
265
266
        $criteria = [
267
            'essential' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
268
                return $value->criteria_type->name == 'essential';
269
            }),
270
            'asset' => $jobPoster->criteria->filter(function($value, $key) {
0 ignored issues
show
Unused Code introduced by
The parameter $key 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...
271
                return $value->criteria_type->name == 'asset';
272
            }),
273
        ];
274
275
        return view('applicant/application_post_05', [
276
277
            /* Application Template Data */
278
            "application_step" => 5,
279
            "application_template" => Lang::get("applicant/application_template"),
280
            "preferred_language_template" => Lang::get('common/preferred_language'),
281
            "citizenship_declaration_template" => Lang::get('common/citizenship_declaration'),
282
            "veteran_status_template" => Lang::get('common/veteran_status'),
283
284
            /* Job Data */
285
            "job" => $jobPoster,
286
287
            /* Skills Data */
288
            "skills" => Skill::all(),
289
            "skill_template" => Lang::get("common/skills"),
290
            "criteria" => $criteria,
291
292
            /* Applicant Data */
293
            "applicant" => $applicant,
294
            "job_application" => $application,
295
        ]);
296
    }
297
298
    /**
299
    * Show the Confirm Submit page for the application for the specified job.
300
    *
301
    * @param  \App\Models\JobPoster  $jobPoster
302
    * @return \Illuminate\Http\Response
303
    */
304
    public function confirm(JobPoster $jobPoster)
305
    {
306
307
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
Unused Code introduced by
$applicant is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
308
309
        $application = $this->getApplicationFromJob($jobPoster);
310
311
        $this->authorize('update', $application);
312
313
        return view('applicant/application_post_06', [
314
            /* Application Template Data */
315
            "application_step" => 6,
316
            "application_template" => Lang::get("applicant/application_template"),
317
318
            /* Used by tracker partial */
319
            "job" => $jobPoster,
320
            "job_application" => $application,
321
322
            /* Submission */
323
            "form_submit_action" => route('job.application.submit', $jobPoster)
324
        ]);
325
    }
326
327
    /**
328
    * Show the application submission information.
329
    *
330
    * @param  \App\Models\JobPoster  $jobPoster
331
    * @return \Illuminate\Http\Response
332
    */
333
    public function complete(JobPoster $jobPoster) {
334
335
        /* Include Applicant Data */
336
337
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
338
339
        /* Include Application Data */
340
341
        $application = $this->getApplicationFromJob($jobPoster);
342
343
        //Ensure user has permissions to view application
344
        $this->authorize('view', $application);
345
346
        /* Return the Completion View */
347
348
        return view('applicant/application_post_complete', [
349
350
            /* Application Template Data */
351
            "application_template" => Lang::get("applicant/application_template"),
352
353
            /* Job Data */
354
            "job" => $jobPoster,
355
356
            /* Applicant Data */
357
            "applicant" => $applicant,
358
            "job_application" => $application
359
360
        ]);
361
    }
362
363
    /**
364
    * Update the Application Basics in storage for the specified job.
365
    *
366
    * @param  \Illuminate\Http\Request  $request
367
    * @param  \App\Models\JobPoster  $jobPoster
368
    * @return \Illuminate\Http\Response
369
    */
370
    public function update_basics(Request $request, JobPoster $jobPoster)
371
    {
372
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
Unused Code introduced by
$applicant is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
373
        $application = $this->getApplicationFromJob($jobPoster);
374
375
        //Ensure user has permissions to update this application
376
        $this->authorize('update', $application);
377
378
        $application->fill([
379
            'citizenship_declaration_id' => $request->input('citizenship_declaration_id'),
380
            'veteran_status_id' => $request->input('veteran_status_id'),
381
            'preferred_language_id' => $request->input('preferred_language_id'),
382
            'language_requirement_confirmed' => $request->input('language_requirement_confirmed')
383
        ]);
384
        $application->save();
385
386
        $questions = $jobPoster->job_poster_questions;
387
        $questionsInput = $request->input('questions');
388
        foreach($questions as $question) {
389
            $answer = null;
390
            if (isset($questionsInput[$question->id])) {
391
                $answer = $questionsInput[$question->id];
392
            }
393
            $answerObj = $application->job_application_answers
394
            ->firstWhere('job_poster_question_id', $question->id);
395
            if ($answerObj == null) {
396
                $answerObj = new JobApplicationAnswer();
397
                $answerObj->job_poster_question_id = $question->id;
398
                $answerObj->job_application_id = $application->id;
399
            }
400
            $answerObj->answer = $answer;
401
            $answerObj->save();
402
        }
403
404
        //Redirect to correct page
405
        switch($request->input('submit')) {
406
            case 'save_and_quit':
407
            case 'previous':
408
            return redirect()->route('applications.index');
0 ignored issues
show
Bug Best Practice introduced by
The return type of return redirect()->route('applications.index'); (Illuminate\Http\RedirectResponse) is incompatible with the return type documented by App\Http\Controllers\App...ntroller::update_basics of type Illuminate\Http\Response.

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...
409
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
410
            case 'save_and_continue':
411
            case 'next':
412
            return redirect()->route('job.application.edit.2', $jobPoster);
0 ignored issues
show
Bug Best Practice introduced by
The return type of return redirect()->route...n.edit.2', $jobPoster); (Illuminate\Http\RedirectResponse) is incompatible with the return type documented by App\Http\Controllers\App...ntroller::update_basics of type Illuminate\Http\Response.

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...
413
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
414
            default:
415
            return redirect()->back()->withInput();
416
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
417
        }
418
    }
419
420
    /**
421
    * Update the Application Basics in storage for the specified job.
422
    *
423
    * @param  \Illuminate\Http\Request  $request
424
    * @param  \App\Models\JobPoster  $jobPoster
425
    * @return \Illuminate\Http\Response
426
    */
427
    public function update_experience(Request $request, JobPoster $jobPoster)
428
    {
429
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
430
        $application = $this->getApplicationFromJob($jobPoster);
431
432
        //Ensure user has permissions to update this application
433
        $this->authorize('update', $application);
434
435
        // Record that the user has saved their experience for this application
436
        $application->experience_saved = true;
437
        $application->save();
438
439
        $degrees = $request->input('degrees');
440
441
        $validatedDegreeData = $request->validate([
0 ignored issues
show
Unused Code introduced by
$validatedDegreeData is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
442
            'degrees.new.*.degree_type_id' => 'required',
443
            'degrees.new.*.area_of_study'  => 'required',
444
            'degrees.new.*.institution'    => 'required',
445
            'degrees.new.*.thesis'         => 'nullable',
446
            'degrees.new.*.start_date'     => 'required|date',
447
            'degrees.new.*.end_date'       => 'required|date',
448
        ]);
449
450
        //Save new degrees
451 View Code Duplication
        if (isset($degrees['new'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
452
            foreach($degrees['new'] as $degreeInput) {
453
                $degree = new Degree();
454
                $degree->applicant_id = $applicant->id;
455
                $degree->fill([
456
                    'degree_type_id' => $degreeInput['degree_type_id'],
457
                    'area_of_study' => $degreeInput['area_of_study'],
458
                    'institution' => $degreeInput['institution'],
459
                    'thesis' => $degreeInput['thesis'],
460
                    'start_date' => $degreeInput['start_date'],
461
                    'end_date' => $degreeInput['end_date']
462
                ]);
463
                $degree->save();
464
            }
465
        }
466
467
        //Update old degrees
468 View Code Duplication
        if (isset($degrees['old'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
469
            foreach($degrees['old'] as $id=>$degreeInput) {
470
                //Ensure this degree belongs to this applicant
471
                $degree = $applicant->degrees->firstWhere('id', $id);
472
                if ($degree != null) {
473
                    $degree->fill([
474
                        'degree_type_id' => $degreeInput['degree_type_id'],
475
                        'area_of_study' => $degreeInput['area_of_study'],
476
                        'institution' => $degreeInput['institution'],
477
                        'thesis' => $degreeInput['thesis'],
478
                        'start_date' => $degreeInput['start_date'],
479
                        'end_date' => $degreeInput['end_date']
480
                    ]);
481
                    $degree->save();
482
                } else {
483
                    Log::warning('Applicant '.$applicant->id.' attempted to update degree with invalid id '.$id);
484
                }
485
            }
486
        }
487
488
        $courses = $request->input('courses');
489
490
        $validatedCourseData = $request->validate([
0 ignored issues
show
Unused Code introduced by
$validatedCourseData is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
491
            'courses.new.*.name'             => 'required',
492
            'courses.new.*.institution'      => 'required',
493
            'courses.new.*.course_status_id' => 'required',
494
            'courses.new.*.start_date'       => 'required|date',
495
            'courses.new.*.end_date'         => 'required|date',
496
        ]);
497
498
        //Save new courses
499 View Code Duplication
        if (isset($courses['new'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
500
            foreach($courses['new'] as $courseInput) {
501
                $course = new Course();
502
                $course->applicant_id = $applicant->id;
503
                $course->fill([
504
                    'name' => $courseInput['name'],
505
                    'institution' => $courseInput['institution'],
506
                    'course_status_id' => $courseInput['course_status_id'],
507
                    'start_date' => $courseInput['start_date'],
508
                    'end_date' => $courseInput['end_date']
509
                ]);
510
                $course->save();
511
            }
512
        }
513
514
        //Update old courses
515
        if (isset($courses['old'])) {
516
            foreach($courses['old'] as $id=>$courseInput) {
517
                //Ensure this course belongs to this applicant
518
                $course = $applicant->courses->firstWhere('id', $id);
519
                if ($course != null) {
520
                    $course->fill([
521
                        'name' => $courseInput['name'],
522
                        'institution' => $courseInput['institution'],
523
                        'course_status_id' => $courseInput['course_status_id'],
524
                        'start_date' => $courseInput['start_date'],
525
                        'end_date' => $courseInput['end_date']
526
                    ]);
527
                    $course->save();
528
                } else {
529
                    Log::warning('Applicant '.$applicant->id.' attempted to update course with invalid id '.$id);
530
                }
531
            }
532
        }
533
534
        $work_experiences = $request->input('work_experiences');
535
536
        $validatedWorkData = $request->validate([
0 ignored issues
show
Unused Code introduced by
$validatedWorkData is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
537
            'work_experiences.new.*.role'        => 'required',
538
            'work_experiences.new.*.company'     => 'required',
539
            'work_experiences.new.*.description' => 'required',
540
            'work_experiences.new.*.start_date'  => 'required|date',
541
            'work_experiences.new.*.end_date'    => 'required|date',
542
        ]);
543
544
        //Save new work_experiences
545 View Code Duplication
        if (isset($work_experiences['new'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across 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...
546
            foreach($work_experiences['new'] as $workExperienceInput) {
547
                $workExperience = new WorkExperience();
548
                $workExperience->applicant_id = $applicant->id;
549
                $workExperience->fill([
550
                    'role' => $workExperienceInput['role'],
551
                    'company' => $workExperienceInput['company'],
552
                    'description' => $workExperienceInput['description'],
553
                    'start_date' => $workExperienceInput['start_date'],
554
                    'end_date' => $workExperienceInput['end_date']
555
                ]);
556
                $workExperience->save();
557
            }
558
        }
559
560
        //Update old work_experiences
561
        if (isset($work_experiences['old'])) {
562
            foreach($work_experiences['old'] as $id=>$workExperienceInput) {
563
                //Ensure this work_experience belongs to this applicant
564
                $workExperience = $applicant->work_experiences->firstWhere('id', $id);
565
                if ($workExperience != null) {
566
                    $workExperience->fill([
567
                        'role' => $workExperienceInput['role'],
568
                        'company' => $workExperienceInput['company'],
569
                        'description' => $workExperienceInput['description'],
570
                        'start_date' => $workExperienceInput['start_date'],
571
                        'end_date' => $workExperienceInput['end_date']
572
                    ]);
573
                    $workExperience->save();
574
                } else {
575
                    Log::warning('Applicant '.$applicant->id.' attempted to update work_experience with invalid id '.$id);
576
                }
577
            }
578
        }
579
580
        //Redirect to correct page
581
        switch($request->input('submit')) {
582
            case 'save_and_quit':
583
            return redirect()->route('applications.index');
584
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
585
            case 'save_and_continue':
586
            case 'next':
587
            return redirect()->route('job.application.edit.3', $jobPoster);
588
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
589
            case 'previous':
590
            return redirect()->route('job.application.edit.1', $jobPoster);
591
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
592
            default:
593
            return redirect()->back()->withInput();
594
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
595
        }
596
    }
597
598
    /**
599
    * Update the Application Essential Skills in storage for the specified job.
600
    *
601
    * @param  \Illuminate\Http\Request  $request
602
    * @param  \App\Models\JobPoster  $jobPoster
603
    * @return \Illuminate\Http\Response
604
    */
605 View Code Duplication
    public function update_essential_skills(Request $request, JobPoster $jobPoster)
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...
606
    {
607
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
608
        $application = $this->getApplicationFromJob($jobPoster);
609
610
        //Ensure user has permissions to update this application
611
        $this->authorize('update', $application);
612
613
        $skillDeclarations = $request->input('skill_declarations');
614
        $claimedStatusId = SkillStatus::where('name', 'claimed')->firstOrFail()->id;
615
616
        //Save new skill declarartions
617
        if (isset($skillDeclarations['new'])) {
618
            foreach($skillDeclarations['new'] as $skillType => $typeInput) {
619
                foreach($typeInput as $criterion_id=>$skillDeclarationInput) {
620
                    $skillDeclaration = new SkillDeclaration();
621
                    $skillDeclaration->applicant_id = $applicant->id;
622
                    $skillDeclaration->skill_id = Criteria::find($criterion_id)->skill->id;
623
                    $skillDeclaration->skill_status_id = $claimedStatusId;
624
                    $skillDeclaration->fill([
625
                        'description' => $skillDeclarationInput['description'],
626
                        'skill_level_id' => isset($skillDeclarationInput['skill_level_id']) ? $skillDeclarationInput['skill_level_id'] : null,
627
                    ]);
628
                    $skillDeclaration->save();
629
630
                    $referenceIds = $this->getRelativeIds($skillDeclarationInput, 'references');
631
                    $skillDeclaration->references()->sync($referenceIds);
632
633
                    $sampleIds = $this->getRelativeIds($skillDeclarationInput, 'samples');
634
                    $skillDeclaration->work_samples()->sync($sampleIds);
635
                }
636
            }
637
        }
638
639
        //Update old declarations
640
        if (isset($skillDeclarations['old'])) {
641
            foreach($skillDeclarations['old'] as $skillType => $typeInput) {
642
                foreach($typeInput as $id=>$skillDeclarationInput) {
643
                    //Ensure this declaration belongs to this applicant
644
                    $skillDeclaration = $applicant->skill_declarations->firstWhere('id', $id);
645
                    if ($skillDeclaration != null) {
646
                        //skill_id and skill_status cannot be changed
647
                        $skillDeclaration->fill([
648
                            'description' => $skillDeclarationInput['description'],
649
                            'skill_level_id' => isset($skillDeclarationInput['skill_level_id']) ? $skillDeclarationInput['skill_level_id'] : null,
650
                        ]);
651
                        $skillDeclaration->save();
652
653
                        $referenceIds = $this->getRelativeIds($skillDeclarationInput, 'references');
654
                        $skillDeclaration->references()->sync($referenceIds);
655
656
                        $sampleIds = $this->getRelativeIds($skillDeclarationInput, 'samples');
657
                        $skillDeclaration->work_samples()->sync($sampleIds);
658
                    } else {
659
                        Log::warning('Applicant '.$applicant->id.' attempted to update skill declaration with invalid id '.$id);
660
                    }
661
                }
662
            }
663
        }
664
665
        //Redirect to correct page
666
        switch($request->input('submit')) {
667
            case 'save_and_quit':
668
            return redirect()->route('applications.index');
669
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
670
            case 'save_and_continue':
671
            case 'next':
672
            return redirect()->route('job.application.edit.4', $jobPoster);
673
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
674
            case 'previous':
675
            return redirect()->route('job.application.edit.2', $jobPoster);
676
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
677
            default:
678
            return redirect()->back()->withInput();
679
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
680
        }
681
    }
682
683
    /**
684
    * Update the Application Asset Skills in storage for the specified job.
685
    *
686
    * @param  \Illuminate\Http\Request  $request
687
    * @param  \App\Models\JobPoster  $jobPoster
688
    * @return \Illuminate\Http\Response
689
    */
690 View Code Duplication
    public function update_asset_skills(Request $request, JobPoster $jobPoster)
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...
691
    {
692
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
693
        $application = $this->getApplicationFromJob($jobPoster);
694
695
        //Ensure user has permissions to update this application
696
        $this->authorize('update', $application);
697
698
        $skillDeclarations = $request->input('skill_declarations');
699
        $claimedStatusId = SkillStatus::where('name', 'claimed')->firstOrFail()->id;
700
701
        //Save new skill declarartions
702
        if (isset($skillDeclarations['new'])) {
703
            foreach($skillDeclarations['new'] as $skillType => $typeInput) {
704
                foreach($typeInput as $criterion_id=>$skillDeclarationInput) {
705
                    $skillDeclaration = new SkillDeclaration();
706
                    $skillDeclaration->applicant_id = $applicant->id;
707
                    $skillDeclaration->skill_id = Criteria::find($criterion_id)->skill->id;
708
                    $skillDeclaration->skill_status_id = $claimedStatusId;
709
                    $skillDeclaration->fill([
710
                        'description' => $skillDeclarationInput['description'],
711
                        'skill_level_id' => isset($skillDeclarationInput['skill_level_id']) ? $skillDeclarationInput['skill_level_id'] : null,
712
                    ]);
713
                    $skillDeclaration->save();
714
715
                    $referenceIds = $this->getRelativeIds($skillDeclarationInput, 'references');
716
                    $skillDeclaration->references()->sync($referenceIds);
717
718
                    $sampleIds = $this->getRelativeIds($skillDeclarationInput, 'samples');
719
                    $skillDeclaration->work_samples()->sync($sampleIds);
720
                }
721
            }
722
        }
723
724
        //Update old declarations
725
        if (isset($skillDeclarations['old'])) {
726
            foreach($skillDeclarations['old'] as $skillType => $typeInput) {
727
                foreach($typeInput as $id=>$skillDeclarationInput) {
728
                    //Ensure this declaration belongs to this applicant
729
                    $skillDeclaration = $applicant->skill_declarations->firstWhere('id', $id);
730
                    if ($skillDeclaration != null) {
731
                        //skill_id and skill_status cannot be changed
732
                        $skillDeclaration->fill([
733
                            'description' => $skillDeclarationInput['description'],
734
                            'skill_level_id' => isset($skillDeclarationInput['skill_level_id']) ? $skillDeclarationInput['skill_level_id'] : null,
735
                        ]);
736
                        $skillDeclaration->save();
737
738
                        $referenceIds = $this->getRelativeIds($skillDeclarationInput, 'references');
739
                        $skillDeclaration->references()->sync($referenceIds);
740
741
                        $sampleIds = $this->getRelativeIds($skillDeclarationInput, 'samples');
742
                        $skillDeclaration->work_samples()->sync($sampleIds);
743
                    } else {
744
                        Log::warning('Applicant '.$applicant->id.' attempted to update skill declaration with invalid id '.$id);
745
                    }
746
                }
747
            }
748
        }
749
750
        //Redirect to correct page
751
        switch($request->input('submit')) {
752
            case 'save_and_quit':
753
            return redirect()->route('applications.index');
754
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
755
            case 'save_and_continue':
756
            case 'next':
757
            return redirect()->route('job.application.edit.5', $jobPoster);
758
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
759
            case 'previous':
760
            return redirect()->route('job.application.edit.3', $jobPoster);
761
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
762
            default:
763
            return redirect()->back()->withInput();
764
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
765
        }
766
    }
767
768
    /**
769
    * Submit the Application for the specified job.
770
    *
771
    * @param  \Illuminate\Http\Request  $request
772
    * @param  \App\Models\JobPoster  $jobPoster
773
    * @return \Illuminate\Http\Response
774
    */
775
    public function submit(Request $request, JobPoster $jobPoster)
776
    {
777
        $applicant = Auth::user()->applicant;
0 ignored issues
show
Bug introduced by
Accessing applicant 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...
Unused Code introduced by
$applicant is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
778
        $application = $this->getApplicationFromJob($jobPoster);
779
780
        //Ensure user has permissions to update this application
781
        $this->authorize('update', $application);
782
783
        //Only complete submission if submit button was pressed
784
        if ($request->input('submit') == "submit") {
785
786
            $request->validate([
787
                'submission_signature' => [
788
                    'required',
789
                    'string',
790
                    'max:191',
791
                ],
792
                'submission_date' => [
793
                    'required',
794
                    'string',
795
                    'max:191',
796
                ]
797
            ]);
798
799
            //Save any final info
800
            $application->fill([
801
                'submission_signature' => $request->input('submission_signature'),
802
                'submission_date' => $request->input('submission_date'),
803
            ]);
804
805
            $validator = new ApplicationValidator();
806
            $validator->validate($application);
807
808
            //Change status to 'submitted'
809
            $application->application_status_id = ApplicationStatus::where('name', 'submitted')->firstOrFail()->id;
810
        }
811
812
        $application->save();
813
814
        //Redirect to correct page
815
        switch($request->input('submit')) {
816
            case 'save_and_quit':
817
            return redirect()->route('applications.index');
818
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
819
            case 'submit':
820
            return redirect()->route('job.application.complete', $jobPoster);
821
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
822
            case 'previous':
823
            return redirect()->route('job.application.edit.4', $jobPoster);
824
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
825
            default:
826
            return redirect()->back()->withInput();
827
            break;
0 ignored issues
show
Unused Code introduced by
break is not strictly necessary here and could be removed.

The break statement is not necessary if it is preceded for example by a return statement:

switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.

Loading history...
828
        }
829
    }
830
}
831