Completed
Pull Request — develop (#323)
by Mathias
15:36
created

AdminCategoriesController::processPost()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 17
Code Lines 13

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 17
rs 9.4285
c 0
b 0
f 0
cc 1
eloc 13
nc 1
nop 1
1
<?php
2
/**
3
 * YAWIK
4
 *
5
 * @filesource
6
 * @license MIT
7
 * @copyright  2013 - 2016 Cross Solution <http://cross-solution.de>
8
 */
9
  
10
/** */
11
namespace Jobs\Controller;
12
13
use Core\Form\SummaryForm;
14
use Jobs\Entity\Category;
15
use Jobs\Listener\Events\JobEvent;
16
use Zend\Mvc\Controller\AbstractActionController;
17
use Zend\View\Model\JsonModel;
18
use Zend\View\Model\ViewModel;
19
20
/**
21
 * Handles the management of the job categories.
22
 * 
23
 * @author Mathias Gelhausen <[email protected]>
24
 * @since 0.29
25
 */
26
class AdminCategoriesController extends AbstractActionController
27
{
28
29
    public function indexAction()
30
    {
31
        $form = $this->setupContainer();
32
33
        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.

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...
34
            return $this->processPost($form);
35
        }
36
37
        $model = new ViewModel([
38
            'form' => $form
39
        ]);
40
        $model->setTemplate('jobs/admin/categories');
41
42
        return $model;
43
    }
44
45
    private function setupContainer()
46
    {
47
        $services = $this->serviceLocator;
48
49
        $forms = $services->get('forms');
50
        $form = $forms->get('Jobs/AdminCategories');
51
52
        $repositories = $services->get('repositories');
53
        $rep = $repositories->get('Jobs/Category');
54
55
        $professions = $rep->findOneBy(['value' => 'professions']);
56
        if (!$professions) {
57
            $professions = new Category('Professions');
58
            $repositories->store($professions);
59
        }
60
61
        $types = $rep->findOneBy(['value' => 'employmentTypes']);
62
        if (!$types) {
63
            $types = new Category('Employment Types', 'employmentTypes');
64
            $repositories->store($types);
65
        }
66
67
        $form->setEntity($professions, 'professions');
68
        $form->setEntity($types, 'employmentTypes');
69
70
        return $form;
71
    }
72
73
    private function processPost($container)
0 ignored issues
show
Coding Style introduced by
processPost 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...
74
    {
75
        $identifier = $this->params()->fromQuery('form');
76
        $form       = $container->getForm($identifier);
77
        $form->setData($_POST);
78
        $valid = $form->isValid();
79
        $this->serviceLocator->get('repositories')->store($form->getObject());
80
        $form->bind($form->getObject());
81
        $form->setRenderMode(SummaryForm::RENDER_SUMMARY);
82
        $helper = $this->serviceLocator->get('ViewHelperManager')->get('summaryform');
83
84
        return new JsonModel([
85
            'content' => $helper($form),
86
            'valid' => $valid,
87
            'errors' => $form->getMessages()
88
        ]);
89
    }
90
}