Completed
Push — master ( 1dbefb...c77374 )
by Jeroen
32s queued 11s
created

NodeAdminController::dispatch()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 11

Duplication

Lines 11
Ratio 100 %

Code Coverage

Tests 0
CRAP Score 6

Importance

Changes 0
Metric Value
dl 11
loc 11
ccs 0
cts 9
cp 0
c 0
b 0
f 0
rs 9.9
cc 2
nc 2
nop 2
crap 6
1
<?php
2
3
namespace Kunstmaan\NodeBundle\Controller;
4
5
use DateTime;
6
use Doctrine\Common\Collections\ArrayCollection;
7
use Doctrine\ORM\EntityManager;
8
use InvalidArgumentException;
9
use Kunstmaan\AdminBundle\Entity\BaseUser;
10
use Kunstmaan\AdminBundle\Entity\EntityInterface;
11
use Kunstmaan\AdminBundle\FlashMessages\FlashTypes;
12
use Kunstmaan\AdminBundle\Helper\FormWidgets\FormWidget;
13
use Kunstmaan\AdminBundle\Helper\FormWidgets\Tabs\Tab;
14
use Kunstmaan\AdminBundle\Helper\FormWidgets\Tabs\TabPane;
15
use Kunstmaan\AdminBundle\Helper\Security\Acl\AclHelper;
16
use Kunstmaan\AdminBundle\Helper\Security\Acl\Permission\PermissionMap;
17
use Kunstmaan\AdminBundle\Service\AclManager;
18
use Kunstmaan\AdminListBundle\AdminList\AdminList;
19
use Kunstmaan\NodeBundle\AdminList\NodeAdminListConfigurator;
20
use Kunstmaan\NodeBundle\Entity\HasNodeInterface;
21
use Kunstmaan\NodeBundle\Entity\Node;
22
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
23
use Kunstmaan\NodeBundle\Entity\NodeVersion;
24
use Kunstmaan\NodeBundle\Entity\QueuedNodeTranslationAction;
25
use Kunstmaan\NodeBundle\Event\AdaptFormEvent;
26
use Kunstmaan\NodeBundle\Event\CopyPageTranslationNodeEvent;
27
use Kunstmaan\NodeBundle\Event\Events;
28
use Kunstmaan\NodeBundle\Event\NodeEvent;
29
use Kunstmaan\NodeBundle\Event\RecopyPageTranslationNodeEvent;
30
use Kunstmaan\NodeBundle\Event\RevertNodeAction;
31
use Kunstmaan\NodeBundle\Form\NodeMenuTabAdminType;
32
use Kunstmaan\NodeBundle\Form\NodeMenuTabTranslationAdminType;
33
use Kunstmaan\NodeBundle\Helper\NodeAdmin\NodeAdminPublisher;
34
use Kunstmaan\NodeBundle\Helper\NodeAdmin\NodeVersionLockHelper;
35
use Kunstmaan\NodeBundle\Helper\PageCloningHelper;
36
use Kunstmaan\NodeBundle\Repository\NodeVersionRepository;
37
use Kunstmaan\UtilitiesBundle\Helper\ClassLookup;
38
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
39
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
40
use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy;
41
use Symfony\Component\HttpFoundation\JsonResponse;
42
use Symfony\Component\HttpFoundation\RedirectResponse;
43
use Symfony\Component\HttpFoundation\Request;
44
use Symfony\Component\Routing\Annotation\Route;
45
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
46
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
47
use Symfony\Component\Translation\TranslatorInterface;
48
49
/**
50
 * NodeAdminController
51
 */
52
class NodeAdminController extends Controller
0 ignored issues
show
Deprecated Code introduced by Daan Poron
The class Symfony\Bundle\Framework...e\Controller\Controller has been deprecated with message: since Symfony 4.2, use "Symfony\Bundle\FrameworkBundle\Controller\AbstractController" instead.

This class, trait or interface has been deprecated. The supplier of the file has supplied an explanatory message.

The explanatory message should give you some clue as to whether and when the type will be removed from the class and what other constant to use instead.

Loading history...
53
{
54
    /**
55
     * @var EntityManager
56
     */
57
    protected $em;
58
59
    /**
60
     * @var string
61
     */
62
    protected $locale;
63
64
    /**
65
     * @var AuthorizationCheckerInterface
66
     */
67
    protected $authorizationChecker;
68
69
    /**
70
     * @var BaseUser
71
     */
72
    protected $user;
73
74
    /**
75
     * @var AclHelper
76
     */
77
    protected $aclHelper;
78
79
    /**
80
     * @var AclManager
81
     */
82
    protected $aclManager;
83
84
    /**
85
     * @var NodeAdminPublisher
86
     */
87
    protected $nodePublisher;
88
89
    /**
90
     * @var TranslatorInterface
91
     */
92
    protected $translator;
93
94
    /** @var PageCloningHelper */
95
    private $pageCloningHelper;
96
97
    /**
98
     * init
99
     *
100
     * @param Request $request
101
     */
102
    protected function init(Request $request)
103
    {
104
        $this->em = $this->getDoctrine()->getManager();
105
        $this->locale = $request->getLocale();
106
        $this->authorizationChecker = $this->container->get('security.authorization_checker');
107
        $this->user = $this->getUser();
108
        $this->aclHelper = $this->container->get('kunstmaan_admin.acl.helper');
109
        $this->aclManager = $this->container->get('kunstmaan_admin.acl.manager');
110
        $this->nodePublisher = $this->container->get('kunstmaan_node.admin_node.publisher');
111
        $this->translator = $this->container->get('translator');
112
        $this->pageCloningHelper = $this->container->get(PageCloningHelper::class);
113
    }
114
115
    /**
116
     * @Route("/", name="KunstmaanNodeBundle_nodes")
117
     * @Template("@KunstmaanNode/Admin/list.html.twig")
118
     *
119
     * @param Request $request
120
     *
121
     * @return array
0 ignored issues
show
Documentation introduced by Kris Pypen
Consider making the return type a bit more specific; maybe use array<string,AdminList>.

This check looks for the generic type array as a return type and suggests a more specific type. This type is inferred from the actual code.

Loading history...
122
     */
123
    public function indexAction(Request $request)
124
    {
125
        $this->init($request);
126
127
        $nodeAdminListConfigurator = new NodeAdminListConfigurator(
128
            $this->em,
129
            $this->aclHelper,
130
            $this->locale,
131
            PermissionMap::PERMISSION_VIEW,
132
            $this->authorizationChecker
133
        );
134
135
        $locale = $this->locale;
136
        $acl = $this->authorizationChecker;
137
        $itemRoute = function (EntityInterface $item) use ($locale, $acl) {
138
            if ($acl->isGranted(PermissionMap::PERMISSION_VIEW, $item->getNode())) {
0 ignored issues
show
Bug introduced by whitewhidow
It seems like you code against a concrete implementation and not the interface Kunstmaan\AdminBundle\Entity\EntityInterface as the method getNode() does only exist in the following implementations of said interface: Kunstmaan\FormBundle\Entity\FormSubmission, Kunstmaan\FormBundle\Tes...List\FakeFormSubmission, Kunstmaan\NodeBundle\Entity\NodeTranslation, Kunstmaan\NodeSearchBundle\Entity\NodeSearch.

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...
139
                return array(
140
                    'path' => '_slug_preview',
141
                    'params' => ['_locale' => $locale, 'url' => $item->getUrl()],
0 ignored issues
show
Bug introduced by whitewhidow
It seems like you code against a concrete implementation and not the interface Kunstmaan\AdminBundle\Entity\EntityInterface as the method getUrl() does only exist in the following implementations of said interface: Kunstmaan\AdminBundle\Entity\Exception, Kunstmaan\MediaBundle\Entity\Media, Kunstmaan\MenuBundle\Entity\BaseMenuItem, Kunstmaan\MenuBundle\Entity\MenuItem, Kunstmaan\NodeBundle\Entity\NodeTranslation, Kunstmaan\PagePartBundle\Entity\LinkPagePart.

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...
142
                );
143
            }
144
        };
145
        $nodeAdminListConfigurator->addSimpleItemAction('action.preview', $itemRoute, 'eye');
146
        $nodeAdminListConfigurator->setDomainConfiguration($this->get('kunstmaan_admin.domain_configuration'));
147
        $nodeAdminListConfigurator->setShowAddHomepage($this->getParameter('kunstmaan_node.show_add_homepage') && $this->isGranted('ROLE_SUPER_ADMIN'));
148
149
        /** @var AdminList $adminlist */
150
        $adminlist = $this->get('kunstmaan_adminlist.factory')->createList($nodeAdminListConfigurator);
151
        $adminlist->bindRequest($request);
152
153
        return array(
154
            'adminlist' => $adminlist,
155
        );
156
    }
157
158
    /**
159
     * @Route(
160
     *      "/{id}/copyfromotherlanguage",
161
     *      requirements={"id" = "\d+"},
162
     *      name="KunstmaanNodeBundle_nodes_copyfromotherlanguage",
163
     *      methods={"GET"}
164
     * )
165
     *
166
     * @param Request $request
167
     * @param int     $id      The node id
168
     *
169
     * @return RedirectResponse
170
     *
171
     * @throws AccessDeniedException
172
     */
173
    public function copyFromOtherLanguageAction(Request $request, $id)
174
    {
175
        $this->init($request);
176
        /* @var Node $node */
177
        $node = $this->em->getRepository(Node::class)->find($id);
178
179
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
180
181
        $originalLanguage = $request->get('originallanguage');
182
        $otherLanguageNodeTranslation = $node->getNodeTranslation($originalLanguage, true);
183
        $otherLanguageNodeNodeVersion = $otherLanguageNodeTranslation->getPublicNodeVersion();
184
        $otherLanguagePage = $otherLanguageNodeNodeVersion->getRef($this->em);
185
        $myLanguagePage = $this->get('kunstmaan_admin.clone.helper')
186
            ->deepCloneAndSave($otherLanguagePage);
187
188
        /* @var NodeTranslation $nodeTranslation */
189
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeTranslationFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

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...
190
            ->createNodeTranslationFor($myLanguagePage, $this->locale, $node, $this->user);
191
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
192
193
        $this->dispatch(
194
            new CopyPageTranslationNodeEvent(
195
                $node,
196
                $nodeTranslation,
197
                $nodeVersion,
198
                $myLanguagePage,
199
                $otherLanguageNodeTranslation,
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $otherLanguageNodeTranslation defined by $node->getNodeTranslatio...originalLanguage, true) on line 182 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
200
                $otherLanguageNodeNodeVersion,
201
                $otherLanguagePage,
202
                $originalLanguage
203
            ),
204
            Events::COPY_PAGE_TRANSLATION
205
        );
206
207
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
208
    }
209
210
    /**
211
     * @Route(
212
     *      "/{id}/recopyfromotherlanguage",
213
     *      requirements={"id" = "\d+"},
214
     *      name="KunstmaanNodeBundle_nodes_recopyfromotherlanguage",
215
     *      methods={"POST"}
216
     * )
217
     *
218
     * @param Request $request
219
     * @param int     $id      The node id
220
     *
221
     * @return RedirectResponse
222
     *
223
     * @throws AccessDeniedException
224
     */
225
    public function recopyFromOtherLanguageAction(Request $request, $id)
226
    {
227
        $this->init($request);
228
        /* @var Node $node */
229
        $node = $this->em->getRepository(Node::class)->find($id);
230
231
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
232
233
        $otherLanguageNodeTranslation = $this->em->getRepository(NodeTranslation::class)->find($request->get('source'));
234
        $otherLanguageNodeNodeVersion = $otherLanguageNodeTranslation->getPublicNodeVersion();
235
        $otherLanguagePage = $otherLanguageNodeNodeVersion->getRef($this->em);
236
        $myLanguagePage = $this->get('kunstmaan_admin.clone.helper')
237
            ->deepCloneAndSave($otherLanguagePage);
238
239
        /* @var NodeTranslation $nodeTranslation */
240
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method addDraftNodeVersionFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

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...
241
            ->addDraftNodeVersionFor($myLanguagePage, $this->locale, $node, $this->user);
242
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
243
244
        $this->dispatch(
245
            new RecopyPageTranslationNodeEvent(
246
                $node,
247
                $nodeTranslation,
248
                $nodeVersion,
249
                $myLanguagePage,
250
                $otherLanguageNodeTranslation,
0 ignored issues
show
Documentation introduced by Sander Goossens
$otherLanguageNodeTranslation is of type object|null, but the function expects a object<Kunstmaan\NodeBun...Entity\NodeTranslation>.

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...
251
                $otherLanguageNodeNodeVersion,
252
                $otherLanguagePage,
253
                $otherLanguageNodeTranslation->getLang()
254
            ),
255
            Events::RECOPY_PAGE_TRANSLATION
256
        );
257
258
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id, 'subaction' => NodeVersion::DRAFT_VERSION)));
259
    }
260
261
    /**
262
     * @Route(
263
     *      "/{id}/createemptypage",
264
     *      requirements={"id" = "\d+"},
265
     *      name="KunstmaanNodeBundle_nodes_createemptypage",
266
     *      methods={"GET"}
267
     * )
268
     *
269
     * @param Request $request
270
     * @param int     $id
271
     *
272
     * @return RedirectResponse
273
     *
274
     * @throws AccessDeniedException
275
     */
276
    public function createEmptyPageAction(Request $request, $id)
277
    {
278
        $this->init($request);
279
        /* @var Node $node */
280
        $node = $this->em->getRepository(Node::class)->find($id);
281
282
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
283
284
        $entityName = $node->getRefEntityName();
285
        /* @var HasNodeInterface $myLanguagePage */
286
        $myLanguagePage = new $entityName();
287
        $myLanguagePage->setTitle('New page');
288
289
        $this->em->persist($myLanguagePage);
290
        $this->em->flush();
291
        /* @var NodeTranslation $nodeTranslation */
292
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeTranslationFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

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...
293
            ->createNodeTranslationFor($myLanguagePage, $this->locale, $node, $this->user);
294
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
295
296
        $this->dispatch(
297
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $myLanguagePage),
298
            Events::ADD_EMPTY_PAGE_TRANSLATION
299
        );
300
301
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
302
    }
303
304
    /**
305
     * @Route("/{id}/publish", requirements={"id" =
306
     *                         "\d+"},
307
     *                         name="KunstmaanNodeBundle_nodes_publish", methods={"GET", "POST"})
308
     *
309
     * @param Request $request
310
     * @param int     $id
311
     *
312
     * @return RedirectResponse
313
     *
314
     * @throws AccessDeniedException
315
     */
316 View Code Duplication
    public function publishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
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...
317
    {
318
        $this->init($request);
319
        /* @var Node $node */
320
        $node = $this->em->getRepository(Node::class)->find($id);
321
322
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
323
        $request = $this->get('request_stack')->getCurrentRequest();
324
        $this->nodePublisher->chooseHowToPublish($request, $nodeTranslation, $this->translator);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 322 can be null; however, Kunstmaan\NodeBundle\Hel...r::chooseHowToPublish() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
325
326
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $node->getId())));
327
    }
328
329
    /**
330
     * @Route(
331
     *      "/{id}/unpublish",
332
     *      requirements={"id" = "\d+"},
333
     *      name="KunstmaanNodeBundle_nodes_unpublish",
334
     *      methods={"GET", "POST"}
335
     * )
336
     *
337
     * @param Request $request
338
     * @param int     $id
339
     *
340
     * @return RedirectResponse
341
     *
342
     * @throws AccessDeniedException
343
     */
344 View Code Duplication
    public function unPublishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
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...
345
    {
346
        $this->init($request);
347
        /* @var Node $node */
348
        $node = $this->em->getRepository(Node::class)->find($id);
349
350
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
351
        $request = $this->get('request_stack')->getCurrentRequest();
352
        $this->nodePublisher->chooseHowToUnpublish($request, $nodeTranslation, $this->translator);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 350 can be null; however, Kunstmaan\NodeBundle\Hel...:chooseHowToUnpublish() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
353
354
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $node->getId())));
355
    }
356
357
    /**
358
     * @Route(
359
     *      "/{id}/unschedulepublish",
360
     *      requirements={"id" = "\d+"},
361
     *      name="KunstmaanNodeBundle_nodes_unschedule_publish",
362
     *      methods={"GET", "POST"}
363
     * )
364
     *
365
     * @param Request $request
366
     * @param int     $id
367
     *
368
     * @return RedirectResponse
369
     *
370
     * @throws AccessDeniedException
371
     */
372 View Code Duplication
    public function unSchedulePublishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
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...
373
    {
374
        $this->init($request);
375
376
        /* @var Node $node */
377
        $node = $this->em->getRepository(Node::class)->find($id);
378
379
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
380
        $this->nodePublisher->unSchedulePublish($nodeTranslation);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 379 can be null; however, Kunstmaan\NodeBundle\Hel...er::unSchedulePublish() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
381
382
        $this->addFlash(
383
            FlashTypes::SUCCESS,
384
            $this->get('translator')->trans('kuma_node.admin.unschedule.flash.success')
385
        );
386
387
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
388
    }
389
390
    /**
391
     * @Route(
392
     *      "/{id}/delete",
393
     *      requirements={"id" = "\d+"},
394
     *      name="KunstmaanNodeBundle_nodes_delete",
395
     *      methods={"POST"}
396
     * )
397
     *
398
     * @param Request $request
399
     * @param int     $id
400
     *
401
     * @return RedirectResponse
402
     *
403
     * @throws AccessDeniedException
404
     */
405
    public function deleteAction(Request $request, $id)
406
    {
407
        $this->init($request);
408
        /* @var Node $node */
409
        $node = $this->em->getRepository(Node::class)->find($id);
410
411
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_DELETE, $node);
412
413
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
414
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
415
        $page = $nodeVersion->getRef($this->em);
416
417
        $this->dispatch(
418
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page),
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 413 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
419
            Events::PRE_DELETE
420
        );
421
422
        $node->setDeleted(true);
423
        $this->em->persist($node);
424
425
        $children = $node->getChildren();
426
        $this->deleteNodeChildren($this->em, $this->user, $this->locale, $children);
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $children defined by $node->getChildren() on line 425 can also be of type array<integer,object<Kun...odeBundle\Entity\Node>>; however, Kunstmaan\NodeBundle\Con...r::deleteNodeChildren() does only seem to accept object<Doctrine\Common\C...ctions\ArrayCollection>, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
427
        $this->em->flush();
428
429
        $event = new NodeEvent($node, $nodeTranslation, $nodeVersion, $page);
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 413 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
430
        $this->dispatch($event, Events::POST_DELETE);
431
        if (null === $response = $event->getResponse()) {
432
            $nodeParent = $node->getParent();
433
            // Check if we have a parent. Otherwise redirect to pages overview.
434
            if ($nodeParent) {
435
                $url = $this->get('router')->generate(
436
                    'KunstmaanNodeBundle_nodes_edit',
437
                    array('id' => $nodeParent->getId())
438
                );
439
            } else {
440
                $url = $this->get('router')->generate(
441
                    'KunstmaanNodeBundle_nodes'
442
                );
443
            }
444
            $response = new RedirectResponse($url);
445
        }
446
447
        $this->addFlash(
448
            FlashTypes::SUCCESS,
449
            $this->get('translator')->trans('kuma_node.admin.delete.flash.success')
450
        );
451
452
        return $response;
453
    }
454
455
    /**
456
     * @Route(
457
     *      "/{id}/duplicate",
458
     *      requirements={"id" = "\d+"},
459
     *      name="KunstmaanNodeBundle_nodes_duplicate",
460
     *      methods={"POST"}
461
     * )
462
     *
463
     * @param Request $request
464
     * @param int     $id
465
     *
466
     * @return RedirectResponse
467
     *
468
     * @throws AccessDeniedException
469
     */
470
    public function duplicateAction(Request $request, $id)
471
    {
472
        $this->init($request);
473
        /* @var Node $parentNode */
474
        $originalNode = $this->em->getRepository(Node::class)
475
            ->find($id);
476
477
        // Check with Acl
478
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $originalNode);
479
480
        $request = $this->get('request_stack')->getCurrentRequest();
481
482
        $originalNodeTranslations = $originalNode->getNodeTranslation($this->locale, true);
483
        $originalRef = $originalNodeTranslations->getPublicNodeVersion()->getRef($this->em);
484
        $newPage = $this->get('kunstmaan_admin.clone.helper')
485
            ->deepCloneAndSave($originalRef);
486
487
        //set the title
488
        $title = $request->get('title');
489
        if (\is_string($title) && !empty($title)) {
490
            $newPage->setTitle($title);
491
        } else {
492
            $newPage->setTitle('New page');
493
        }
494
495
        //set the parent
496
        $parentNodeTranslation = $originalNode->getParent()->getNodeTranslation($this->locale, true);
497
        $parent = $parentNodeTranslation->getPublicNodeVersion()->getRef($this->em);
498
        $newPage->setParent($parent);
499
        $this->em->persist($newPage);
500
        $this->em->flush();
501
502
        /* @var Node $nodeNewPage */
503
        $nodeNewPage = $this->em->getRepository(Node::class)->createNodeFor(
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

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...
504
            $newPage,
505
            $this->locale,
506
            $this->user
507
        );
508
509
        $nodeTranslation = $nodeNewPage->getNodeTranslation($this->locale, true);
510
        if ($newPage->isStructureNode()) {
511
            $nodeTranslation->setSlug('');
512
            $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by woutervandamme
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 509 can be null; however, Doctrine\ORM\EntityManager::persist() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
513
        }
514
        $this->em->flush();
515
516
        $this->aclManager->updateNodeAcl($originalNode, $nodeNewPage);
0 ignored issues
show
Documentation introduced by Peter Zentjens
$originalNode is of type object|null, but the function expects a object<Kunstmaan\NodeBundle\Entity\Node>.

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...
517
518
        $this->addFlash(
519
            FlashTypes::SUCCESS,
520
            $this->get('translator')->trans('kuma_node.admin.duplicate.flash.success')
521
        );
522
523
        return $this->redirect(
524
            $this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $nodeNewPage->getId()))
525
        );
526
    }
527
528
    /**
529
     * @Route(
530
     *      "/{id}/duplicate-with-children",
531
     *      requirements={"id" = "\d+"},
532
     *      name="KunstmaanNodeBundle_nodes_duplicate_with_children",
533
     *      methods={"POST"}
534
     * )
535
     *
536
     * @throws AccessDeniedException
537
     */
538
    public function duplicateWithChildrenAction(Request $request, $id)
539
    {
540
        if (!$this->getParameter('kunstmaan_node.show_duplicate_with_children')) {
541
            return $this->redirectToRoute('KunstmaanNodeBundle_nodes_edit', ['id' => $id]);
542
        }
543
544
        $this->init($request);
545
        $title = $request->get('title', null);
546
547
        $nodeNewPage = $this->pageCloningHelper->duplicateWithChildren($id, $this->locale, $this->user, $title);
548
549
        $this->addFlash(FlashTypes::SUCCESS, $this->get('translator')->trans('kuma_node.admin.duplicate.flash.success'));
550
551
        return $this->redirectToRoute('KunstmaanNodeBundle_nodes_edit', ['id' => $nodeNewPage->getId()]);
552
    }
553
554
    /**
555
     * @Route(
556
     *      "/{id}/revert",
557
     *      requirements={"id" = "\d+"},
558
     *      defaults={"subaction" = "public"},
559
     *      name="KunstmaanNodeBundle_nodes_revert",
560
     *      methods={"GET"}
561
     * )
562
     *
563
     * @param Request $request
564
     * @param int     $id      The node id
565
     *
566
     * @return RedirectResponse
567
     *
568
     * @throws AccessDeniedException
569
     * @throws InvalidArgumentException
570
     */
571
    public function revertAction(Request $request, $id)
572
    {
573
        $this->init($request);
574
        /* @var Node $node */
575
        $node = $this->em->getRepository(Node::class)->find($id);
576
577
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
578
579
        $version = $request->get('version');
580
581
        if (empty($version) || !is_numeric($version)) {
582
            throw new InvalidArgumentException('No version was specified');
583
        }
584
585
        /* @var NodeVersionRepository $nodeVersionRepo */
586
        $nodeVersionRepo = $this->em->getRepository(NodeVersion::class);
587
        /* @var NodeVersion $nodeVersion */
588
        $nodeVersion = $nodeVersionRepo->find($version);
589
590
        if (\is_null($nodeVersion)) {
591
            throw new InvalidArgumentException('Version does not exist');
592
        }
593
594
        /* @var NodeTranslation $nodeTranslation */
595
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
596
        $page = $nodeVersion->getRef($this->em);
597
        /* @var HasNodeInterface $clonedPage */
598
        $clonedPage = $this->get('kunstmaan_admin.clone.helper')
599
            ->deepCloneAndSave($page);
600
        $newNodeVersion = $nodeVersionRepo->createNodeVersionFor(
601
            $clonedPage,
602
            $nodeTranslation,
603
            $this->user,
604
            $nodeVersion,
605
            'draft'
606
        );
607
608
        $nodeTranslation->setTitle($clonedPage->getTitle());
609
        $this->em->persist($nodeTranslation);
610
        $this->em->flush();
611
612
        $this->dispatch(
613
            new RevertNodeAction(
614
                $node,
615
                $nodeTranslation,
616
                $newNodeVersion,
617
                $clonedPage,
618
                $nodeVersion,
619
                $page
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $page defined by $nodeVersion->getRef($this->em) on line 596 can be null; however, Kunstmaan\NodeBundle\Eve...deAction::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
620
            ),
621
            Events::REVERT
622
        );
623
624
        $this->addFlash(
625
            FlashTypes::SUCCESS,
626
            $this->get('translator')->trans('kuma_node.admin.revert.flash.success')
627
        );
628
629
        return $this->redirect(
630
            $this->generateUrl(
631
                'KunstmaanNodeBundle_nodes_edit',
632
                array(
633
                    'id' => $id,
634
                    'subaction' => 'draft',
635
                )
636
            )
637
        );
638
    }
639
640
    /**
641
     * @Route(
642
     *      "/{id}/add",
643
     *      requirements={"id" = "\d+"},
644
     *      name="KunstmaanNodeBundle_nodes_add",
645
     *      methods={"POST"}
646
     * )
647
     *
648
     * @param Request $request
649
     * @param int     $id
650
     *
651
     * @return RedirectResponse
652
     *
653
     * @throws AccessDeniedException
654
     * @throws InvalidArgumentException
655
     */
656
    public function addAction(Request $request, $id)
657
    {
658
        $this->init($request);
659
        /* @var Node $parentNode */
660
        $parentNode = $this->em->getRepository(Node::class)->find($id);
661
662
        // Check with Acl
663
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $parentNode);
664
665
        $parentNodeTranslation = $parentNode->getNodeTranslation($this->locale, true);
666
        $parentNodeVersion = $parentNodeTranslation->getPublicNodeVersion();
667
        $parentPage = $parentNodeVersion->getRef($this->em);
668
669
        $type = $this->validatePageType($request);
670
        $newPage = $this->createNewPage($request, $type);
671
        $newPage->setParent($parentPage);
672
673
        /* @var Node $nodeNewPage */
674
        $nodeNewPage = $this->em->getRepository(Node::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

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...
675
            ->createNodeFor($newPage, $this->locale, $this->user);
676
        $nodeTranslation = $nodeNewPage->getNodeTranslation(
677
            $this->locale,
678
            true
679
        );
680
        $weight = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getMaxChildrenWeight() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

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...
681
                ->getMaxChildrenWeight($parentNode, $this->locale) + 1;
682
        $nodeTranslation->setWeight($weight);
683
684
        if ($newPage->isStructureNode()) {
685
            $nodeTranslation->setSlug('');
686
        }
687
688
        $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by mdx
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 676 can be null; however, Doctrine\ORM\EntityManager::persist() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
689
        $this->em->flush();
690
691
        $this->aclManager->updateNodeAcl($parentNode, $nodeNewPage);
692
693
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
694
695
        $this->dispatch(
696
            new NodeEvent(
697
                $nodeNewPage, $nodeTranslation, $nodeVersion, $newPage
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 676 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
698
            ),
699
            Events::ADD_NODE
700
        );
701
702
        return $this->redirect(
703
            $this->generateUrl(
704
                'KunstmaanNodeBundle_nodes_edit',
705
                array('id' => $nodeNewPage->getId())
706
            )
707
        );
708
    }
709
710
    /**
711
     * @Route("/add-homepage", name="KunstmaanNodeBundle_nodes_add_homepage", methods={"POST"})
712
     *
713
     * @return RedirectResponse
714
     *
715
     * @throws AccessDeniedException
716
     * @throws InvalidArgumentException
717
     */
718
    public function addHomepageAction(Request $request)
719
    {
720
        $this->init($request);
721
722
        // Check with Acl
723
        $this->denyAccessUnlessGranted('ROLE_SUPER_ADMIN');
724
725
        $type = $this->validatePageType($request);
726
727
        $newPage = $this->createNewPage($request, $type);
728
729
        /* @var Node $nodeNewPage */
730
        $nodeNewPage = $this->em->getRepository(Node::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

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...
731
            ->createNodeFor($newPage, $this->locale, $this->user);
732
        $nodeTranslation = $nodeNewPage->getNodeTranslation(
733
            $this->locale,
734
            true
735
        );
736
        $this->em->flush();
737
738
        // Set default permissions
739
        $this->container->get('kunstmaan_node.acl_permission_creator_service')
740
            ->createPermission($nodeNewPage);
741
742
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
743
744
        $this->dispatch(
745
            new NodeEvent(
746
                $nodeNewPage, $nodeTranslation, $nodeVersion, $newPage
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 732 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
747
            ),
748
            Events::ADD_NODE
749
        );
750
751
        return $this->redirect(
752
            $this->generateUrl(
753
                'KunstmaanNodeBundle_nodes_edit',
754
                array('id' => $nodeNewPage->getId())
755
            )
756
        );
757
    }
758
759
    /**
760
     * @Route("/reorder", name="KunstmaanNodeBundle_nodes_reorder", methods={"POST"})
761
     *
762
     * @param Request $request
763
     *
764
     * @return string
0 ignored issues
show
Documentation introduced by Matthieu Cornut
Should the return type not be JsonResponse?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
765
     *
766
     * @throws AccessDeniedException
767
     */
768
    public function reorderAction(Request $request)
769
    {
770
        $this->init($request);
771
        $nodes = array();
772
        $nodeIds = $request->get('nodes');
773
        $changeParents = $request->get('parent');
774
775
        foreach ($nodeIds as $id) {
776
            /* @var Node $node */
777
            $node = $this->em->getRepository(Node::class)->find($id);
778
            $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
779
            $nodes[] = $node;
780
        }
781
782
        $weight = 1;
783
        foreach ($nodes as $node) {
784
            $newParentId = isset($changeParents[$node->getId()]) ? $changeParents[$node->getId()] : null;
785
            if ($newParentId) {
786
                $parent = $this->em->getRepository(Node::class)->find($newParentId);
787
                $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $parent);
788
                $node->setParent($parent);
789
                $this->em->persist($node);
790
                $this->em->flush($node);
791
            }
792
793
            /* @var NodeTranslation $nodeTranslation */
794
            $nodeTranslation = $node->getNodeTranslation($this->locale, true);
795
796
            if ($nodeTranslation) {
797
                $nodeVersion = $nodeTranslation->getPublicNodeVersion();
798
                $page = $nodeVersion->getRef($this->em);
799
800
                $this->dispatch(
801
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page),
0 ignored issues
show
Bug introduced by Samuele Mazza
It seems like $page defined by $nodeVersion->getRef($this->em) on line 798 can be null; however, Kunstmaan\NodeBundle\Eve...odeEvent::__construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
802
                    Events::PRE_PERSIST
803
                );
804
805
                $nodeTranslation->setWeight($weight);
806
                $this->em->persist($nodeTranslation);
807
                $this->em->flush($nodeTranslation);
808
809
                $this->dispatch(
810
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page),
0 ignored issues
show
Bug introduced by Samuele Mazza
It seems like $page defined by $nodeVersion->getRef($this->em) on line 798 can be null; however,