Completed
Push — 5.6 ( 679697...f4d50c )
by Jeroen
16:35 queued 10:49
created

NodeHelper::getUser()   A

Complexity

Conditions 5
Paths 3

Size

Total Lines 12

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 6
CRAP Score 5.0729

Importance

Changes 0
Metric Value
dl 0
loc 12
ccs 6
cts 7
cp 0.8571
rs 9.5555
c 0
b 0
f 0
cc 5
nc 3
nop 0
crap 5.0729
1
<?php
2
3
namespace Kunstmaan\NodeBundle\Helper;
4
5
use Doctrine\ORM\EntityManagerInterface;
6
use Kunstmaan\AdminBundle\Entity\User;
7
use Kunstmaan\AdminBundle\Helper\CloneHelper;
8
use Kunstmaan\AdminBundle\Helper\FormWidgets\Tabs\TabPane;
9
use Kunstmaan\NodeBundle\Entity\HasNodeInterface;
10
use Kunstmaan\NodeBundle\Entity\Node;
11
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
12
use Kunstmaan\NodeBundle\Entity\NodeVersion;
13
use Kunstmaan\NodeBundle\Event\CopyPageTranslationNodeEvent;
14
use Kunstmaan\NodeBundle\Event\Events;
15
use Kunstmaan\NodeBundle\Event\NodeEvent;
16
use Kunstmaan\NodeBundle\Event\RecopyPageTranslationNodeEvent;
17
use Kunstmaan\NodeBundle\Helper\NodeAdmin\NodeAdminPublisher;
18
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
19
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
20
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
21
22
/**
23
 * Class NodeHelper
24
 */
25
class NodeHelper
26
{
27
    /** @var EntityManagerInterface */
28
    private $em;
29
30
    /** @var NodeAdminPublisher */
31
    private $nodeAdminPublisher;
32
33
    /** @var TokenStorageInterface */
34
    private $tokenStorage;
35
36
    /** @var CloneHelper */
37
    private $cloneHelper;
38
39
    /** @var EventDispatcherInterface */
40
    private $eventDispatcher;
41
42
    /**
43
     * NodeHelper constructor.
44
     */
45 12 View Code Duplication
    public function __construct(
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...
46
        EntityManagerInterface $em,
47
        NodeAdminPublisher $nodeAdminPublisher,
48
        TokenStorageInterface $tokenStorage,
49
        CloneHelper $cloneHelper,
50
        EventDispatcherInterface $eventDispatcher
51
    ) {
52 12
        $this->em = $em;
53 12
        $this->nodeAdminPublisher = $nodeAdminPublisher;
54 12
        $this->tokenStorage = $tokenStorage;
55 12
        $this->cloneHelper = $cloneHelper;
56 12
        $this->eventDispatcher = $eventDispatcher;
57 12
    }
58
59
    /**
60
     * @param HasNodeInterface $page            The page
61
     * @param NodeTranslation  $nodeTranslation The node translation
62
     * @param NodeVersion      $nodeVersion     The node version
63
     *
64
     * @return NodeVersion
65
     */
66 1
    public function createDraftVersion(
67
        HasNodeInterface $page,
68
        NodeTranslation $nodeTranslation,
69
        NodeVersion $nodeVersion
70
    ) {
71 1
        $user = $this->getAdminUser();
72 1
        $publicPage = $this->cloneHelper->deepCloneAndSave($page);
73
74
        /* @var NodeVersion $publicNodeVersion */
75 1
        $publicNodeVersion = $this->em->getRepository(NodeVersion::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 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...
76 1
            ->createNodeVersionFor(
77 1
                $publicPage,
78
                $nodeTranslation,
79
                $user,
80 1
                $nodeVersion->getOrigin(),
81 1
                NodeVersion::PUBLIC_VERSION,
82 1
                $nodeVersion->getCreated()
83
            );
84
85 1
        $nodeTranslation->setPublicNodeVersion($publicNodeVersion);
86 1
        $nodeVersion->setType(NodeVersion::DRAFT_VERSION);
87 1
        $nodeVersion->setOrigin($publicNodeVersion);
88 1
        $nodeVersion->setCreated(new \DateTime());
89
90 1
        $this->em->persist($nodeTranslation);
91 1
        $this->em->persist($nodeVersion);
92 1
        $this->em->flush();
93
94 1
        $this->eventDispatcher->dispatch(
95 1
            Events::CREATE_DRAFT_VERSION,
96 1
            new NodeEvent(
97 1
                $nodeTranslation->getNode(),
98
                $nodeTranslation,
99
                $nodeVersion,
100
                $page
101
            )
102
        );
103
104 1
        return $nodeVersion;
105
    }
106
107
    /**
108
     * @param int  $nodeVersionTimeout
109
     * @param bool $nodeVersionIsLocked
110
     */
111 2
    public function prepareNodeVersion(NodeVersion $nodeVersion, NodeTranslation $nodeTranslation, $nodeVersionTimeout, $nodeVersionIsLocked)
112
    {
113 2
        $user = $this->getAdminUser();
114 2
        $thresholdDate = date('Y-m-d H:i:s', time() - $nodeVersionTimeout);
115 2
        $updatedDate = date('Y-m-d H:i:s', strtotime($nodeVersion->getUpdated()->format('Y-m-d H:i:s')));
116
117 2 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...
118 2
            $page = $nodeVersion->getRef($this->em);
0 ignored issues
show
Compatibility introduced by
$this->em of type object<Doctrine\ORM\EntityManagerInterface> is not a sub-type of object<Doctrine\ORM\EntityManager>. It seems like you assume a concrete implementation of the interface Doctrine\ORM\EntityManagerInterface to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
119 2
            if ($nodeVersion === $nodeTranslation->getPublicNodeVersion()) {
120 1
                $this->nodeAdminPublisher
121 1
                    ->createPublicVersion(
122 1
                        $page,
0 ignored issues
show
Bug introduced by
It seems like $page defined by $nodeVersion->getRef($this->em) on line 118 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...
123
                        $nodeTranslation,
124
                        $nodeVersion,
125
                        $user
126
                    );
127
            } else {
128 1
                $this->createDraftVersion(
129 1
                    $page,
0 ignored issues
show
Bug introduced by
It seems like $page defined by $nodeVersion->getRef($this->em) on line 118 can be null; however, Kunstmaan\NodeBundle\Hel...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...
130
                    $nodeTranslation,
131
                    $nodeVersion
132
                );
133
            }
134
        }
135 2
    }
136
137
    /**
138
     * @param bool    $isStructureNode
139
     * @param TabPane $tabPane
0 ignored issues
show
Documentation introduced by
Should the type for parameter $tabPane not be null|TabPane?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
140
     *
141
     * @return NodeTranslation
142
     */
143 1
    public function updatePage(
144
        Node $node,
145
        NodeTranslation $nodeTranslation,
146
        NodeVersion $nodeVersion,
147
        HasNodeInterface $page,
148
        $isStructureNode,
149
        TabPane $tabPane = null
150
    ) {
151 1
        $this->eventDispatcher->dispatch(
152 1
            Events::PRE_PERSIST,
153 1
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
154
        );
155
156 1
        $nodeTranslation->setTitle($page->getTitle());
157 1
        if ($isStructureNode) {
158
            $nodeTranslation->setSlug('');
159
        }
160
161 1
        $nodeVersion->setUpdated(new \DateTime());
162 1
        if ($nodeVersion->getType() == NodeVersion::PUBLIC_VERSION) {
163 1
            $nodeTranslation->setUpdated($nodeVersion->getUpdated());
164
        }
165 1
        $this->em->persist($nodeTranslation);
166 1
        $this->em->persist($nodeVersion);
167 1
        $this->em->persist($node);
168 1
        if (null !== $tabPane) {
169
            $tabPane->persist($this->em);
0 ignored issues
show
Compatibility introduced by
$this->em of type object<Doctrine\ORM\EntityManagerInterface> is not a sub-type of object<Doctrine\ORM\EntityManager>. It seems like you assume a concrete implementation of the interface Doctrine\ORM\EntityManagerInterface to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
170
        }
171 1
        $this->em->flush();
172
173 1
        $this->eventDispatcher->dispatch(
174 1
            Events::POST_PERSIST,
175 1
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
176
        );
177
178 1
        return $nodeTranslation;
179
    }
180
181
    /**
182
     * @param string $refEntityType
183
     * @param string $pageTitle
184
     * @param string $locale
185
     *
186
     * @return NodeTranslation
0 ignored issues
show
Documentation introduced by
Should the return type not be NodeTranslation|null?

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...
187
     */
188 1
    public function createPage(
189
        $refEntityType,
190
        $pageTitle,
191
        $locale,
192
        Node $parentNode = null)
193
    {
194 1
        $user = $this->getAdminUser();
195
196 1
        $newPage = $this->createNewPage($refEntityType, $pageTitle);
197 1
        if (null !== $parentNode) {
198 1
            $parentNodeTranslation = $parentNode->getNodeTranslation($locale, true);
199 1
            $parentNodeVersion = $parentNodeTranslation->getPublicNodeVersion();
200 1
            $parentPage = $parentNodeVersion->getRef($this->em);
201 1
            $newPage->setParent($parentPage);
202
        }
203
204
        /* @var Node $nodeNewPage */
205 1
        $nodeNewPage = $this->em->getRepository(Node::class)->createNodeFor($newPage, $locale, $user);
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...
206 1
        $nodeTranslation = $nodeNewPage->getNodeTranslation($locale, true);
207 1
        if (null !== $parentNode) {
208 1
            $weight = $this->em->getRepository(NodeTranslation::class)->getMaxChildrenWeight(
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...
209 1
                    $parentNode,
210
                    $locale
211 1
                ) + 1;
212 1
            $nodeTranslation->setWeight($weight);
213
        }
214
215 1
        if ($newPage->isStructureNode()) {
216
            $nodeTranslation->setSlug('');
217
        }
218
219 1
        $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTranslation($locale, true) on line 206 can be null; however, Doctrine\Persistence\ObjectManager::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...
220 1
        $this->em->flush();
221
222 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
223
224 1
        $this->eventDispatcher->dispatch(
225 1
            Events::ADD_NODE,
226 1
            new NodeEvent(
227 1
                $nodeNewPage, $nodeTranslation, $nodeVersion, $newPage
0 ignored issues
show
Bug introduced by
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTranslation($locale, true) on line 206 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...
228
            )
229
        );
230
231 1
        return $nodeTranslation;
232
    }
233
234
    /**
235
     * @param string $locale
236
     *
237
     * @return NodeTranslation
0 ignored issues
show
Documentation introduced by
Should the return type not be NodeTranslation|null?

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...
238
     */
239 1
    public function deletePage(Node $node, $locale)
240
    {
241 1
        $nodeTranslation = $node->getNodeTranslation($locale, true);
242 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
243 1
        $page = $nodeVersion->getRef($this->em);
244
245 1
        $this->eventDispatcher->dispatch(
246 1
            Events::PRE_DELETE,
247 1
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
0 ignored issues
show
Bug introduced by
It seems like $nodeTranslation defined by $node->getNodeTranslation($locale, true) on line 241 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...
248
        );
249
250 1
        $node->setDeleted(true);
251 1
        $this->em->persist($node);
252
253 1
        $this->deleteNodeChildren($node, $locale);
254 1
        $this->em->flush();
255
256 1
        $this->eventDispatcher->dispatch(
257 1
            Events::POST_DELETE,
258 1
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $page)
0 ignored issues
show
Bug introduced by
It seems like $nodeTranslation defined by $node->getNodeTranslation($locale, true) on line 241 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...
259
        );
260
261 1
        return $nodeTranslation;
262
    }
263
264
    /**
265
     * @param string $locale
266
     *
267
     * @return HasNodeInterface
268
     */
269 1
    public function getPageWithNodeInterface(Node $node, $locale)
270
    {
271 1
        $nodeTranslation = $node->getNodeTranslation($locale, true);
272 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
273
274 1
        return $nodeVersion->getRef($this->em);
275
    }
276
277
    /**
278
     * @param string $sourceLocale
279
     * @param string $locale
280
     *
281
     * @return NodeTranslation
282
     */
283 1 View Code Duplication
    public function copyPageFromOtherLanguage(Node $node, $sourceLocale, $locale)
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...
284
    {
285 1
        $user = $this->getAdminUser();
286
287 1
        $sourceNodeTranslation = $node->getNodeTranslation($sourceLocale, true);
288 1
        $sourceNodeVersion = $sourceNodeTranslation->getPublicNodeVersion();
289 1
        $sourcePage = $sourceNodeVersion->getRef($this->em);
290 1
        $targetPage = $this->cloneHelper->deepCloneAndSave($sourcePage);
291
292
        /* @var NodeTranslation $nodeTranslation */
293 1
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)->createNodeTranslationFor($targetPage, $locale, $node, $user);
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...
294 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
295
296 1
        $this->eventDispatcher->dispatch(
297 1
            Events::COPY_PAGE_TRANSLATION,
298 1
            new CopyPageTranslationNodeEvent(
299 1
                $node,
300
                $nodeTranslation,
301
                $nodeVersion,
302
                $targetPage,
303
                $sourceNodeTranslation,
0 ignored issues
show
Bug introduced by
It seems like $sourceNodeTranslation defined by $node->getNodeTranslation($sourceLocale, true) on line 287 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...
304
                $sourceNodeVersion,
305
                $sourcePage,
306
                $sourceLocale
307
            )
308
        );
309
310 1
        return $nodeTranslation;
311
    }
312
313
    /**
314
     * @param string $locale
315
     * @param string $title
316
     *
317
     * @return NodeTranslation|null
318
     */
319 1
    public function duplicatePage(Node $node, $locale, $title = 'New page')
320
    {
321 1
        $user = $this->getAdminUser();
322
323 1
        $sourceNodeTranslations = $node->getNodeTranslation($locale, true);
324 1
        $sourcePage = $sourceNodeTranslations->getPublicNodeVersion()->getRef($this->em);
325 1
        $targetPage = $this->cloneHelper->deepCloneAndSave($sourcePage);
326 1
        $targetPage->setTitle($title);
327
328 1
        if ($node->getParent()) {
329 1
            $parentNodeTranslation = $node->getParent()->getNodeTranslation($locale, true);
330 1
            $parent = $parentNodeTranslation->getPublicNodeVersion()->getRef($this->em);
331 1
            $targetPage->setParent($parent);
332
        }
333 1
        $this->em->persist($targetPage);
334 1
        $this->em->flush();
335
336
        /* @var Node $nodeNewPage */
337 1
        $nodeNewPage = $this->em->getRepository(Node::class)->createNodeFor($targetPage, $locale, $user);
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...
338
339 1
        $nodeTranslation = $nodeNewPage->getNodeTranslation($locale, true);
340 1
        if ($targetPage->isStructureNode()) {
341
            $nodeTranslation->setSlug('');
342
            $this->em->persist($nodeTranslation);
0 ignored issues
show
Bug introduced by
It seems like $nodeTranslation defined by $nodeNewPage->getNodeTranslation($locale, true) on line 339 can be null; however, Doctrine\Persistence\ObjectManager::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...
343
        }
344 1
        $this->em->flush();
345
346 1
        return $nodeTranslation;
347
    }
348
349
    /**
350
     * @param int    $sourceNodeTranslationId
351
     * @param string $locale
352
     *
353
     * @return NodeTranslation
354
     */
355 1 View Code Duplication
    public function createPageDraftFromOtherLanguage(Node $node, $sourceNodeTranslationId, $locale)
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...
356
    {
357 1
        $user = $this->getAdminUser();
358
359 1
        $sourceNodeTranslation = $this->em->getRepository(NodeTranslation::class)->find($sourceNodeTranslationId);
360 1
        $sourceNodeVersion = $sourceNodeTranslation->getPublicNodeVersion();
361 1
        $sourcePage = $sourceNodeVersion->getRef($this->em);
362 1
        $targetPage = $this->cloneHelper->deepCloneAndSave($sourcePage);
363
364
        /* @var NodeTranslation $nodeTranslation */
365 1
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)->addDraftNodeVersionFor($targetPage, $locale, $node, $user);
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...
366 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
367
368 1
        $this->eventDispatcher->dispatch(
369 1
            Events::RECOPY_PAGE_TRANSLATION,
370 1
            new RecopyPageTranslationNodeEvent(
371 1
                $node,
372
                $nodeTranslation,
373
                $nodeVersion,
374
                $targetPage,
375
                $sourceNodeTranslation,
0 ignored issues
show
Documentation introduced by
$sourceNodeTranslation 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...
376
                $sourceNodeVersion,
377
                $sourcePage,
378 1
                $sourceNodeTranslation->getLang()
379
            )
380
        );
381
382 1
        return $nodeTranslation;
383
    }
384
385
    /**
386
     * @param string $locale
387
     *
388
     * @return NodeTranslation
389
     */
390 1
    public function createEmptyPage(Node $node, $locale)
391
    {
392 1
        $user = $this->getAdminUser();
393
394 1
        $refEntityName = $node->getRefEntityName();
395 1
        $targetPage = $this->createNewPage($refEntityName);
396
397
        /* @var NodeTranslation $nodeTranslation */
398 1
        $nodeTranslation = $this->em->getRepository(NodeTranslation::class)->createNodeTranslationFor($targetPage, $locale, $node, $user);
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...
399 1
        $nodeVersion = $nodeTranslation->getPublicNodeVersion();
400
401 1
        $this->eventDispatcher->dispatch(
402 1
            Events::ADD_EMPTY_PAGE_TRANSLATION,
403 1
            new NodeEvent($node, $nodeTranslation, $nodeVersion, $targetPage)
404
        );
405
406 1
        return $nodeTranslation;
407
    }
408
409
    /**
410
     * @param string $entityType
411
     * @param string $title
412
     *
413
     * @return HasNodeInterface
414
     */
415 2
    protected function createNewPage($entityType, $title = 'No title')
416
    {
417
        /* @var HasNodeInterface $newPage */
418 2
        $newPage = new $entityType();
419 2
        $newPage->setTitle($title);
420
421 2
        $this->em->persist($newPage);
422 2
        $this->em->flush();
423
424 2
        return $newPage;
425
    }
426
427
    /**
428
     * @param string $locale
429
     */
430 1
    protected function deleteNodeChildren(Node $node, $locale)
431
    {
432 1
        $children = $node->getChildren();
433
434
        /* @var Node $childNode */
435 1
        foreach ($children as $childNode) {
436 1
            $childNodeTranslation = $childNode->getNodeTranslation($locale, true);
437 1
            $childNodeVersion = $childNodeTranslation->getPublicNodeVersion();
438 1
            $childNodePage = $childNodeVersion->getRef($this->em);
439
440 1
            $this->eventDispatcher->dispatch(
441 1
                Events::PRE_DELETE,
442 1
                new NodeEvent(
443 1
                    $childNode,
444
                    $childNodeTranslation,
445
                    $childNodeVersion,
446
                    $childNodePage
447
                )
448
            );
449
450 1
            $childNode->setDeleted(true);
451 1
            $this->em->persist($childNode);
452
453 1
            $this->deleteNodeChildren($childNode, $locale);
454
455 1
            $this->eventDispatcher->dispatch(
456 1
                Events::POST_DELETE,
457 1
                new NodeEvent(
458 1
                    $childNode,
459
                    $childNodeTranslation,
460
                    $childNodeVersion,
461
                    $childNodePage
462
                )
463
            );
464
        }
465 1
    }
466
467
    /**
468
     * @return mixed|null
0 ignored issues
show
Documentation introduced by
Consider making the return type a bit more specific; maybe use User|null.

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...
469
     */
470 8
    protected function getUser()
471
    {
472 8
        $token = $this->tokenStorage->getToken();
473 8
        if ($token) {
474 8
            $user = $token->getUser();
475 8
            if ($user && $user !== 'anon.' && $user instanceof User) {
476 8
                return $user;
477
            }
478
        }
479
480
        return null;
481
    }
482
483
    /**
484
     * @return mixed
0 ignored issues
show
Documentation introduced by
Consider making the return type a bit more specific; maybe use User.

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...
485
     */
486 8
    protected function getAdminUser()
487
    {
488 8
        $user = $this->getUser();
489 8
        if (!$user) {
490
            throw new AccessDeniedException('Access denied: User should be an admin user');
491
        }
492
493 8
        return $user;
494
    }
495
}
496