Merge branch 'develop'

hackzilla · hackzilla · commit 66e9e90ae9ed · 2016-01-23T22:24:55.000Z
diff --git a/Controller/TicketController.php b/Controller/TicketController.php
@@ -151,7 +151,7 @@ public function showAction(Ticket $ticket = null)
 
         if ($userManager->getCurrentUser() && $this->get('hackzilla_ticket.user_manager')->hasRole(
                 $userManager->getCurrentUser(),
-                TicketRole::Admin
+                TicketRole::ADMIN
             )
         ) {
             $data['delete_form'] = $this->createDeleteForm($ticket->getId())->createView();
@@ -168,7 +168,7 @@ private function checkUserPermission($user, Ticket $ticket)
     {
         if (!\is_object($user) || (!$this->get('hackzilla_ticket.user_manager')->hasRole(
                     $user,
-                    TicketRole::Admin
+                    TicketRole::ADMIN
                 ) && $ticket->getUserCreated() != $user->getId())
         ) {
             throw new \Symfony\Component\HttpKernel\Exception\HttpException(403);
@@ -231,7 +231,7 @@ public function deleteAction(Request $request, Ticket $ticket)
         $userManager = $this->get('hackzilla_ticket.user_manager');
         $user = $userManager->getCurrentUser();
 
-        if (!\is_object($user) || !$userManager->hasRole($user, TicketRole::Admin)) {
+        if (!\is_object($user) || !$userManager->hasRole($user, TicketRole::ADMIN)) {
             throw new \Symfony\Component\HttpKernel\Exception\HttpException(403);
         }
 
diff --git a/Entity/TicketRepository.php b/Entity/TicketRepository.php
@@ -41,7 +41,7 @@ public function getTicketList($userManager, $ticketStatus, $ticketPriority = nul
         $user = $userManager->getCurrentUser();
 
         if (\is_object($user)) {
-            if (!$userManager->hasRole($user, TicketRole::Admin)) {
+            if (!$userManager->hasRole($user, TicketRole::ADMIN)) {
                 $query
                     ->andWhere('t.userCreated = :userId')
                     ->setParameter('userId', $user->getId());
diff --git a/EventListener/UserLoad.php b/EventListener/UserLoad.php
@@ -5,15 +5,14 @@
 use Doctrine\ORM\Event\LifecycleEventArgs;
 use Hackzilla\Bundle\TicketBundle\Entity\Ticket;
 use Hackzilla\Bundle\TicketBundle\Entity\TicketMessage;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 
 class UserLoad
 {
-    protected $container;
+    protected $userRepository;
 
-    public function __construct(ContainerInterface $container)
+    public function __construct($userRepository)
     {
-        $this->container = $container;
+        $this->userRepository = $userRepository;
     }
 
     public function getSubscribedEvents()
@@ -26,18 +25,18 @@ public function getSubscribedEvents()
     public function postLoad(LifecycleEventArgs $args)
     {
         $entity = $args->getEntity();
-        $userManager = $this->container->get('hackzilla_ticket.user_manager');
+        $userRepository = $args->getEntityManager()->getRepository($this->userRepository);
 
         if ($entity instanceof Ticket) {
             if (\is_null($entity->getUserCreatedObject())) {
-                $entity->setUserCreated($userManager->getUserById($entity->getUserCreated()));
+                $entity->setUserCreated($userRepository->find($entity->getUserCreated()));
             }
             if (\is_null($entity->getLastUserObject())) {
-                $entity->setLastUser($userManager->getUserById($entity->getLastUser()));
+                $entity->setLastUser($userRepository->find($entity->getLastUser()));
             }
         } elseif ($entity instanceof TicketMessage) {
             if (\is_null($entity->getUserObject())) {
-                $entity->setUser($userManager->getUserById($entity->getUser()));
+                $entity->setUser($userRepository->find($entity->getUser()));
             }
         }
     }
diff --git a/Form/Type/TicketMessageType.php b/Form/Type/TicketMessageType.php
@@ -48,7 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
         if (isset($options['new_ticket']) && !$options['new_ticket']) {
             $user = $this->userManager->getCurrentUser();
 
-            if ($this->userManager->hasRole($user, TicketRole::Admin)) {
+            if ($this->userManager->hasRole($user, TicketRole::ADMIN)) {
                 $builder->add(
                     'status',
                     method_exists(AbstractType::class, 'getBlockPrefix') ? StatusType::class : new StatusType(),
diff --git a/Manager/TicketManager.php b/Manager/TicketManager.php
@@ -8,9 +8,12 @@
 
 class TicketManager implements TicketManagerInterface
 {
+    private $objectManager;
+    private $repository;
+
     public function __construct(ObjectManager $om)
     {
-        $this->ObjectManager = $om;
+        $this->objectManager = $om;
         $this->repository = $om->getRepository('HackzillaTicketBundle:Ticket');
     }
 
@@ -42,12 +45,12 @@ public function createMessage()
     public function updateTicket(Ticket $ticket, TicketMessage $message = null)
     {
         if (!\is_null($ticket)) {
-            $this->ObjectManager->persist($ticket);
+            $this->objectManager->persist($ticket);
         }
         if (!\is_null($message)) {
-            $this->ObjectManager->persist($message);
+            $this->objectManager->persist($message);
         }
-        $this->ObjectManager->flush();
+        $this->objectManager->flush();
 
         return $ticket;
     }
@@ -59,8 +62,8 @@ public function updateTicket(Ticket $ticket, TicketMessage $message = null)
      */
     public function deleteTicket(Ticket $ticket)
     {
-        $this->ObjectManager->remove($ticket);
-        $this->ObjectManager->flush();
+        $this->objectManager->remove($ticket);
+        $this->objectManager->flush();
     }
 
     /**
diff --git a/Manager/UserManager.php b/Manager/UserManager.php
@@ -5,20 +5,16 @@
 use Doctrine\ORM\EntityRepository;
 use Hackzilla\Bundle\TicketBundle\Model\UserInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
-use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
 
 class UserManager implements UserManagerInterface
 {
-    private $authorizationChecker;
     private $tokenStorage;
     private $userRepository;
 
     public function __construct(
-        AuthorizationChecker $authorizationChecker,
         TokenStorage $tokenStorage,
         EntityRepository $userRepository
     ) {
-        $this->authorizationChecker = $authorizationChecker;
         $this->tokenStorage = $tokenStorage;
         $this->userRepository = $userRepository;
     }
@@ -44,6 +40,10 @@ public function getCurrentUser()
      */
     public function getUserById($userId)
     {
+        if (!$userId) {
+            return;
+        }
+
         $user = $this->userRepository->find($userId);
 
         return $user;
diff --git a/README.md b/README.md
@@ -62,6 +62,8 @@ hackzilla_ticket:
 
 Your user class needs to implement ```Hackzilla\Bundle\TicketBundle\Model\UserInterface```
 
+Roles are now checked against the User
+
 
 ## Pull Requests
 
diff --git a/Resources/config/services.yml b/Resources/config/services.yml
@@ -1,7 +1,8 @@
 services:
     hackzilla_ticket.listener:
         class: Hackzilla\Bundle\TicketBundle\EventListener\UserLoad
-        arguments: ['@service_container']
+        arguments:
+            - %hackzilla_ticket.model.user.class%
         tags:
             - { name: doctrine.event_listener, event: postLoad }
 
@@ -13,12 +14,13 @@ services:
 
     hackzilla_ticket.user_manager:
         class: Hackzilla\Bundle\TicketBundle\Manager\UserManager
-        arguments: ['@security.authorization_checker', '@security.token_storage', '@hackzilla_ticket.user_repository']
+        arguments:
+            - '@security.token_storage'
+            - '@hackzilla_ticket.user_repository'
 
     hackzilla_ticket.user_repository:
         class: Doctrine\ORM\EntityRepository
-        factory_service: doctrine.orm.default_entity_manager
-        factory_method: getRepository
+        factory: ['@doctrine.orm.default_entity_manager', getRepository]
         arguments:
             - %hackzilla_ticket.model.user.class%
 
diff --git a/Resources/views/Ticket/new.html.twig b/Resources/views/Ticket/new.html.twig
@@ -4,8 +4,7 @@
     <h1>{{ 'HEADING_NEW_TICKET'|trans }}</h1>
 
     <div class="well well-sm">
-        <form action="{{ path('hackzilla_ticket_create') }}" method="post" {{ form_enctype(form) }}
-              class="form-horizontal" role="form">
+        {{ form_start(form, {'method': 'POST', 'action': path('hackzilla_ticket_create')}) }}
             <div class="form-group row">
                 {{ form_label(form.subject, '', { 'label_attr': {'class': 'col-lg-2 col-md-2 col-sm-2 control-label'} }) }}
                 <div class="col-lg-8 col-md-8 col-sm-8">
@@ -26,6 +25,6 @@
                     </a>
                 </div>
             </div>
-        </form>
+        {{ form_end(form) }}
     </div>
 {% endblock %}
diff --git a/Resources/views/Ticket/show.html.twig b/Resources/views/Ticket/show.html.twig
@@ -62,25 +62,25 @@
 
     {% if form is defined %}
         <div class="well well-sm">
-            <form action="{{ path('hackzilla_ticket_reply', {id: ticket.id}) }}" method="post" {{ form_enctype(form) }}>
-                {% include 'HackzillaTicketBundle:Ticket:prototype.html.twig' with {'form': form} %}
+            {{ form_start(form, {'method': 'POST', 'action': path('hackzilla_ticket_reply', {'id': ticket.id})}) }}
+            {% include 'HackzillaTicketBundle:Ticket:prototype.html.twig' with {'form': form} %}
 
-                {{ form_rest(form) }}
+            {{ form_rest(form) }}
 
-                <p class="form_actions">
-                    <button type="submit" class="btn btn-primary">{{ 'BUTTON_UPDATE'|trans }}</button>
-                </p>
-            </form>
+            <p class="form_actions">
+                <button type="submit" class="btn btn-primary">{{ 'BUTTON_UPDATE'|trans }}</button>
+            </p>
+            {{ form_end(form) }}
         </div>
     {% endif %}
 
     {% if delete_form is defined %}
-        <form action="{{ path('hackzilla_ticket_delete', { 'id': ticket.id }) }}" method="post" class="text-right">
-            <input type="hidden" name="_method" value="DELETE"/>
-            {{ form_widget(delete_form) }}
+        {{ form_start(form, {'method': 'POST', 'action': path('hackzilla_ticket_delete', {'id': ticket.id}), 'attr': {'class': 'text-right'}}) }}
+        <input type="hidden" name="_method" value="DELETE"/>
+        {{ form_widget(delete_form) }}
 
-            <button type="submit" class="btn btn-danger">{{ 'BUTTON_DELETE'|trans }}</button>
-        </form>
+        <button type="submit" class="btn btn-danger">{{ 'BUTTON_DELETE'|trans }}</button>
+        {{ form_end(delete_form) }}
     {% endif %}
 
 {% endblock %}
diff --git a/Tests/EventListener/UserLoadTest.php b/Tests/EventListener/UserLoadTest.php
@@ -2,6 +2,7 @@
 
 namespace Hackzilla\Bundle\TicketBundle\Tests\EventListener;
 
+use Hackzilla\Bundle\TicketBundle\Manager\UserManager;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 
 class UserLoadTest extends WebTestCase
@@ -10,14 +11,17 @@ class UserLoadTest extends WebTestCase
 
     public function setUp()
     {
-        $container = $this->getMockContainer();
+        $userManager = $this->getUserManagerMock();
 
-        $this->object = new \Hackzilla\Bundle\TicketBundle\EventListener\UserLoad($container);
+        $this->object = new \Hackzilla\Bundle\TicketBundle\EventListener\UserLoad($userManager);
     }
 
-    public function getMockContainer()
+    public function getUserManagerMock()
     {
-        return $this->getMock('Symfony\Component\DependencyInjection\Container');
+        return $this
+            ->getMockBuilder(UserManager::class)
+            ->disableOriginalConstructor()
+            ->getMock();
     }
 
     public function tearDown()
diff --git a/Tests/Manager/UserManagerTest.php b/Tests/Manager/UserManagerTest.php
@@ -5,7 +5,6 @@
 use Doctrine\ORM\EntityRepository;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
-use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
 
 class UserManagerTest extends WebTestCase
 {
@@ -17,20 +16,11 @@ public function setUp()
         $this->tokenStorage = new TokenStorage();
 
         $this->object = new \Hackzilla\Bundle\TicketBundle\Manager\UserManager(
-            $this->getMockAuthorizationChecker(),
             $this->tokenStorage,
             $this->getMockUserRepository()
         );
     }
 
-    private function getMockAuthorizationChecker()
-    {
-        return $this
-            ->getMockBuilder(AuthorizationChecker::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-    }
-
     private function getMockUserRepository()
     {
         $userRepository = $this
diff --git a/TicketRole.php b/TicketRole.php
@@ -4,5 +4,5 @@
 
 class TicketRole
 {
-    const Admin = 'ROLE_TICKET_ADMIN';
+    const ADMIN = 'ROLE_TICKET_ADMIN';
 }

Original file line number	Diff line number	Diff line change
`@@ -5,15 +5,14 @@`
`5`	`5`	`use Doctrine\ORM\Event\LifecycleEventArgs;`
`6`	`6`	`use Hackzilla\Bundle\TicketBundle\Entity\Ticket;`
`7`	`7`	`use Hackzilla\Bundle\TicketBundle\Entity\TicketMessage;`
`8`		`-use Symfony\Component\DependencyInjection\ContainerInterface;`
`9`	`8`
`10`	`9`	`class UserLoad`
`11`	`10`	`{`
`12`		`- protected $container;`
	`11`	`+ protected $userRepository;`
`13`	`12`
`14`		`- public function __construct(ContainerInterface $container)`
	`13`	`+ public function __construct($userRepository)`
`15`	`14`	`{`
`16`		`- $this->container = $container;`
	`15`	`+ $this->userRepository = $userRepository;`
`17`	`16`	`}`
`18`	`17`
`19`	`18`	`public function getSubscribedEvents()`
`@@ -26,18 +25,18 @@ public function getSubscribedEvents()`
`26`	`25`	`public function postLoad(LifecycleEventArgs $args)`
`27`	`26`	`{`
`28`	`27`	`$entity = $args->getEntity();`
`29`		`- $userManager = $this->container->get('hackzilla_ticket.user_manager');`
	`28`	`+ $userRepository = $args->getEntityManager()->getRepository($this->userRepository);`
`30`	`29`
`31`	`30`	`if ($entity instanceof Ticket) {`
`32`	`31`	`if (\is_null($entity->getUserCreatedObject())) {`
`33`		`- $entity->setUserCreated($userManager->getUserById($entity->getUserCreated()));`
	`32`	`+ $entity->setUserCreated($userRepository->find($entity->getUserCreated()));`
`34`	`33`	`}`
`35`	`34`	`if (\is_null($entity->getLastUserObject())) {`
`36`		`- $entity->setLastUser($userManager->getUserById($entity->getLastUser()));`
	`35`	`+ $entity->setLastUser($userRepository->find($entity->getLastUser()));`
`37`	`36`	`}`
`38`	`37`	`} elseif ($entity instanceof TicketMessage) {`
`39`	`38`	`if (\is_null($entity->getUserObject())) {`
`40`		`- $entity->setUser($userManager->getUserById($entity->getUser()));`
	`39`	`+ $entity->setUser($userRepository->find($entity->getUser()));`
`41`	`40`	`}`
`42`	`41`	`}`
`43`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,9 +8,12 @@`
`8`	`8`
`9`	`9`	`class TicketManager implements TicketManagerInterface`
`10`	`10`	`{`
	`11`	`+ private $objectManager;`
	`12`	`+ private $repository;`
	`13`	`+`
`11`	`14`	`public function __construct(ObjectManager $om)`
`12`	`15`	`{`
`13`		`- $this->ObjectManager = $om;`
	`16`	`+ $this->objectManager = $om;`
`14`	`17`	`$this->repository = $om->getRepository('HackzillaTicketBundle:Ticket');`
`15`	`18`	`}`
`16`	`19`
`@@ -42,12 +45,12 @@ public function createMessage()`
`42`	`45`	`public function updateTicket(Ticket $ticket, TicketMessage $message = null)`
`43`	`46`	`{`
`44`	`47`	`if (!\is_null($ticket)) {`
`45`		`- $this->ObjectManager->persist($ticket);`
	`48`	`+ $this->objectManager->persist($ticket);`
`46`	`49`	`}`
`47`	`50`	`if (!\is_null($message)) {`
`48`		`- $this->ObjectManager->persist($message);`
	`51`	`+ $this->objectManager->persist($message);`
`49`	`52`	`}`
`50`		`- $this->ObjectManager->flush();`
	`53`	`+ $this->objectManager->flush();`
`51`	`54`
`52`	`55`	`return $ticket;`
`53`	`56`	`}`
`@@ -59,8 +62,8 @@ public function updateTicket(Ticket $ticket, TicketMessage $message = null)`
`59`	`62`	`*/`
`60`	`63`	`public function deleteTicket(Ticket $ticket)`
`61`	`64`	`{`
`62`		`- $this->ObjectManager->remove($ticket);`
`63`		`- $this->ObjectManager->flush();`
	`65`	`+ $this->objectManager->remove($ticket);`
	`66`	`+ $this->objectManager->flush();`
`64`	`67`	`}`
`65`	`68`
`66`	`69`	`/**`