NodeAdminController::editAction()   F
last analyzed

Complexity

Conditions 24
Paths 205

Size

Total Lines 206

Duplication

Lines 18
Ratio 8.74 %

Code Coverage

Tests 0
CRAP Score 600

Importance

Changes 0
Metric Value
dl 18
loc 206
ccs 0
cts 178
cp 0
rs 2.6166
c 0
b 0
f 0
cc 24
nc 205
nop 3
crap 600

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

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\HttpFoundation\JsonResponse;
41
use Symfony\Component\HttpFoundation\RedirectResponse;
42
use Symfony\Component\HttpFoundation\Request;
43
use Symfony\Component\Routing\Annotation\Route;
44
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
45
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
46
use Symfony\Component\Translation\TranslatorInterface;
47
48
/**
49
 * NodeAdminController
50
 */
51
class NodeAdminController extends Controller
0 ignored issues
show
Deprecated Code introduced by Daan Poron
The class Symfony\Bundle\Framework...e\Controller\Controller has been deprecated with message: since Symfony 4.2, use "Symfony\Bundle\FrameworkBundle\Controller\AbstractController" instead.

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

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

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

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

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

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
138
                return array(
139
                    'path' => '_slug_preview',
140
                    'params' => ['_locale' => $locale, 'url' => $item->getUrl()],
0 ignored issues
show
Bug introduced by whitewhidow
It seems like you code against a concrete implementation and not the interface Kunstmaan\AdminBundle\Entity\EntityInterface as the method getUrl() does only exist in the following implementations of said interface: Kunstmaan\AdminBundle\Entity\Exception, Kunstmaan\MediaBundle\Entity\Media, Kunstmaan\MenuBundle\Entity\BaseMenuItem, Kunstmaan\MenuBundle\Entity\MenuItem, Kunstmaan\NodeBundle\Entity\NodeTranslation, Kunstmaan\PagePartBundle\Entity\LinkPagePart.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

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

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
189
            ->createNodeTranslationFor($myLanguagePage, $this->locale, $node, $this->user);
190
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
191
192
        $this->get('event_dispatcher')->dispatch(
193
            Events::COPY_PAGE_TRANSLATION,
194
            new CopyPageTranslationNodeEvent(
195
                $node,
196
                $nodeTranslation,
197
                $nodeVersion,
198
                $myLanguagePage,
199
                $otherLanguageNodeTranslation,
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $otherLanguageNodeTranslation defined by $node->getNodeTranslatio...originalLanguage, true) on line 181 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
        );
205
206
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
207
    }
208
209
    /**
210
     * @Route(
211
     *      "/{id}/recopyfromotherlanguage",
212
     *      requirements={"id" = "\d+"},
213
     *      name="KunstmaanNodeBundle_nodes_recopyfromotherlanguage",
214
     *      methods={"POST"}
215
     * )
216
     *
217
     * @param Request $request
218
     * @param int     $id      The node id
219
     *
220
     * @return RedirectResponse
221
     *
222
     * @throws AccessDeniedException
223
     */
224
    public function recopyFromOtherLanguageAction(Request $request, $id)
225
    {
226
        $this->init($request);
227
        /* @var Node $node */
228
        $node = $this->em->getRepository(Node::class)->find($id);
229
230
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
231
232
        $otherLanguageNodeTranslation = $this->em->getRepository(NodeTranslation::class)->find($request->get('source'));
233
        $otherLanguageNodeNodeVersion = $otherLanguageNodeTranslation->getPublicNodeVersion();
234
        $otherLanguagePage = $otherLanguageNodeNodeVersion->getRef($this->em);
235
        $myLanguagePage = $this->get('kunstmaan_admin.clone.helper')
236
            ->deepCloneAndSave($otherLanguagePage);
237
238
        /* @var NodeTranslation $nodeTranslation */
239
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method addDraftNodeVersionFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
240
            ->addDraftNodeVersionFor($myLanguagePage, $this->locale, $node, $this->user);
241
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
242
243
        $this->get('event_dispatcher')->dispatch(
244
            Events::RECOPY_PAGE_TRANSLATION,
245
            new RecopyPageTranslationNodeEvent(
246
                $node,
247
                $nodeTranslation,
248
                $nodeVersion,
249
                $myLanguagePage,
250
                $otherLanguageNodeTranslation,
0 ignored issues
show
Documentation introduced by Sander Goossens
$otherLanguageNodeTranslation is of type object|null, but the function expects a object<Kunstmaan\NodeBun...Entity\NodeTranslation>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

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

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
292
            ->createNodeTranslationFor($myLanguagePage, $this->locale, $node, $this->user);
293
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
294
295
        $this->get('event_dispatcher')->dispatch(
296
            Events::ADD_EMPTY_PAGE_TRANSLATION,
297
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $myLanguagePage)
298
        );
299
300
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
301
    }
302
303
    /**
304
     * @Route("/{id}/publish", requirements={"id" =
305
     *                         "\d+"},
306
     *                         name="KunstmaanNodeBundle_nodes_publish", methods={"GET", "POST"})
307
     *
308
     * @param Request $request
309
     * @param int     $id
310
     *
311
     * @return RedirectResponse
312
     *
313
     * @throws AccessDeniedException
314
     */
315 View Code Duplication
    public function publishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
This method seems to be duplicated in your project.

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

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

Loading history...
316
    {
317
        $this->init($request);
318
        /* @var Node $node */
319
        $node = $this->em->getRepository(Node::class)->find($id);
320
321
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
322
        $request = $this->get('request_stack')->getCurrentRequest();
323
        $this->nodePublisher->chooseHowToPublish($request, $nodeTranslation, $this->translator);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 321 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...
324
325
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $node->getId())));
326
    }
327
328
    /**
329
     * @Route(
330
     *      "/{id}/unpublish",
331
     *      requirements={"id" = "\d+"},
332
     *      name="KunstmaanNodeBundle_nodes_unpublish",
333
     *      methods={"GET", "POST"}
334
     * )
335
     *
336
     * @param Request $request
337
     * @param int     $id
338
     *
339
     * @return RedirectResponse
340
     *
341
     * @throws AccessDeniedException
342
     */
343 View Code Duplication
    public function unPublishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
This method seems to be duplicated in your project.

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

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

Loading history...
344
    {
345
        $this->init($request);
346
        /* @var Node $node */
347
        $node = $this->em->getRepository(Node::class)->find($id);
348
349
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
350
        $request = $this->get('request_stack')->getCurrentRequest();
351
        $this->nodePublisher->chooseHowToUnpublish($request, $nodeTranslation, $this->translator);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 349 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...
352
353
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $node->getId())));
354
    }
355
356
    /**
357
     * @Route(
358
     *      "/{id}/unschedulepublish",
359
     *      requirements={"id" = "\d+"},
360
     *      name="KunstmaanNodeBundle_nodes_unschedule_publish",
361
     *      methods={"GET", "POST"}
362
     * )
363
     *
364
     * @param Request $request
365
     * @param int     $id
366
     *
367
     * @return RedirectResponse
368
     *
369
     * @throws AccessDeniedException
370
     */
371 View Code Duplication
    public function unSchedulePublishAction(Request $request, $id)
0 ignored issues
show
Duplication introduced by Piotr Belina
This method seems to be duplicated in your project.

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

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

Loading history...
372
    {
373
        $this->init($request);
374
375
        /* @var Node $node */
376
        $node = $this->em->getRepository(Node::class)->find($id);
377
378
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
379
        $this->nodePublisher->unSchedulePublish($nodeTranslation);
0 ignored issues
show
Bug introduced by Peter Zentjens
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 378 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...
380
381
        $this->addFlash(
382
            FlashTypes::SUCCESS,
383
            $this->get('translator')->trans('kuma_node.admin.unschedule.flash.success')
384
        );
385
386
        return $this->redirect($this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $id)));
387
    }
388
389
    /**
390
     * @Route(
391
     *      "/{id}/delete",
392
     *      requirements={"id" = "\d+"},
393
     *      name="KunstmaanNodeBundle_nodes_delete",
394
     *      methods={"POST"}
395
     * )
396
     *
397
     * @param Request $request
398
     * @param int     $id
399
     *
400
     * @return RedirectResponse
401
     *
402
     * @throws AccessDeniedException
403
     */
404
    public function deleteAction(Request $request, $id)
405
    {
406
        $this->init($request);
407
        /* @var Node $node */
408
        $node = $this->em->getRepository(Node::class)->find($id);
409
410
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_DELETE, $node);
411
412
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
413
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
414
        $page = $nodeVersion->getRef($this->em);
415
416
        $this->get('event_dispatcher')->dispatch(
417
            Events::PRE_DELETE,
418
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 412 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
        );
420
421
        $node->setDeleted(true);
422
        $this->em->persist($node);
423
424
        $children = $node->getChildren();
425
        $this->deleteNodeChildren($this->em, $this->user, $this->locale, $children);
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $children defined by $node->getChildren() on line 424 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...
426
        $this->em->flush();
427
428
        $event = new NodeEvent($node, $nodeTranslation, $nodeVersion, $page);
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $node->getNodeTranslation($this->locale, true) on line 412 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...
429
        $this->get('event_dispatcher')->dispatch(Events::POST_DELETE, $event);
430
        if (null === $response = $event->getResponse()) {
431
            $nodeParent = $node->getParent();
432
            // Check if we have a parent. Otherwise redirect to pages overview.
433
            if ($nodeParent) {
434
                $url = $this->get('router')->generate(
435
                    'KunstmaanNodeBundle_nodes_edit',
436
                    array('id' => $nodeParent->getId())
437
                );
438
            } else {
439
                $url = $this->get('router')->generate(
440
                    'KunstmaanNodeBundle_nodes'
441
                );
442
            }
443
            $response = new RedirectResponse($url);
444
        }
445
446
        $this->addFlash(
447
            FlashTypes::SUCCESS,
448
            $this->get('translator')->trans('kuma_node.admin.delete.flash.success')
449
        );
450
451
        return $response;
452
    }
453
454
    /**
455
     * @Route(
456
     *      "/{id}/duplicate",
457
     *      requirements={"id" = "\d+"},
458
     *      name="KunstmaanNodeBundle_nodes_duplicate",
459
     *      methods={"POST"}
460
     * )
461
     *
462
     * @param Request $request
463
     * @param int     $id
464
     *
465
     * @return RedirectResponse
466
     *
467
     * @throws AccessDeniedException
468
     */
469
    public function duplicateAction(Request $request, $id)
470
    {
471
        $this->init($request);
472
        /* @var Node $parentNode */
473
        $originalNode = $this->em->getRepository(Node::class)
474
            ->find($id);
475
476
        // Check with Acl
477
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $originalNode);
478
479
        $request = $this->get('request_stack')->getCurrentRequest();
480
481
        $originalNodeTranslations = $originalNode->getNodeTranslation($this->locale, true);
482
        $originalRef = $originalNodeTranslations->getPublicNodeVersion()->getRef($this->em);
483
        $newPage = $this->get('kunstmaan_admin.clone.helper')
484
            ->deepCloneAndSave($originalRef);
485
486
        //set the title
487
        $title = $request->get('title');
488
        if (\is_string($title) && !empty($title)) {
489
            $newPage->setTitle($title);
490
        } else {
491
            $newPage->setTitle('New page');
492
        }
493
494
        //set the parent
495
        $parentNodeTranslation = $originalNode->getParent()->getNodeTranslation($this->locale, true);
496
        $parent = $parentNodeTranslation->getPublicNodeVersion()->getRef($this->em);
497
        $newPage->setParent($parent);
498
        $this->em->persist($newPage);
499
        $this->em->flush();
500
501
        /* @var Node $nodeNewPage */
502
        $nodeNewPage = $this->em->getRepository(Node::class)->createNodeFor(
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
503
            $newPage,
504
            $this->locale,
505
            $this->user
506
        );
507
508
        $nodeTranslation = $nodeNewPage->getNodeTranslation($this->locale, true);
509
        if ($newPage->isStructureNode()) {
510
            $nodeTranslation->setSlug('');
511
            $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by woutervandamme
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 508 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...
512
        }
513
        $this->em->flush();
514
515
        $this->aclManager->updateNodeAcl($originalNode, $nodeNewPage);
0 ignored issues
show
Documentation introduced by Peter Zentjens
$originalNode is of type object|null, but the function expects a object<Kunstmaan\NodeBundle\Entity\Node>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
516
517
        $this->addFlash(
518
            FlashTypes::SUCCESS,
519
            $this->get('translator')->trans('kuma_node.admin.duplicate.flash.success')
520
        );
521
522
        return $this->redirect(
523
            $this->generateUrl('KunstmaanNodeBundle_nodes_edit', array('id' => $nodeNewPage->getId()))
524
        );
525
    }
526
527
    /**
528
     * @Route(
529
     *      "/{id}/duplicate-with-children",
530
     *      requirements={"id" = "\d+"},
531
     *      name="KunstmaanNodeBundle_nodes_duplicate_with_children",
532
     *      methods={"POST"}
533
     * )
534
     *
535
     * @throws AccessDeniedException
536
     */
537
    public function duplicateWithChildrenAction(Request $request, $id)
538
    {
539
        if (!$this->getParameter('kunstmaan_node.show_duplicate_with_children')) {
540
            return $this->redirectToRoute('KunstmaanNodeBundle_nodes_edit', ['id' => $id]);
541
        }
542
543
        $this->init($request);
544
        $title = $request->get('title', null);
545
546
        $nodeNewPage = $this->pageCloningHelper->duplicateWithChildren($id, $this->locale, $this->user, $title);
547
548
        $this->addFlash(FlashTypes::SUCCESS, $this->get('translator')->trans('kuma_node.admin.duplicate.flash.success'));
549
550
        return $this->redirectToRoute('KunstmaanNodeBundle_nodes_edit', ['id' => $nodeNewPage->getId()]);
551
    }
552
553
    /**
554
     * @Route(
555
     *      "/{id}/revert",
556
     *      requirements={"id" = "\d+"},
557
     *      defaults={"subaction" = "public"},
558
     *      name="KunstmaanNodeBundle_nodes_revert",
559
     *      methods={"GET"}
560
     * )
561
     *
562
     * @param Request $request
563
     * @param int     $id      The node id
564
     *
565
     * @return RedirectResponse
566
     *
567
     * @throws AccessDeniedException
568
     * @throws InvalidArgumentException
569
     */
570
    public function revertAction(Request $request, $id)
571
    {
572
        $this->init($request);
573
        /* @var Node $node */
574
        $node = $this->em->getRepository(Node::class)->find($id);
575
576
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
577
578
        $version = $request->get('version');
579
580
        if (empty($version) || !is_numeric($version)) {
581
            throw new InvalidArgumentException('No version was specified');
582
        }
583
584
        /* @var NodeVersionRepository $nodeVersionRepo */
585
        $nodeVersionRepo = $this->em->getRepository(NodeVersion::class);
586
        /* @var NodeVersion $nodeVersion */
587
        $nodeVersion = $nodeVersionRepo->find($version);
588
589
        if (\is_null($nodeVersion)) {
590
            throw new InvalidArgumentException('Version does not exist');
591
        }
592
593
        /* @var NodeTranslation $nodeTranslation */
594
        $nodeTranslation = $node->getNodeTranslation($this->locale, true);
595
        $page = $nodeVersion->getRef($this->em);
596
        /* @var HasNodeInterface $clonedPage */
597
        $clonedPage = $this->get('kunstmaan_admin.clone.helper')
598
            ->deepCloneAndSave($page);
599
        $newNodeVersion = $nodeVersionRepo->createNodeVersionFor(
600
            $clonedPage,
601
            $nodeTranslation,
602
            $this->user,
603
            $nodeVersion,
604
            'draft'
605
        );
606
607
        $nodeTranslation->setTitle($clonedPage->getTitle());
608
        $this->em->persist($nodeTranslation);
609
        $this->em->flush();
610
611
        $this->get('event_dispatcher')->dispatch(
612
            Events::REVERT,
613
            new RevertNodeAction(
614
                $node,
615
                $nodeTranslation,
616
                $newNodeVersion,
617
                $clonedPage,
618
                $nodeVersion,
619
                $page
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $page defined by $nodeVersion->getRef($this->em) on line 595 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
        );
622
623
        $this->addFlash(
624
            FlashTypes::SUCCESS,
625
            $this->get('translator')->trans('kuma_node.admin.revert.flash.success')
626
        );
627
628
        return $this->redirect(
629
            $this->generateUrl(
630
                'KunstmaanNodeBundle_nodes_edit',
631
                array(
632
                    'id' => $id,
633
                    'subaction' => 'draft',
634
                )
635
            )
636
        );
637
    }
638
639
    /**
640
     * @Route(
641
     *      "/{id}/add",
642
     *      requirements={"id" = "\d+"},
643
     *      name="KunstmaanNodeBundle_nodes_add",
644
     *      methods={"POST"}
645
     * )
646
     *
647
     * @param Request $request
648
     * @param int     $id
649
     *
650
     * @return RedirectResponse
651
     *
652
     * @throws AccessDeniedException
653
     * @throws InvalidArgumentException
654
     */
655
    public function addAction(Request $request, $id)
656
    {
657
        $this->init($request);
658
        /* @var Node $parentNode */
659
        $parentNode = $this->em->getRepository(Node::class)->find($id);
660
661
        // Check with Acl
662
        $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $parentNode);
663
664
        $parentNodeTranslation = $parentNode->getNodeTranslation($this->locale, true);
665
        $parentNodeVersion = $parentNodeTranslation->getPublicNodeVersion();
666
        $parentPage = $parentNodeVersion->getRef($this->em);
667
668
        $type = $this->validatePageType($request);
669
        $newPage = $this->createNewPage($request, $type);
670
        $newPage->setParent($parentPage);
671
672
        /* @var Node $nodeNewPage */
673
        $nodeNewPage = $this->em->getRepository(Node::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
674
            ->createNodeFor($newPage, $this->locale, $this->user);
675
        $nodeTranslation = $nodeNewPage->getNodeTranslation(
676
            $this->locale,
677
            true
678
        );
679
        $weight = $this->em->getRepository(NodeTranslation::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getMaxChildrenWeight() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Rep...deTranslationRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
680
                ->getMaxChildrenWeight($parentNode, $this->locale) + 1;
681
        $nodeTranslation->setWeight($weight);
682
683
        if ($newPage->isStructureNode()) {
684
            $nodeTranslation->setSlug('');
685
        }
686
687
        $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by mdx
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 675 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...
688
        $this->em->flush();
689
690
        $this->aclManager->updateNodeAcl($parentNode, $nodeNewPage);
691
692
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
693
694
        $this->get('event_dispatcher')->dispatch(
695
            Events::ADD_NODE,
696
            new NodeEvent(
697
                $nodeNewPage, $nodeTranslation, $nodeVersion, $newPage
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 675 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
        );
700
701
        return $this->redirect(
702
            $this->generateUrl(
703
                'KunstmaanNodeBundle_nodes_edit',
704
                array('id' => $nodeNewPage->getId())
705
            )
706
        );
707
    }
708
709
    /**
710
     * @Route("/add-homepage", name="KunstmaanNodeBundle_nodes_add_homepage", methods={"POST"})
711
     *
712
     * @return RedirectResponse
713
     *
714
     * @throws AccessDeniedException
715
     * @throws InvalidArgumentException
716
     */
717
    public function addHomepageAction(Request $request)
718
    {
719
        $this->init($request);
720
721
        // Check with Acl
722
        $this->denyAccessUnlessGranted('ROLE_SUPER_ADMIN');
723
724
        $type = $this->validatePageType($request);
725
726
        $newPage = $this->createNewPage($request, $type);
727
728
        /* @var Node $nodeNewPage */
729
        $nodeNewPage = $this->em->getRepository(Node::class)
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method createNodeFor() does only exist in the following implementations of said interface: Kunstmaan\NodeBundle\Repository\NodeRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
730
            ->createNodeFor($newPage, $this->locale, $this->user);
731
        $nodeTranslation = $nodeNewPage->getNodeTranslation(
732
            $this->locale,
733
            true
734
        );
735
        $this->em->flush();
736
737
        // Set default permissions
738
        $this->container->get('kunstmaan_node.acl_permission_creator_service')
739
            ->createPermission($nodeNewPage);
740
741
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
742
743
        $this->get('event_dispatcher')->dispatch(
744
            Events::ADD_NODE,
745
            new NodeEvent(
746
                $nodeNewPage, $nodeTranslation, $nodeVersion, $newPage
0 ignored issues
show
Bug introduced by Wim Vandersmissen
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTra...on($this->locale, true) on line 731 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
        );
749
750
        return $this->redirect(
751
            $this->generateUrl(
752
                'KunstmaanNodeBundle_nodes_edit',
753
                array('id' => $nodeNewPage->getId())
754
            )
755
        );
756
    }
757
758
    /**
759
     * @Route("/reorder", name="KunstmaanNodeBundle_nodes_reorder", methods={"POST"})
760
     *
761
     * @param Request $request
762
     *
763
     * @return string
0 ignored issues
show
Documentation introduced by Matthieu Cornut
Should the return type not be JsonResponse?

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

Loading history...
764
     *
765
     * @throws AccessDeniedException
766
     */
767
    public function reorderAction(Request $request)
768
    {
769
        $this->init($request);
770
        $nodes = array();
771
        $nodeIds = $request->get('nodes');
772
        $changeParents = $request->get('parent');
773
774
        foreach ($nodeIds as $id) {
775
            /* @var Node $node */
776
            $node = $this->em->getRepository(Node::class)->find($id);
777
            $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $node);
778
            $nodes[] = $node;
779
        }
780
781
        $weight = 1;
782
        foreach ($nodes as $node) {
783
            $newParentId = isset($changeParents[$node->getId()]) ? $changeParents[$node->getId()] : null;
784
            if ($newParentId) {
785
                $parent = $this->em->getRepository(Node::class)->find($newParentId);
786
                $this->denyAccessUnlessGranted(PermissionMap::PERMISSION_EDIT, $parent);
787
                $node->setParent($parent);
788
                $this->em->persist($node);
789
                $this->em->flush($node);
790
            }
791
792
            /* @var NodeTranslation $nodeTranslation */
793
            $nodeTranslation = $node->getNodeTranslation($this->locale, true);
794
795
            if ($nodeTranslation) {
796
                $nodeVersion = $nodeTranslation->getPublicNodeVersion();
797
                $page = $nodeVersion->getRef($this->em);
798
799
                $this->get('event_dispatcher')->dispatch(
800
                    Events::PRE_PERSIST,
801
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
0 ignored issues
show
Bug introduced by Samuele Mazza
It seems like $page defined by $nodeVersion->getRef($this->em) on line 797 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
                );
803
804
                $nodeTranslation->setWeight($weight);
805
                $this->em->persist($nodeTranslation);
806
                $this->em->flush($nodeTranslation);
807
808
                $this->get('event_dispatcher')->dispatch(
809
                    Events::POST_PERSIST,
810
                    new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
0 ignored issues
show
Bug introduced by Samuele Mazza
It seems like $page defined by $nodeVersion->getRef($this->em) on line 797 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() {