NodeAdminController   F
last analyzed

Complexity

Total Complexity 77

Size/Duplication

Total Lines 1273
Duplicated Lines 8.88 %

Coupling/Cohesion

Components 1
Dependencies 32

Test Coverage

Coverage 0%

Importance

Changes 0
Metric Value
wmc 77
lcom 1
cbo 32
dl 113
loc 1273
ccs 0
cts 755
cp 0
c 0
b 0
f 0
rs 1.04

25 Methods

Rating   Name   Duplication   Size   Complexity  
A init() 0 12 1
A indexAction() 0 34 3
A copyFromOtherLanguageAction() 0 36 1
A recopyFromOtherLanguageAction() 0 35 1
A createEmptyPageAction() 0 27 1
A publishAction() 12 12 1
A unPublishAction() 12 12 1
A unSchedulePublishAction() 17 17 1
A deleteAction() 0 49 3
B duplicateAction() 0 57 4
A duplicateWithChildrenAction() 0 15 2
B revertAction() 0 68 4
A addAction() 0 53 2
A addHomepageAction() 0 40 1
B reorderAction() 0 56 6
F editAction() 18 206 24
A checkNodeVersionLockAction() 0 29 4
A isNodeVersionLocked() 0 12 2
A createDraftVersion() 0 41 1
A checkPermission() 0 6 2
A deleteNodeChildren() 43 43 2
A createNewPage() 0 16 3
A validatePageType() 0 10 2
A renderNodeNotTranslatedPage() 0 38 3
A dispatch() 11 11 2

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like NodeAdminController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use NodeAdminController, and based on these observations, apply Extract Interface, too.

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
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
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
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
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
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
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
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
$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
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
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
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
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
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
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
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
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
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
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
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
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
$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
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
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
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
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
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
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
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
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
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
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...
811
                    Events::POST_PERSIST
812
                );
813
814
                ++$weight;
815
            }
816
        }
817
818
        return new JsonResponse(
819
            array(
820
                'Success' => 'The node-translations for [' . $this->locale . '] have got new weight values',
821
            )
822
        );
823
    }
824
825
    /**
826
     * @Route(
827
     *      "/{id}/{subaction}",
828
     *      requirements={"id" = "\d+"},
829
     *      defaults={"subaction" = "public"},
830
     *      name="KunstmaanNodeBundle_nodes_edit",
831
     *      methods={"GET", "POST"}
832
     * )
833
     * @Template("@KunstmaanNode/NodeAdmin/edit.html.twig")
834
     *
835
     * @param Request $request
836
     * @param int     $id        The node id
837
     * @param string  $subaction The subaction (draft|public)
838
     *
839
     * @return RedirectResponse|array
0 ignored issues
show
Documentation introduced by
Should the return type not be \Symfony\Component\HttpFoundation\Response|array?

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...
840
     *
841
     * @throws AccessDeniedException
842
     */
843
    public function editAction(Request $request, $id, $subaction)
844
    {
845
        $this->init($request);
846
        /* @var Node $node */
847
        $node = $this->em->getRepository(Node::class)->find($id);
848
849
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
850
851
        $tabPane = new TabPane(
852
            'todo',
853
            $request,
854
            $this->container->get('form.factory')
0 ignored issues
show
Documentation introduced by
$this->container->get('form.factory') is of type object|null, but the function expects a object<Symfony\Component...m\FormFactoryInterface>.

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...
855
        );
856
857
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
858
        if (!$nodeTranslation) {
859
            return $this->renderNodeNotTranslatedPage($node);
860
        }
861
862
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
863
        $draftNodeVersion = $nodeTranslation->getDraftNodeVersion();
864
        $nodeVersionIsLocked = false;
865
866
        /* @var HasNodeInterface $page */
867
        $page = null;
0 ignored issues
show
Unused Code introduced by
$page is not used, you could remove the assignment.

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

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

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

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

Loading history...
868
        $draft = ($subaction == 'draft');
869
        $saveAsDraft = $request->get('saveasdraft');
870
        if ((!$draft && !empty($saveAsDraft)) || ($draft && \is_null($draftNodeVersion))) {
871
            // Create a new draft version
872
            $draft = true;
873
            $subaction = 'draft';
874
            $page = $nodeVersion->getRef($this->em);
875
            $nodeVersion = $this->createDraftVersion(
876
                $page,
0 ignored issues
show
Bug introduced by
It seems like $page defined by $nodeVersion->getRef($this->em) on line 874 can be null; however, Kunstmaan\NodeBundle\Con...r::createDraftVersion() 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...
877
                $nodeTranslation,
878
                $nodeVersion
879
            );
880
            $draftNodeVersion = $nodeVersion;
881
        } elseif ($draft) {
882
            $nodeVersion = $draftNodeVersion;
883
            $page = $nodeVersion->getRef($this->em);
884
        } else {
885
            if ($request->getMethod() == 'POST') {
886
                $nodeVersionIsLocked = $this->isNodeVersionLocked($nodeTranslation, true);
887
888
                //Check the version timeout and make a new nodeversion if the timeout is passed
889
                $thresholdDate = date(
890
                    'Y-m-d H:i:s',
891
                    time() - $this->getParameter(
892
                        'kunstmaan_node.version_timeout'
893
                    )
894
                );
895
                $updatedDate = date(
896
                    'Y-m-d H:i:s',
897
                    strtotime($nodeVersion->getUpdated()->format('Y-m-d H:i:s'))
898
                );
899 View Code Duplication
                if ($thresholdDate >= $updatedDate || $nodeVersionIsLocked) {
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...
900
                    $page = $nodeVersion->getRef($this->em);
901
                    if ($nodeVersion === $nodeTranslation->getPublicNodeVersion()) {
902
                        $this->nodePublisher
903
                            ->createPublicVersion(
904
                                $page,
0 ignored issues
show
Bug introduced by
It seems like $page defined by $nodeVersion->getRef($this->em) on line 900 can be null; however, Kunstmaan\NodeBundle\Hel...::createPublicVersion() 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...
905
                                $nodeTranslation,
906
                                $nodeVersion,
907
                                $this->user
908
                            );
909
                    } else {
910
                        $this->createDraftVersion(
911
                            $page,
0 ignored issues
show
Bug introduced by
It seems like $page defined by $nodeVersion->getRef($this->em) on line 900 can be null; however, Kunstmaan\NodeBundle\Con...r::createDraftVersion() 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...
912
                            $nodeTranslation,
913
                            $nodeVersion
914
                        );
915
                    }
916
                }
917
            }
918
            $page = $nodeVersion->getRef($this->em);
919
        }
920
        $isStructureNode = $page->isStructureNode();
921
922
        $menubuilder = $this->get('kunstmaan_node.actions_menu_builder');
923
        $menubuilder->setActiveNodeVersion($nodeVersion);
924
        $menubuilder->setEditableNode(!$isStructureNode);
925
926
        // Building the form
927
        $propertiesWidget = new FormWidget();
928
        $propertiesWidget->addType('main', $page->getDefaultAdminType(), $page);
929
        $propertiesWidget->addType('node', $node->getDefaultAdminType(), $node);
930
        $tabPane->addTab(new Tab('kuma_node.tab.properties.title', $propertiesWidget));
931
932
        // Menu tab
933
        $menuWidget = new FormWidget();
934
        $menuWidget->addType(
935
            'menunodetranslation',
936
            NodeMenuTabTranslationAdminType::class,
937
            $nodeTranslation,
938
            ['slugable' => !$isStructureNode]
939
        );
940
        $menuWidget->addType('menunode', NodeMenuTabAdminType::class, $node, ['available_in_nav' => !$isStructureNode]);
941
        $tabPane->addTab(new Tab('kuma_node.tab.menu.title', $menuWidget));
942
943
        $this->dispatch(
944
            new AdaptFormEvent(
945
                $request,
946
                $tabPane,
947
                $page,
948
                $node,
949
                $nodeTranslation,
950
                $nodeVersion
951
            ),
952
            Events::ADAPT_FORM
953
        );
954
955
        $tabPane->buildForm();
956
957
        if ($request->getMethod() == 'POST') {
958
            $tabPane->bindRequest($request);
959
960
            // Don't redirect to listing when coming from ajax request, needed for url chooser.
961
            if ($tabPane->isValid() && !$request->isXmlHttpRequest()) {
962
                $this->dispatch(
963
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page),
0 ignored issues
show
Bug introduced by
It seems like $nodeVersion defined by $draftNodeVersion on line 882 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...
964
                    Events::PRE_PERSIST
965
                );
966
967
                $nodeTranslation->setTitle($page->getTitle());
968
                if ($isStructureNode) {
969
                    $nodeTranslation->setSlug('');
970
                }
971
                $nodeVersion->setUpdated(new DateTime());
972
                if ($nodeVersion->getType() == 'public') {
973
                    $nodeTranslation->setUpdated($nodeVersion->getUpdated());
974
                }
975
                $this->em->persist($nodeTranslation);
976
                $this->em->persist($nodeVersion);
0 ignored issues
show
Bug introduced by
It seems like $nodeVersion defined by $draftNodeVersion on line 882 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...
977
                $tabPane->persist($this->em);
978
                $this->em->flush();
979
980
                $this->dispatch(
981
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page),
0 ignored issues
show
Bug introduced by
It seems like $nodeVersion defined by $draftNodeVersion on line 882 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...
982
                    Events::POST_PERSIST
983
                );
984
985
                if ($nodeVersionIsLocked) {
986
                    $this->addFlash(
987
                        FlashTypes::SUCCESS,
988
                        $this->get('translator')->trans('kuma_node.admin.edit.flash.locked_success')
989
                    );
990
                } elseif ($request->request->has('publishing') || $request->request->has('publish_later')) {
991
                    $this->nodePublisher->chooseHowToPublish($request, $nodeTranslation, $this->translator);
992
                } elseif ($request->request->has('unpublishing') || $request->request->has('unpublish_later')) {
993
                    $this->nodePublisher->chooseHowToUnpublish($request, $nodeTranslation, $this->translator);
994
                } else {
995
                    $this->addFlash(
996
                        FlashTypes::SUCCESS,
997
                        $this->get('translator')->trans('kuma_node.admin.edit.flash.success')
998
                    );
999
                }
1000
1001
                $params = [
1002
                    'id' => $node->getId(),
1003
                    'subaction' => $subaction,
1004
                    'currenttab' => $tabPane->getActiveTab(),
1005
                ];
1006
                $params = array_merge(
1007
                    $params,
1008
                    $tabPane->getExtraParams($request)
1009
                );
1010
1011
                if ($subaction === 'draft' && ($request->request->has('publishing') || $request->request->has('publish_later'))) {
1012
                    unset($params['subaction']);
1013
                }
1014
1015
                return $this->redirect(
1016
                    $this->generateUrl(
1017
                        'KunstmaanNodeBundle_nodes_edit',
1018
                        $params
1019
                    )
1020
                );
1021
            }
1022
        }
1023
1024
        $nodeVersions = $this->em->getRepository(NodeVersion::class)->findBy(
1025
            ['nodeTranslation' => $nodeTranslation],
1026
            ['updated' => 'DESC']
1027
        );
1028
        $queuedNodeTranslationAction = $this->em->getRepository(QueuedNodeTranslationAction::class)->findOneBy(['nodeTranslation' => $nodeTranslation]);
1029
1030
        return [
1031
            'page' => $page,
1032
            'entityname' => ClassLookup::getClass($page),
1033
            'nodeVersions' => $nodeVersions,
1034
            'node' => $node,
1035
            'nodeTranslation' => $nodeTranslation,
1036
            'draft' => $draft,
1037
            'draftNodeVersion' => $draftNodeVersion,
1038
            'showDuplicateWithChildren' => $this->getParameter('kunstmaan_node.show_duplicate_with_children'),
1039
            'nodeVersion' => $nodeVersion,
1040
            'subaction' => $subaction,
1041
            'tabPane' => $tabPane,
1042
            'editmode' => true,
1043
            'childCount' => $this->em->getRepository('KunstmaanNodeBundle:Node')->getChildCount($node),
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getChildCount() 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...
1044
            'queuedNodeTranslationAction' => $queuedNodeTranslationAction,
1045
            'nodeVersionLockCheck' => $this->container->getParameter('kunstmaan_node.lock_enabled'),
1046
            'nodeVersionLockInterval' => $this->container->getParameter('kunstmaan_node.lock_check_interval'),
1047
        ];
1048
    }
1049
1050
    /**
1051
     * @Route(
1052
     *      "checkNodeVersionLock/{id}/{public}",
1053
     *      requirements={"id" = "\d+", "public" = "(0|1)"},
1054
     *      name="KunstmaanNodeBundle_nodes_versionlock_check"
1055
     * )
1056
     *
1057
     * @param Request $request
1058
     * @param $id
1059
     *
1060
     * @return JsonResponse
1061
     */
1062
    public function checkNodeVersionLockAction(Request $request, $id, $public)
1063
    {
1064
        $nodeVersionIsLocked = false;
1065
        $message = '';
1066
        $this->init($request);
1067
1068
        /* @var Node $node */
1069
        $node = $this->em->getRepository(Node::class)->find($id);
1070
1071
        try {
1072
            $this->checkPermission($node, PermissionMap::PERMISSION_EDIT);
1073
1074
            /** @var NodeVersionLockHelper $nodeVersionLockHelper */
1075
            $nodeVersionLockHelper = $this->get('kunstmaan_node.admin_node.node_version_lock_helper');
1076
            $nodeTranslation = $node->getNodeTranslation($this->locale, true);
1077
1078
            if ($nodeTranslation) {
1079
                $nodeVersionIsLocked = $nodeVersionLockHelper->isNodeVersionLocked($this->getUser(), $nodeTranslation, $public);
0 ignored issues
show
Documentation introduced by
$this->getUser() is of type null|object, but the function expects a object<Kunstmaan\AdminBundle\Entity\BaseUser>.

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...
1080
1081
                if ($nodeVersionIsLocked) {
1082
                    $users = $nodeVersionLockHelper->getUsersWithNodeVersionLock($nodeTranslation, $public, $this->getUser());
1083
                    $message = $this->get('translator')->trans('kuma_node.admin.edit.flash.locked', array('%users%' => implode(', ', $users)));
1084
                }
1085
            }
1086
        } catch (AccessDeniedException $ade) {
0 ignored issues
show
Coding Style Comprehensibility introduced by
Consider adding a comment why this CATCH block is empty.
Loading history...
1087
        }
1088
1089
        return new JsonResponse(['lock' => $nodeVersionIsLocked, 'message' => $message]);
1090
    }
1091
1092
    /**
1093
     * @param NodeTranslation $nodeTranslation
1094
     * @param bool            $isPublic
1095
     *
1096
     * @return bool
1097
     */
1098
    private function isNodeVersionLocked(NodeTranslation $nodeTranslation, $isPublic)
1099
    {
1100
        if ($this->container->getParameter('kunstmaan_node.lock_enabled')) {
1101
            /** @var NodeVersionLockHelper $nodeVersionLockHelper */
1102
            $nodeVersionLockHelper = $this->get('kunstmaan_node.admin_node.node_version_lock_helper');
1103
            $nodeVersionIsLocked = $nodeVersionLockHelper->isNodeVersionLocked($this->getUser(), $nodeTranslation, $isPublic);
0 ignored issues
show
Documentation introduced by
$this->getUser() is of type null|object, but the function expects a object<Kunstmaan\AdminBundle\Entity\BaseUser>.

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...
1104
1105
            return $nodeVersionIsLocked;
1106
        }
1107
1108
        return false;
1109
    }
1110
1111
    /**
1112
     * @param HasNodeInterface $page            The page
1113
     * @param NodeTranslation  $nodeTranslation The node translation
1114
     * @param NodeVersion      $nodeVersion     The node version
1115
     *
1116
     * @return NodeVersion
1117
     */
1118
    private function createDraftVersion(
1119
        HasNodeInterface $page,
1120
        NodeTranslation $nodeTranslation,
1121
        NodeVersion $nodeVersion
1122
    ) {
1123
        $publicPage = $this->get('kunstmaan_admin.clone.helper')
1124
            ->deepCloneAndSave($page);
1125
        /* @var NodeVersion $publicNodeVersion */
1126
1127
        $publicNodeVersion = $this->em->getRepository(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeVersionFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...y\NodeVersionRepository.

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...
1128
            NodeVersion::class
1129
        )->createNodeVersionFor(
1130
            $publicPage,
1131
            $nodeTranslation,
1132
            $this->user,
1133
            $nodeVersion->getOrigin(),
1134
            'public',
1135
            $nodeVersion->getCreated()
1136
        );
1137
1138
        $nodeTranslation->setPublicNodeVersion($publicNodeVersion);
1139
        $nodeVersion->setType('draft');
1140
        $nodeVersion->setOrigin($publicNodeVersion);
1141
        $nodeVersion->setCreated(new DateTime());
1142
1143
        $this->em->persist($nodeTranslation);
1144
        $this->em->persist($nodeVersion);
1145
        $this->em->flush();
1146
1147
        $this->dispatch(
1148
            new NodeEvent(
1149
                $nodeTranslation->getNode(),
1150
                $nodeTranslation,
1151
                $nodeVersion,
1152
                $page
1153
            ),
1154
            Events::CREATE_DRAFT_VERSION
1155
        );
1156
1157
        return $nodeVersion;
1158
    }
1159
1160
    /**
1161
     * @param Node   $node       The node
1162
     * @param string $permission The permission to check for
1163
     *
1164
     * @throws AccessDeniedException
1165
     */
1166
    private function checkPermission(Node $node, $permission)
1167
    {
1168
        if (false === $this->authorizationChecker->isGranted($permission, $node)) {
1169
            throw new AccessDeniedException();
1170
        }
1171
    }
1172
1173
    /**
1174
     * @param EntityManager   $em       The Entity Manager
1175
     * @param BaseUser        $user     The user who deletes the children
1176
     * @param string          $locale   The locale that was used
1177
     * @param ArrayCollection $children The children array
1178
     */
1179 View Code Duplication
    private function deleteNodeChildren(
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
1180
        EntityManager $em,
1181
        BaseUser $user,
1182
        $locale,
1183
        ArrayCollection $children
1184
    ) {
1185
        /* @var Node $childNode */
1186
        foreach ($children as $childNode) {
1187
            $childNodeTranslation = $childNode->getNodeTranslation(
1188
                $this->locale,
1189
                true
1190
            );
1191
1192
            $childNodeVersion = $childNodeTranslation->getPublicNodeVersion();
1193
            $childNodePage = $childNodeVersion->getRef($this->em);
1194
1195
            $this->dispatch(
1196
                new NodeEvent(
1197
                    $childNode,
1198
                    $childNodeTranslation,
1199
                    $childNodeVersion,
1200
                    $childNodePage
1201
                ),
1202
                Events::PRE_DELETE
1203
            );
1204
1205
            $childNode->setDeleted(true);
1206
            $this->em->persist($childNode);
1207
1208
            $children2 = $childNode->getChildren();
1209
            $this->deleteNodeChildren($em, $user, $locale, $children2);
1210
1211
            $this->dispatch(
1212
                new NodeEvent(
1213
                    $childNode,
1214
                    $childNodeTranslation,
1215
                    $childNodeVersion,
1216
                    $childNodePage
1217
                ),
1218
                Events::POST_DELETE
1219
            );
1220
        }
1221
    }
1222
1223
    /**
1224
     * @param Request $request
1225
     * @param string  $type
1226
     *
1227
     * @return HasNodeInterface
1228
     */
1229
    private function createNewPage(Request $request, $type)
1230
    {
1231
        /* @var HasNodeInterface $newPage */
1232
        $newPage = new $type();
1233
1234
        $title = $request->get('title');
1235
        if (\is_string($title) && !empty($title)) {
1236
            $newPage->setTitle($title);
1237
        } else {
1238
            $newPage->setTitle($this->get('translator')->trans('kuma_node.admin.new_page.title.default'));
1239
        }
1240
        $this->em->persist($newPage);
1241
        $this->em->flush();
1242
1243
        return $newPage;
1244
    }
1245
1246
    /**
1247
     * @param Request $request
1248
     *
1249
     * @return string
1250
     * @throw InvalidArgumentException
1251
     */
1252
    private function validatePageType($request)
1253
    {
1254
        $type = $request->get('type');
1255
1256
        if (empty($type)) {
1257
            throw new InvalidArgumentException('Please specify a type of page you want to create');
1258
        }
1259
1260
        return $type;
1261
    }
1262
1263
    /**
1264
     * @param Node $node
1265
     *
1266
     * @return \Symfony\Component\HttpFoundation\Response
1267
     */
1268
    private function renderNodeNotTranslatedPage(Node $node)
1269
    {
1270
        //try to find a parent node with the correct translation, if there is none allow copy.
1271
        //if there is a parent but it doesn't have the language to copy to don't allow it
1272
        $parentNode = $node->getParent();
1273
        if ($parentNode) {
1274
            $parentNodeTranslation = $parentNode->getNodeTranslation(
1275
                $this->locale,
1276
                true
1277
            );
1278
            $parentsAreOk = false;
1279
1280
            if ($parentNodeTranslation) {
1281
                $parentsAreOk = $this->em->getRepository(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method hasParentNodeTranslationsForLanguage() 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...
1282
                    NodeTranslation::class
1283
                )->hasParentNodeTranslationsForLanguage(
1284
                    $node->getParent()->getNodeTranslation(
1285
                        $this->locale,
1286
                        true
1287
                    ),
1288
                    $this->locale
1289
                );
1290
            }
1291
        } else {
1292
            $parentsAreOk = true;
1293
        }
1294
1295
        return $this->render(
1296
            '@KunstmaanNode/NodeAdmin/pagenottranslated.html.twig',
1297
            array(
1298
                'node' => $node,
1299
                'nodeTranslations' => $node->getNodeTranslations(
1300
                    true
1301
                ),
1302
                'copyfromotherlanguages' => $parentsAreOk,
1303
            )
1304
        );
1305
    }
1306
1307
    /**
1308
     * @param object $event
1309
     * @param string $eventName
1310
     *
1311
     * @return object
1312
     */
1313 View Code Duplication
    private function dispatch($event, string $eventName)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
1314
    {
1315
        $eventDispatcher = $this->container->get('event_dispatcher');
1316
        if (class_exists(LegacyEventDispatcherProxy::class)) {
1317
            $eventDispatcher = LegacyEventDispatcherProxy::decorate($eventDispatcher);
1318
1319
            return $eventDispatcher->dispatch($event, $eventName);
1320
        }
1321
1322
        return $eventDispatcher->dispatch($eventName, $event);
1323
    }
1324
}
1325