Scrutinizer GitHub App not installed

We could not synchronize checks via GitHub's checks API since Scrutinizer's GitHub App is not installed for this repository.

Install GitHub App

Completed
Pull Request — master (#61)
by Oliver
01:39
created

PageCrudController::getTemplates()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 13
Code Lines 7

Duplication

Lines 0
Ratio 0 %

Importance

Changes 2
Bugs 1 Features 0
Metric Value
cc 2
eloc 7
c 2
b 1
f 0
nc 2
nop 1
dl 0
loc 13
rs 9.4285
1
<?php
2
3
namespace Backpack\PageManager\app\Http\Controllers\Admin;
4
5
use App\PageTemplates;
6
// VALIDATION: change the requests to match your own file names if you need form validation
7
use Backpack\PageManager\app\TraitReflections;
8
use Backpack\CRUD\app\Http\Controllers\CrudController;
9
use Backpack\PageManager\app\Http\Requests\PageRequest as StoreRequest;
10
use Backpack\PageManager\app\Http\Requests\PageRequest as UpdateRequest;
11
12
class PageCrudController extends CrudController
13
{
14
    use PageTemplates;
15
    use TraitReflections;
16
17
    public function setup($template_name = false)
0 ignored issues
show
Unused Code introduced by
The parameter $template_name 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...
Coding Style Naming introduced by
The parameter $template_name is not named in camelCase.

This check marks parameter names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
18
    {
19
        parent::__construct();
0 ignored issues
show
Comprehensibility Bug introduced by
It seems like you call parent on a different method (__construct() instead of setup()). Are you sure this is correct? If so, you might want to change this to $this->__construct().

This check looks for a call to a parent method whose name is different than the method from which it is called.

Consider the following code:

class Daddy
{
    protected function getFirstName()
    {
        return "Eidur";
    }

    protected function getSurName()
    {
        return "Gudjohnsen";
    }
}

class Son
{
    public function getFirstName()
    {
        return parent::getSurname();
    }
}

The getFirstName() method in the Son calls the wrong method in the parent class.

Loading history...
20
21
        $modelClass = config('backpack.pagemanager.page_model_class', 'Backpack\PageManager\app\Models\Page');
22
23
        $this->checkForTemplatesAndUniquePagesNotDistinct();
24
25
        /*
26
        |--------------------------------------------------------------------------
27
        | BASIC CRUD INFORMATION
28
        |--------------------------------------------------------------------------
29
        */
30
        $this->crud->setModel($modelClass);
31
        $this->crud->setRoute(config('backpack.base.route_prefix').'/page');
32
        $this->crud->setEntityNameStrings(trans('backpack::pagemanager.page'), trans('backpack::pagemanager.pages'));
33
34
        $template_names = $this->getTemplateNames();
35
        $this->crud->addClause('whereIn', 'template', $template_names);
36
37
        /*
38
        |--------------------------------------------------------------------------
39
        | COLUMNS
40
        |--------------------------------------------------------------------------
41
        */
42
43
        $this->crud->addColumn([
44
                                'name' => 'name',
45
                                'label' => trans('backpack::pagemanager.name'),
46
                                ]);
47
        $this->crud->addColumn([
48
                                'name' => 'template',
49
                                'label' => trans('backpack::pagemanager.template'),
50
                                'type' => 'model_function',
51
                                'function_name' => 'getTemplateName',
52
                                ]);
53
        $this->crud->addColumn([
54
                                'name' => 'slug',
55
                                'label' => trans('backpack::pagemanager.slug'),
56
                                ]);
57
58
        /*
59
        |--------------------------------------------------------------------------
60
        | FIELDS
61
        |--------------------------------------------------------------------------
62
        */
63
64
        // In PageManager,
65
        // - default fields, that all templates are using, are set using $this->addDefaultPageFields();
66
        // - template-specific fields are set per-template, in the PageTemplates trait;
67
68
        /*
69
        |--------------------------------------------------------------------------
70
        | BUTTONS
71
        |--------------------------------------------------------------------------
72
        */
73
        $this->crud->addButtonFromModelFunction('line', 'open', 'getOpenButton', 'beginning');
0 ignored issues
show
Documentation introduced by
'beginning' is of type string, but the function expects a boolean.

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...
74
    }
75
76
    // -----------------------------------------------
77
    // Overwrites of CrudController
78
    // -----------------------------------------------
79
80
    // Overwrites the CrudController create() method to add template usage.
81
    public function create($template = false)
82
    {
83
        $this->addDefaultPageFields($template);
0 ignored issues
show
Documentation introduced by
$template is of type boolean, but the function expects a false|string.

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...
84
        $this->useTemplate($template);
0 ignored issues
show
Documentation introduced by
$template is of type boolean, but the function expects a false|string.

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...
85
86
        return parent::create();
87
    }
88
89
    // Overwrites the CrudController store() method to add template usage.
90
    public function store(StoreRequest $request)
0 ignored issues
show
Unused Code introduced by
The parameter $request 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...
91
    {
92
        $this->addDefaultPageFields(\Request::input('template'));
0 ignored issues
show
Bug introduced by
It seems like \Request::input('template') targeting Illuminate\Http\Concerns...ractsWithInput::input() can also be of type array or null; however, Backpack\PageManager\app...:addDefaultPageFields() does only seem to accept false|string, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
93
        $this->useTemplate(\Request::input('template'));
0 ignored issues
show
Bug introduced by
It seems like \Request::input('template') targeting Illuminate\Http\Concerns...ractsWithInput::input() can also be of type array or null; however, Backpack\PageManager\app...ntroller::useTemplate() does only seem to accept false|string, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
94
95
        return parent::storeCrud();
0 ignored issues
show
Comprehensibility Bug introduced by
It seems like you call parent on a different method (storeCrud() instead of store()). Are you sure this is correct? If so, you might want to change this to $this->storeCrud().

This check looks for a call to a parent method whose name is different than the method from which it is called.

Consider the following code:

class Daddy
{
    protected function getFirstName()
    {
        return "Eidur";
    }

    protected function getSurName()
    {
        return "Gudjohnsen";
    }
}

class Son
{
    public function getFirstName()
    {
        return parent::getSurname();
    }
}

The getFirstName() method in the Son calls the wrong method in the parent class.

Loading history...
96
    }
97
98
    // Overwrites the CrudController edit() method to add template usage.
99
    public function edit($id, $template = false)
100
    {
101
        // if the template in the GET parameter is missing, figure it out from the db
102
        if ($template == false) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
103
            $model = $this->crud->model;
104
            $this->data['entry'] = $model::findOrFail($id);
105
            $template = $this->data['entry']->template;
106
        }
107
108
        $this->addDefaultPageFields($template);
109
        $this->useTemplate($template);
110
111
        return parent::edit($id);
112
    }
113
114
    // Overwrites the CrudController update() method to add template usage.
115
    public function update(UpdateRequest $request)
0 ignored issues
show
Unused Code introduced by
The parameter $request 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...
116
    {
117
        $this->addDefaultPageFields(\Request::input('template'));
0 ignored issues
show
Bug introduced by
It seems like \Request::input('template') targeting Illuminate\Http\Concerns...ractsWithInput::input() can also be of type array or null; however, Backpack\PageManager\app...:addDefaultPageFields() does only seem to accept false|string, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
118
        $this->useTemplate(\Request::input('template'));
0 ignored issues
show
Bug introduced by
It seems like \Request::input('template') targeting Illuminate\Http\Concerns...ractsWithInput::input() can also be of type array or null; however, Backpack\PageManager\app...ntroller::useTemplate() does only seem to accept false|string, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
119
120
        return parent::updateCrud();
0 ignored issues
show
Comprehensibility Bug introduced by
It seems like you call parent on a different method (updateCrud() instead of update()). Are you sure this is correct? If so, you might want to change this to $this->updateCrud().

This check looks for a call to a parent method whose name is different than the method from which it is called.

Consider the following code:

class Daddy
{
    protected function getFirstName()
    {
        return "Eidur";
    }

    protected function getSurName()
    {
        return "Gudjohnsen";
    }
}

class Son
{
    public function getFirstName()
    {
        return parent::getSurname();
    }
}

The getFirstName() method in the Son calls the wrong method in the parent class.

Loading history...
121
    }
122
123
    // -----------------------------------------------
124
    // Methods that are particular to the PageManager.
125
    // -----------------------------------------------
126
127
    /**
128
     * Populate the create/update forms with basic fields, that all pages need.
129
     *
130
     * @param string $template The name of the template that should be used in the current form.
0 ignored issues
show
Documentation introduced by
Should the type for parameter $template not be false|string?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
131
     */
132
    public function addDefaultPageFields($template = false)
133
    {
134
        $this->crud->addField([
135
                                'name' => 'template',
136
                                'label' => trans('backpack::pagemanager.template'),
137
                                'type' => 'select_page_template',
138
                                'options' => $this->getTemplatesArray(),
139
                                'value' => $template,
140
                                'allows_null' => false,
141
                                'wrapperAttributes' => [
142
                                    'class' => 'form-group col-md-6',
143
                                ],
144
                            ]);
145
        $this->crud->addField([
146
                                'name' => 'name',
147
                                'label' => trans('backpack::pagemanager.page_name'),
148
                                'type' => 'text',
149
                                'wrapperAttributes' => [
150
                                    'class' => 'form-group col-md-6',
151
                                ],
152
                                // 'disabled' => 'disabled'
0 ignored issues
show
Unused Code Comprehensibility introduced by
50% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
153
                            ]);
154
        $this->crud->addField([
155
                                'name' => 'title',
156
                                'label' => trans('backpack::pagemanager.page_title'),
157
                                'type' => 'text',
158
                                // 'disabled' => 'disabled'
0 ignored issues
show
Unused Code Comprehensibility introduced by
50% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
159
                            ]);
160
        $this->crud->addField([
161
                                'name' => 'slug',
162
                                'label' => trans('backpack::pagemanager.page_slug'),
163
                                'type' => 'text',
164
                                'hint' => trans('backpack::pagemanager.page_slug_hint'),
165
                                // 'disabled' => 'disabled'
0 ignored issues
show
Unused Code Comprehensibility introduced by
50% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
166
                            ]);
167
    }
168
169
    /**
170
     * Add the fields defined for a specific template.
171
     *
172
     * @param  string $template_name The name of the template that should be used in the current form.
0 ignored issues
show
Documentation introduced by
Should the type for parameter $template_name not be false|string?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
173
     */
174
    public function useTemplate($template_name = false)
0 ignored issues
show
Coding Style Naming introduced by
The parameter $template_name is not named in camelCase.

This check marks parameter names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
175
    {
176
        $templates = $this->getTemplates();
177
178
        // set the default template
179
        if ($template_name == false) {
0 ignored issues
show
Bug introduced by
It seems like you are loosely comparing $template_name of type false|string against false; this is ambiguous if the string can be empty. Consider using a strict comparison === instead.
Loading history...
180
            $template_name = $templates[0]->name;
181
        }
182
183
        // actually use the template
184
        if ($template_name) {
185
            $this->{$template_name}();
186
        }
187
    }
188
189
    /**
190
     * Get all defined template as an array.
191
     *
192
     * Used to populate the template dropdown in the create/update forms.
193
     */
194
    public function getTemplatesArray()
195
    {
196
        $templates = $this->getTemplates();
197
198
        foreach ($templates as $template) {
199
            $templates_array[$template->name] = $this->crud->makeLabel($template->name);
0 ignored issues
show
Coding Style Comprehensibility introduced by
$templates_array was never initialized. Although not strictly required by PHP, it is generally a good practice to add $templates_array = array(); before regardless.

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

Let’s take a look at an example:

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

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

    // do something with $myArray
}

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

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

Loading history...
200
        }
201
202
        return $templates_array;
0 ignored issues
show
Bug introduced by
The variable $templates_array does not seem to be defined for all execution paths leading up to this point.

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

Let’s take a look at an example:

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

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

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

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

Available Fixes

  1. Check for existence of the variable explicitly:

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

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

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
203
    }
204
}
205