Completed
Push — master ( 05caed...55eada )
by Alexander
03:06
created

AbstractAdminGridController::listAction()   B

Complexity

Conditions 6
Paths 32

Size

Total Lines 38
Code Lines 19

Duplication

Lines 0
Ratio 0 %

Importance

Changes 6
Bugs 0 Features 1
Metric Value
c 6
b 0
f 1
dl 0
loc 38
rs 8.439
nc 32
cc 6
eloc 19
nop 0
1
<?php
2
3
namespace AtAdmin\Controller;
4
5
use AtDataGrid\DataGrid;
6
use AtDataGrid\Form\FormBuilder;
7
use AtDataGrid\Manager as GridManager;
8
use Zend\EventManager\EventManager;
9
use Zend\Form\Form;
10
use Zend\View\Model\ViewModel;
11
12
abstract class AbstractAdminGridController extends AbstractAdminController
13
{
14
    // Events
15
    const EVENT_CHECK_PERMISSIONS_LIST   = 'at-admin.check-permissions.list';
16
    const EVENT_CHECK_PERMISSIONS_CREATE = 'at-admin.check-permissions.create';
17
    const EVENT_CHECK_PERMISSIONS_EDIT   = 'at-admin.check-permissions.edit';
18
    const EVENT_CHECK_PERMISSIONS_DELETE = 'at-admin.check-permissions.delete';
19
20
    const EVENT_VALIDATION_EDIT_PRE      = 'at-admin.validation.edit.pre';
21
22
    const EVENT_SAVE_PRE                 = 'at-admin.save.pre';
23
    const EVENT_SAVE_POST                = 'at-admin.save.post';
24
    const EVENT_DELETE_PRE               = 'at-admin.delete.pre';
25
    const EVENT_DELETE_POST              = 'at-admin.delete.post';
26
27
    const EVENT_SET_TEMPLATE_CREATE      = 'at-admin.set-template.create';
28
    const EVENT_SET_TEMPLATE_EDIT        = 'at-admin.set-template.edit';
29
30
    // Page titles
31
    // @todo Remove 
32
    const TITLE_ACTION_LIST              = '';
33
    const TITLE_ACTION_CREATE            = 'Create';
34
    const TITLE_ACTION_EDIT              = 'Edit';
35
    const TITLE_ACTION_DELETE            = 'Delete';
36
37
    /**
38
     * @var GridManager
39
     */
40
    protected $gridManager;
41
42
    /**
43
     * @return mixed
44
     */
45
    public function getAction()
0 ignored issues
show
Coding Style introduced by
getAction uses the super-global variable $_POST which is generally not recommended.

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

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

// Better
class Router
{
    private $host;

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

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

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

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
46
    {
47
        $this->getEventManager()->trigger(self::EVENT_CHECK_PERMISSIONS_LIST, $this);
48
49
        // Save back url to redirect after actions
50
        $this->backTo()->setBackUrl();
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
51
52
        // Check for mass actions
53
        if (isset($_POST['cmd'])) {
54
            $this->forward($_POST['cmd']);    // @todo refactor this
0 ignored issues
show
Unused Code introduced by
The call to AbstractAdminGridController::forward() has too many arguments starting with $_POST['cmd'].

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

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

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

Loading history...
55
        }
56
57
        $gridManager = $this->getGridManager();
58
        $grid = $gridManager->getGrid();
59
60
        if ($this->request->getQuery('order')) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
61
            $order = explode('~', $this->request->getQuery('order'));
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
62
            $grid->setOrder([$order[0] => $order[1]]);
63
        }
64
65
        if ($this->request->getQuery('page')) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
66
            $grid->setCurrentPage($this->request->getQuery('page'));
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
67
        }
68
69
        if ($this->request->getQuery('show_items')) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
70
            $grid->setItemsPerPage($this->request->getQuery('show_items'));
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
71
        }
72
73
        $filtersForm = $gridManager->getFiltersForm();
74
        $filtersForm->setData($this->request->getQuery());
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getQuery() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
75
        if (!$filtersForm->isValid()) {
0 ignored issues
show
Unused Code introduced by
This if statement is empty and can be removed.

This check looks for the bodies of if statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.

These if bodies can be removed. If you have an empty if but statements in the else branch, consider inverting the condition.

if (rand(1, 6) > 3) {
//print "Check failed";
} else {
    print "Check succeeded";
}

could be turned into

if (rand(1, 6) <= 3) {
    print "Check succeeded";
}

This is much more concise to read.

Loading history...
76
            //$this->flashMessenger()->addMessage($filtersForm->getMessages());
0 ignored issues
show
Unused Code Comprehensibility introduced by
80% 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...
77
        }
78
79
        $grid->setFiltersData($filtersForm->getData());
80
81
        return $gridManager->render();
82
    }
83
84
    /**
85
     * @return mixed|ViewModel
86
     * @throws \Exception
87
     */
88
    public function createAction()
89
    {
90
        /** @var EventManager $eventManager */
91
        $eventManager = $this->getEventManager();
92
93
        $this->getEventManager()->trigger(self::EVENT_CHECK_PERMISSIONS_CREATE, $this);
94
95
        /** @var GridManager $gridManager */
96
        $gridManager = $this->getGridManager();
97
        if (! $gridManager->isAllowCreate()) {
98
            throw new \Exception('Creating is disabled');
99
        }
100
101
        /** @var FormBuilder $formManager */
102
        $formBuilder = $gridManager->getFormBuilder();
103
104
        /** @var DataGrid $grid */
105
        $grid = $gridManager->getGrid();
106
107
        /** @var Form $form */
108
        $form = $formBuilder->build($grid);
109
110
        if ($this->getRequest()->isPost()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
111
            $post = $this->getRequest()->getPost();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
112
            $form->setData($post);
113
114 View Code Duplication
            if ($form->isValid()) {
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...
115
                // Replace POST data with filtered and validated form values
116
                // POST data may contains not only form data
117
                $data = array_replace($post->toArray(), $form->getData());
118
119
                $eventManager->trigger(self::EVENT_SAVE_PRE, null, $data);
120
                $item = $grid->save($data);
121
                $eventManager->trigger(self::EVENT_SAVE_POST, $item, $data);
122
123
                return $this->backTo()->previous('Record created');
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
124
            } else {
125
                $this->flashMessenger()->addMessage('Check form data.');
126
            }
127
        }
128
129
        $viewModel = new ViewModel([
130
            'title'        => static::TITLE_ACTION_CREATE,
131
            'form'         => $form,
132
            'customJs'     => $formBuilder->getCustomJs(),
133
            'formSections' => $formBuilder->getFormSections(),
134
            'backUrl'      => $this->backTo()->getBackUrl(false),
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
135
        ]);
136
137
        $viewModel->setTemplate('at-admin/create.phtml');
138
        $eventResult = $eventManager->trigger(self::EVENT_SET_TEMPLATE_CREATE, $viewModel)->last();
139
        if ($eventResult) {
140
            $viewModel = $eventResult;
141
        }
142
143
        return $viewModel;
144
    }
145
146
    /**
147
     * @return ViewModel
148
     * @throws \Exception
149
     */
150
    public function editAction()
151
    {
152
        /** @var Manager $gridManager */
153
        $gridManager = $this->getGridManager();
154
155
        /** @var DataGrid $grid */
156
        $grid = $gridManager->getGrid();
157
158
        if (!$gridManager->isAllowEdit()) {
159
            throw new \Exception('Editing is disabled');
160
        }
161
162
        $id = $this->params($grid->getIdentifierColumnName());
163
        if (!$id) {
164
            return $this->notFoundAction();
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this->notFoundAction(); (array) is incompatible with the return type documented by AtAdmin\Controller\Abstr...dController::editAction of type Zend\View\Model\ViewModel.

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...
165
        }
166
167
        $item = $grid->getRow($id);
168
        if (!$item) {
169
            return $this->notFoundAction();
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this->notFoundAction(); (array) is incompatible with the return type documented by AtAdmin\Controller\Abstr...dController::editAction of type Zend\View\Model\ViewModel.

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...
170
        }
171
172
        /** @var EventManager $eventManager */
173
        $eventManager = $this->getEventManager();
174
        $eventManager->trigger(self::EVENT_CHECK_PERMISSIONS_EDIT, $item);
175
176
        /** @var FormBuilder $formBuilder */
177
        $formBuilder = $gridManager->getFormBuilder();
178
179
        /** @var Form $form */
180
        $form = $formBuilder->build($grid, FormBuilder::FORM_CONTEXT_EDIT, $item);
181
182
        if ($this->getRequest()->isPost()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
183
            $post = $this->getRequest()->getPost();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method getPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request, Zend\Psr7Bridge\Zend\Request.

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...
184
            $form->setData($post);
185
186
            $this->getEventManager()->trigger(self::EVENT_VALIDATION_EDIT_PRE, $form, ['oldData' => $item, 'newData' => $post]);
187
188 View Code Duplication
            if ($form->isValid()) {
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...
189
                // Replace POST data with filtered and validated form values
190
                // POST data may contains not only form data (extra data from sections)
191
                $data = array_replace($post->toArray(), $form->getData());
192
193
                $eventManager->trigger(self::EVENT_SAVE_PRE, $item, $data);
194
                $item = $grid->save($data, $id);
195
                $eventManager->trigger(self::EVENT_SAVE_POST, $item, $data);
196
197
                $this->backTo()->previous('Record was updated');
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
198
            } else {
199
                $this->flashMessenger()->addMessage('Check form data');
200
            }
201
        }
202
203
        $viewModel = new ViewModel([
204
            'title'        => static::TITLE_ACTION_EDIT,
205
            'item'         => $item,
206
            'form'         => $form,
207
            'customJs'     => $formBuilder->getCustomJs(),
208
            'formSections' => $formBuilder->getFormSections(),
209
            'backUrl'      => $this->backTo()->getBackUrl(false),
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
210
        ]);
211
212
        $viewModel->setTemplate('at-admin/edit.phtml');
213
        $eventResult = $this->getEventManager()->trigger(self::EVENT_SET_TEMPLATE_EDIT, $viewModel, ['item' => $item])->last();
214
        if ($eventResult) {
215
            $viewModel = $eventResult;
216
        }
217
218
        return $viewModel;
219
    }
220
221
    /**
222
     * @throws \Exception
223
     */
224
    public function deleteAction()
225
    {
226
        $gridManager = $this->getGridManager();
227
        if (!$gridManager->isAllowDelete()) {
228
            throw new \Exception('Deleting is disabled.');
229
        }
230
231
        $id = $this->params('id');
232
        if (!$id) {
233
            return $this->notFoundAction();
234
        }
235
236
        $grid = $gridManager->getGrid();
237
        $item = $grid->getRow($id);
238
        if (!$item) {
239
            return $this->notFoundAction();
240
        }
241
242
        $evm = $this->getEventManager();
243
        $evm->trigger(self::EVENT_CHECK_PERMISSIONS_DELETE, $item);
244
245
        // Get additional params
246
        $params = array_merge_recursive(
247
            $this->params()->fromQuery(),
248
            $this->params()->fromPost()
249
        );
250
251
        $evm->trigger(self::EVENT_DELETE_PRE, $item, $params);
252
        $grid->delete($id);
253
        $evm->trigger(self::EVENT_DELETE_POST, $item, $params);
254
255
        $this->backTo()->previous('Record deleted.');
0 ignored issues
show
Documentation Bug introduced by
The method backTo does not exist on object<AtAdmin\Controlle...actAdminGridController>? Since you implemented __call, maybe consider adding a @method annotation.

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

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

class ParentClass {
    private $data = array();

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

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

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
256
    }
257
258
    /**
259
     * @param GridManager $manager
260
     */
261
    public function setGridManager(GridManager $manager)
262
    {
263
        $this->gridManager = $manager;
264
    }
265
266
    /**
267
     * @return Manager
268
     */
269
    public function getGridManager()
270
    {
271
        if (!$this->gridManager) {
272
            throw new \RuntimeException('GridManager is not set');
273
        }
274
275
        return $this->gridManager;
276
    }
277
}