Completed
Pull Request — master (#344)
by Leny
06:07
created

BlogController::feedAction()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 6
Code Lines 3

Duplication

Lines 0
Ratio 0 %

Importance

Changes 1
Bugs 0 Features 1
Metric Value
c 1
b 0
f 1
dl 0
loc 6
rs 9.4285
cc 1
eloc 3
nc 1
nop 2
1
<?php
2
3
namespace Victoire\Bundle\BlogBundle\Controller;
4
5
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
6
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
7
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
8
use Symfony\Component\HttpFoundation\JsonResponse;
9
use Symfony\Component\HttpFoundation\Request;
10
use Symfony\Component\HttpFoundation\Response;
11
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
12
use Victoire\Bundle\BlogBundle\Entity\Blog;
13
use Victoire\Bundle\BlogBundle\Form\ChooseBlogType;
14
use Victoire\Bundle\BlogBundle\Repository\BlogRepository;
15
use Victoire\Bundle\BusinessPageBundle\Entity\BusinessTemplate;
16
use Victoire\Bundle\PageBundle\Controller\BasePageController;
17
use Victoire\Bundle\PageBundle\Entity\BasePage;
18
use Victoire\Bundle\ViewReferenceBundle\ViewReference\ViewReference;
19
20
/**
21
 * blog Controller.
22
 *
23
 * @Route("/victoire-dcms/blog")
24
 */
25
class BlogController extends BasePageController
26
{
27
    protected $routes;
28
29
    /**
30
     * Constructor.
31
     */
32
    public function __construct()
33
    {
34
        $this->routes = [
35
            'new'       => 'victoire_blog_new',
36
            'show'      => 'victoire_core_page_show',
37
            'settings'  => 'victoire_blog_settings',
38
            'articles'  => 'victoire_blog_articles',
39
            'category'  => 'victoire_blog_category',
40
            'delete'    => 'victoire_blog_delete',
41
        ];
42
    }
43
44
    /**
45
     * New page.
46
     *
47
     * @Route("/index/{blogId}/{tab}", name="victoire_blog_index", defaults={"blogId" = null, "tab" = "articles"})
48
     *
49
     * @param Request $request
50
     *
51
     * @return JsonResponse
52
     */
53
    public function indexAction(Request $request, $blogId = null, $tab = 'articles')
54
    {
55
        /** @var BlogRepository $blogRepo */
56
        $blogRepo = $this->get('doctrine.orm.entity_manager')->getRepository('VictoireBlogBundle:Blog');
57
        $blogs = $blogRepo->getAll()->run();
58
        $blog = reset($blogs);
59
        if (is_numeric($blogId)) {
60
            $blog = $blogRepo->find($blogId);
61
        }
62
        $options['blog'] = $blog;
0 ignored issues
show
Coding Style Comprehensibility introduced by
$options was never initialized. Although not strictly required by PHP, it is generally a good practice to add $options = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
63
        $template = $this->getBaseTemplatePath().':index.html.twig';
64
        $chooseBlogForm = $this->createForm(new ChooseBlogType(), null, $options);
65
66
        $chooseBlogForm->handleRequest($request);
67
        if ($chooseBlogForm->isValid()) {
68
            $blog = $chooseBlogForm->getData()['blog'];
69
            $template = $this->getBaseTemplatePath().':_blogItem.html.twig';
70
            $chooseBlogForm = $this->createForm(new ChooseBlogType(), null, ['blog' => $blog]);
71
        }
72
        $businessProperties = [];
73
74
        if ($blog instanceof BusinessTemplate) {
75
            //we can use the business entity properties on the seo
76
            $businessEntity = $this->get('victoire_core.helper.business_entity_helper')->findById($blog->getBusinessEntityId());
77
            $businessProperties = $businessEntity->getBusinessPropertiesByType('seoable');
78
        }
79
80
        return new JsonResponse(
81
            [
82
                'html' => $this->container->get('victoire_templating')->render(
83
                    $template,
84
                    [
85
                        'blog'               => $blog,
86
                        'currentTab'         => $tab,
87
                        'tabs'               => ['articles', 'settings', 'category'],
88
                        'chooseBlogForm'     => $chooseBlogForm->createView(),
89
                        'businessProperties' => $businessProperties,
90
                    ]
91
                ),
92
            ]
93
        );
94
    }
95
96
    /**
97
     * New page.
98
     *
99
     * @Route("/feed/{slug}.rss", name="victoire_blog_rss", defaults={"_format" = "rss"})
100
     *
101
     * @param Request $request
102
     * @Template("VictoireBlogBundle:Blog:feed.rss.twig")
103
     * @return JsonResponse
104
     */
105
    public function feedAction(Request $request, Blog $blog)
0 ignored issues
show
Unused Code introduced by
The parameter $request is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
106
    {
107
        return [
108
                'blog' => $blog,
109
            ];
110
    }
111
112
    /**
113
     * New page.
114
     *
115
     * @Route("/new", name="victoire_blog_new")
116
     * @Template()
117
     *
118
     * @return JsonResponse
119
     */
120
    public function newAction($isHomepage = false)
121
    {
122
        return new JsonResponse(parent::newAction());
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Symfony\Comp...e(parent::newAction()); (Symfony\Component\HttpFoundation\JsonResponse) is incompatible with the return type of the parent method Victoire\Bundle\PageBund...geController::newAction of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
123
    }
124
125
    /**
126
     * Blog settings.
127
     *
128
     * @param Request  $request
129
     * @param BasePage $blog
130
     *
131
     * @return JsonResponse
132
     * @Route("/{id}/settings", name="victoire_blog_settings")
133
     * @ParamConverter("blog", class="VictoirePageBundle:BasePage")
134
     */
135
    public function settingsAction(Request $request, BasePage $blog)
136
    {
137
        $entityManager = $this->getDoctrine()->getManager();
138
        $form = $this->createForm($this->getPageSettingsType(), $blog);
139
        $businessProperties = [];
140
141
        $form->handleRequest($request);
142
143
        if ($form->isValid()) {
144
            $entityManager->persist($blog);
145
            $entityManager->flush();
146
147
            /** @var ViewReference $reference */
148
            $reference = $this->get('victoire_view_reference.repository')
149
            ->getOneReferenceByParameters(['viewId' => $blog->getId()]);
150
151
            return new JsonResponse([
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Symfony\Comp...eference->getUrl())))); (Symfony\Component\HttpFoundation\JsonResponse) is incompatible with the return type of the parent method Victoire\Bundle\PageBund...troller::settingsAction of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
152
                'success' => true,
153
                'url'     => $this->generateUrl(
154
                    'victoire_core_page_show', [
155
                        '_locale' => $blog->getLocale(), 'url' => $reference->getUrl(),
156
                ]),
157
            ]);
158
        }
159
        //we display the form
160
        $errors = $this->get('victoire_form.error_helper')->getRecursiveReadableErrors($form);
161
        if ($errors != '') {
162
            return new JsonResponse(['html' => $this->container->get('victoire_templating')->render(
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Symfony\Comp...'message' => $errors)); (Symfony\Component\HttpFoundation\JsonResponse) is incompatible with the return type of the parent method Victoire\Bundle\PageBund...troller::settingsAction of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
163
                        $this->getBaseTemplatePath().':Tabs/_settings.html.twig',
164
                            [
165
                                'blog'               => $blog,
166
                                'form'               => $form->createView(),
167
                                'businessProperties' => $businessProperties,
168
                            ]
169
                        ),
170
                        'message' => $errors,
171
                    ]
172
                );
173
        }
174
175
        return new Response($this->container->get('victoire_templating')->render(
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Symfony\Comp...$businessProperties))); (Symfony\Component\HttpFoundation\Response) is incompatible with the return type of the parent method Victoire\Bundle\PageBund...troller::settingsAction of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
176
                    $this->getBaseTemplatePath().':Tabs/_settings.html.twig',
177
                    [
178
                        'blog'               => $blog,
179
                        'form'               => $form->createView(),
180
                        'businessProperties' => $businessProperties,
181
                    ]
182
                )
183
        );
184
    }
185
186
    /**
187
     * Blog settings.
188
     *
189
     * @param Request  $request
190
     * @param BasePage $blog
191
     *
192
     * @return Response
193
     * @Route("/{id}/category", name="victoire_blog_category")
194
     * @ParamConverter("blog", class="VictoirePageBundle:BasePage")
195
     */
196
    public function categoryAction(Request $request, BasePage $blog)
197
    {
198
        $entityManager = $this->getDoctrine()->getManager();
199
        $form = $this->createForm($this->getPageCategoryType(), $blog);
200
        $businessProperties = [];
201
202
        //if the page is a business entity page
203
        if ($blog instanceof BusinessTemplate) {
204
            //we can use the business entity properties on the seo
205
            $businessEntity = $this->get('victoire_core.helper.business_entity_helper')->findById($blog->getBusinessEntityId());
206
            $businessProperties = $businessEntity->getBusinessPropertiesByType('seoable');
207
        }
208
209
        $form->handleRequest($request);
210
211
        if ($form->isValid()) {
212
            $entityManager->persist($blog);
213
            $entityManager->flush();
214
215
            return new JsonResponse([
216
                'success' => true,
217
                'url'     => $this->generateUrl('victoire_core_page_show', ['_locale' => $blog->getLocale(), 'url' => $blog->getUrl()]), ]);
218
        }
219
        //we display the form
220
        $errors = $this->get('victoire_form.error_helper')->getRecursiveReadableErrors($form);
221
        if ($errors != '') {
222
            return new JsonResponse(['html' => $this->container->get('victoire_templating')->render(
223
                        $this->getBaseTemplatePath().':Tabs/_category.html.twig',
224
                            [
225
                                'blog'               => $blog,
226
                                'form'               => $form->createView(),
227
                                'businessProperties' => $businessProperties,
228
                            ]
229
                        ),
230
                        'message' => $errors,
231
                    ]
232
                );
233
        }
234
235
        return new Response($this->container->get('victoire_templating')->render(
236
                    $this->getBaseTemplatePath().':Tabs/_category.html.twig',
237
                    [
238
                        'blog'               => $blog,
239
                        'form'               => $form->createView(),
240
                        'businessProperties' => $businessProperties,
241
                    ]
242
                )
243
        );
244
    }
245
246
    /**
247
     * Blog settings.
248
     *
249
     * @param Request  $request
250
     * @param BasePage $blog
251
     *
252
     * @return Response
253
     * @Route("/{id}/articles", name="victoire_blog_articles")
254
     * @ParamConverter("blog", class="VictoirePageBundle:BasePage")
255
     */
256
    public function articlesAction(Request $request, BasePage $blog)
0 ignored issues
show
Unused Code introduced by
The parameter $request is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
257
    {
258
        return new Response($this->container->get('victoire_templating')->render(
259
                    $this->getBaseTemplatePath().':Tabs/_articles.html.twig',
260
                    [
261
                        'blog' => $blog,
262
                    ]
263
                )
264
        );
265
    }
266
267
    /**
268
     * Page delete.
269
     *
270
     * @param Blog $blog
271
     *
272
     * @return JsonResponse
273
     * @Route("/{id}/delete", name="victoire_blog_delete")
274
     * @Template()
275
     * @ParamConverter("blog", class="VictoirePageBundle:BasePage")
276
     */
277
    public function deleteAction(BasePage $blog)
278
    {
279
        if (!$this->get('security.authorization_checker')->isGranted('ROLE_VICTOIRE', $blog)) {
280
            throw new AccessDeniedException("Nop ! you can't do such an action");
281
        }
282
283
        foreach ($blog->getArticles() as $_article) {
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Victoire\Bundle\PageBundle\Entity\BasePage as the method getArticles() does only exist in the following sub-classes of Victoire\Bundle\PageBundle\Entity\BasePage: Victoire\Bundle\BlogBundle\Entity\Blog. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

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

class MyUser extends 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 sub-classes 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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
284
            $bep = $this->get('victoire_page.page_helper')->findPageByParameters(
285
                [
286
                    'templateId' => $_article->getTemplate()->getId(),
287
                    'entityId'   => $_article->getId(),
288
                ]
289
            );
290
            $this->get('victoire_blog.manager.article')->delete($_article, $bep);
291
        }
292
293
        return new JsonResponse(parent::deleteAction($blog));
0 ignored issues
show
Bug Best Practice introduced by
The return type of return new \Symfony\Comp...::deleteAction($blog)); (Symfony\Component\HttpFoundation\JsonResponse) is incompatible with the return type of the parent method Victoire\Bundle\PageBund...ontroller::deleteAction of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
294
    }
295
296
    /**
297
     * @return string
298
     */
299
    protected function getPageSettingsType()
300
    {
301
        return 'victoire_blog_settings_type';
302
    }
303
304
    /**
305
     * @return string
306
     */
307
    protected function getPageCategoryType()
308
    {
309
        return 'victoire_blog_category_type';
310
    }
311
312
    /**
313
     * @return string
314
     */
315
    protected function getNewPageType()
316
    {
317
        return 'victoire_blog_type';
318
    }
319
320
    /**
321
     * @return \Victoire\Bundle\BlogBundle\Entity\Blog
322
     */
323
    protected function getNewPage()
324
    {
325
        return new Blog();
326
    }
327
328
    /**
329
     * @return string
330
     */
331
    protected function getBaseTemplatePath()
332
    {
333
        return 'VictoireBlogBundle:Blog';
334
    }
335
336
    /**
337
     * @param unknown $action
338
     */
339
    protected function getRoutes($action)
340
    {
341
        return $this->routes[$action];
342
    }
343
}
344