From 4e344dbe6eaf0c8c1ba208d95883513e10bbdca0 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sun, 7 Jan 2018 19:39:43 +0100 Subject: [PATCH] Create timesheet for User #31 (#47) * added doctrine listener to calculate duration on each update #31 * added constraints to base objects #31 * new form to create timesheet entries #31 * added unit test for TimesheetVoter #31 --- app/config/services.yml | 6 ++ src/AppBundle/Repository/Query/BaseQuery.php | 5 +- src/AppBundle/Voter/AbstractVoter.php | 2 +- .../Controller/TimesheetController.php | 50 ++++++++++++- src/TimesheetBundle/Entity/Activity.php | 4 + src/TimesheetBundle/Entity/Customer.php | 3 + src/TimesheetBundle/Entity/Project.php | 5 ++ src/TimesheetBundle/Entity/Timesheet.php | 5 ++ .../EventListener/TimesheetListener.php | 69 +++++++++++++++++ .../Repository/Query/TimesheetQuery.php | 2 +- .../Repository/TimesheetRepository.php | 8 +- .../views/Navbar/active-entries.html.twig | 2 +- .../Resources/views/timesheet/edit.html.twig | 2 +- .../Resources/views/timesheet/index.html.twig | 2 +- src/TimesheetBundle/Voter/TimesheetVoter.php | 5 -- .../Repository/Query/TimesheetQueryTest.php | 74 ++++++------------- .../Voter/TimesheetVoterTest.php | 73 ++++++++++++++++++ 17 files changed, 246 insertions(+), 71 deletions(-) create mode 100644 src/TimesheetBundle/EventListener/TimesheetListener.php create mode 100644 tests/TimesheetBundle/Voter/TimesheetVoterTest.php diff --git a/app/config/services.yml b/app/config/services.yml index 82c92a74..6477ba37 100644 --- a/app/config/services.yml +++ b/app/config/services.yml @@ -94,6 +94,12 @@ services: tags: - { name: doctrine.event_subscriber } + app.database_listener.timesheet: + class: TimesheetBundle\EventListener\TimesheetListener + tags: + - { name: doctrine.event_listener, event: prePersist, lazy: true } + - { name: doctrine.event_listener, event: preUpdate, lazy: true } + # Uncomment the following lines to define a service for the Post Doctrine repository. # It's not mandatory to create these services, but if you use repositories a lot, # these services simplify your code: diff --git a/src/AppBundle/Repository/Query/BaseQuery.php b/src/AppBundle/Repository/Query/BaseQuery.php index f2699170..37a39c92 100644 --- a/src/AppBundle/Repository/Query/BaseQuery.php +++ b/src/AppBundle/Repository/Query/BaseQuery.php @@ -19,6 +19,9 @@ namespace AppBundle\Repository\Query; class BaseQuery { + const ORDER_ASC = 'ASC'; + const ORDER_DESC = 'DESC'; + const DEFAULT_PAGESIZE = 25; const DEFAULT_PAGE = 1; @@ -118,7 +121,7 @@ class BaseQuery */ public function setOrder($order) { - if (in_array($order, ['ASC', 'DESC'])) { + if (in_array($order, [self::ORDER_ASC, self::ORDER_DESC])) { $this->order = $order; } return $this; diff --git a/src/AppBundle/Voter/AbstractVoter.php b/src/AppBundle/Voter/AbstractVoter.php index 5e3e7a9f..188ea508 100644 --- a/src/AppBundle/Voter/AbstractVoter.php +++ b/src/AppBundle/Voter/AbstractVoter.php @@ -41,7 +41,7 @@ abstract class AbstractVoter extends Voter * @param TokenInterface $token * @return bool */ - public function hasRole($role, TokenInterface $token) + protected function hasRole($role, TokenInterface $token) { if ($this->decisionManager->decide($token, array($role))) { return true; diff --git a/src/TimesheetBundle/Controller/TimesheetController.php b/src/TimesheetBundle/Controller/TimesheetController.php index 2200a674..553a1d23 100644 --- a/src/TimesheetBundle/Controller/TimesheetController.php +++ b/src/TimesheetBundle/Controller/TimesheetController.php @@ -14,6 +14,7 @@ namespace TimesheetBundle\Controller; use AppBundle\Controller\AbstractController; use Pagerfanta\Pagerfanta; use TimesheetBundle\Entity\Activity; +use TimesheetBundle\Entity\Customer; use TimesheetBundle\Entity\Timesheet; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; @@ -121,7 +122,7 @@ class TimesheetController extends AbstractController } /** - * The route to edit an existing entry or to create a complete new entry. + * The route to edit an existing entry. * * @Route("/{id}/edit", name="timesheet_edit") * @Method({"GET", "POST"}) @@ -155,6 +156,53 @@ class TimesheetController extends AbstractController ); } + /** + * The route to create a new entry by form. + * + * @Route("/create", name="timesheet_create") + * @Method({"GET", "POST"}) + * + * @param Request $request + * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response + */ + public function createAction(Request $request) + { + $entry = new Timesheet(); + $entry->setUser($this->getUser()); + $entry->setBegin(new \DateTime()); + + $createForm = $this->createForm( + TimesheetEditForm::class, + $entry, + [ + 'action' => $this->generateUrl('timesheet_create'), + 'method' => 'POST', + 'currency' => Customer::DEFAULT_CURRENCY, + ] + ); + + $createForm->handleRequest($request); + + if ($createForm->isSubmitted() && $createForm->isValid()) { + $entityManager = $this->getDoctrine()->getManager(); + $entityManager->persist($entry); + + $entityManager->flush(); + + $this->flashSuccess('action.updated_successfully'); + + return $this->redirectToRoute('timesheet'); + } + + return $this->render( + 'TimesheetBundle:timesheet:edit.html.twig', + [ + 'entry' => $entry, + 'form' => $createForm->createView(), + ] + ); + } + /** * @param Timesheet $entry * @param int $page diff --git a/src/TimesheetBundle/Entity/Activity.php b/src/TimesheetBundle/Entity/Activity.php index ee16e260..1d82363b 100644 --- a/src/TimesheetBundle/Entity/Activity.php +++ b/src/TimesheetBundle/Entity/Activity.php @@ -12,6 +12,7 @@ namespace TimesheetBundle\Entity; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; /** * Activity @@ -37,6 +38,7 @@ class Activity * @var Project * * @ORM\ManyToOne(targetEntity="TimesheetBundle\Entity\Project", inversedBy="activities") + * @Assert\NotNull() */ private $project; @@ -44,6 +46,7 @@ class Activity * @var string * * @ORM\Column(name="name", type="string", length=255, nullable=false) + * @Assert\NotBlank() */ private $name; @@ -58,6 +61,7 @@ class Activity * @var boolean * * @ORM\Column(name="visible", type="boolean", nullable=false) + * @Assert\NotNull() */ private $visible = true; diff --git a/src/TimesheetBundle/Entity/Customer.php b/src/TimesheetBundle/Entity/Customer.php index c2538d84..3ab15bb1 100644 --- a/src/TimesheetBundle/Entity/Customer.php +++ b/src/TimesheetBundle/Entity/Customer.php @@ -40,6 +40,7 @@ class Customer * @var string * * @ORM\Column(name="name", type="string", length=255, nullable=false) + * @Assert\NotBlank() */ private $name; @@ -61,6 +62,7 @@ class Customer * @var boolean * * @ORM\Column(name="visible", type="boolean", nullable=false) + * @Assert\NotNull() */ private $visible = true; @@ -106,6 +108,7 @@ class Customer * @var string * * @ORM\Column(name="currency", type="string", length=3, nullable=false) + * @Assert\NotBlank() */ private $currency = self::DEFAULT_CURRENCY; diff --git a/src/TimesheetBundle/Entity/Project.php b/src/TimesheetBundle/Entity/Project.php index de1da20f..5c470330 100644 --- a/src/TimesheetBundle/Entity/Project.php +++ b/src/TimesheetBundle/Entity/Project.php @@ -12,6 +12,7 @@ namespace TimesheetBundle\Entity; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; /** * Project @@ -37,6 +38,7 @@ class Project * @var Customer * * @ORM\ManyToOne(targetEntity="TimesheetBundle\Entity\Customer", inversedBy="projects") + * @Assert\NotNull() */ private $customer; @@ -44,6 +46,7 @@ class Project * @var string * * @ORM\Column(name="name", type="string", length=255, nullable=false) + * @Assert\NotNull() */ private $name; @@ -58,6 +61,7 @@ class Project * @var boolean * * @ORM\Column(name="visible", type="boolean", nullable=false) + * @Assert\NotNull() */ private $visible = true; @@ -65,6 +69,7 @@ class Project * @var string * * @ORM\Column(name="budget", type="decimal", precision=10, scale=2, nullable=false) + * @Assert\NotNull() */ private $budget = 0.00; diff --git a/src/TimesheetBundle/Entity/Timesheet.php b/src/TimesheetBundle/Entity/Timesheet.php index fd9db0e3..36a25f64 100644 --- a/src/TimesheetBundle/Entity/Timesheet.php +++ b/src/TimesheetBundle/Entity/Timesheet.php @@ -13,6 +13,7 @@ namespace TimesheetBundle\Entity; use AppBundle\Entity\User; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; /** * Timesheet entity. @@ -25,6 +26,7 @@ use Doctrine\ORM\Mapping as ORM; * @ORM\Index(name="activity", columns={"activity"}) * } * ) + * @ORM\HasLifecycleCallbacks() * * @author Kevin Papst */ @@ -44,6 +46,7 @@ class Timesheet * @var \DateTime * * @ORM\Column(name="start_time", type="datetime", nullable=false) + * @Assert\NotNull() */ private $begin; @@ -66,6 +69,7 @@ class Timesheet * * @ORM\ManyToOne(targetEntity="AppBundle\Entity\User") * @ORM\JoinColumn(name="user", referencedColumnName="id") + * @Assert\NotNull() */ private $user; @@ -74,6 +78,7 @@ class Timesheet * * @ORM\ManyToOne(targetEntity="TimesheetBundle\Entity\Activity") * @ORM\JoinColumn(name="activity", referencedColumnName="id") + * @Assert\NotNull() */ private $activity; diff --git a/src/TimesheetBundle/EventListener/TimesheetListener.php b/src/TimesheetBundle/EventListener/TimesheetListener.php new file mode 100644 index 00000000..93cadadd --- /dev/null +++ b/src/TimesheetBundle/EventListener/TimesheetListener.php @@ -0,0 +1,69 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace TimesheetBundle\EventListener; + +use Doctrine\Common\EventSubscriber; +use Doctrine\ORM\Event\PreUpdateEventArgs; +use Doctrine\ORM\Event\LifecycleEventArgs; +use TimesheetBundle\Entity\Timesheet; + +/** + * A listener to make sure all Timesheet entries will have a proper duration. + * + * @author Kevin Papst + */ +class TimesheetListener implements EventSubscriber +{ + + /** + * @return array + */ + public function getSubscribedEvents() + { + return array( + 'prePersist', + 'preUpdate', + ); + } + + /** + * @param PreUpdateEventArgs $args + */ + public function preUpdate(PreUpdateEventArgs $args) + { + $this->calculateFields($args); + } + + /** + * @param LifecycleEventArgs $args + */ + public function prePersist(LifecycleEventArgs $args) + { + $this->calculateFields($args); + } + + /** + * @param LifecycleEventArgs $args + */ + protected function calculateFields(LifecycleEventArgs $args) + { + $entity = $args->getObject(); + + if ($entity instanceof Timesheet) { + if ($entity->getEnd() !== null) { + $entity->setDuration($entity->getEnd()->getTimestamp() - $entity->getBegin()->getTimestamp()); + } + + // TODO calculate hourly rate + } + } +} diff --git a/src/TimesheetBundle/Repository/Query/TimesheetQuery.php b/src/TimesheetBundle/Repository/Query/TimesheetQuery.php index a2909848..0789d365 100644 --- a/src/TimesheetBundle/Repository/Query/TimesheetQuery.php +++ b/src/TimesheetBundle/Repository/Query/TimesheetQuery.php @@ -32,7 +32,7 @@ class TimesheetQuery extends BaseQuery * Overwritten for different default order * @var string */ - protected $order = 'DESC'; + protected $order = self::ORDER_DESC; /** * Overwritten for different default order * @var string diff --git a/src/TimesheetBundle/Repository/TimesheetRepository.php b/src/TimesheetBundle/Repository/TimesheetRepository.php index d94d2f80..0fa4b192 100644 --- a/src/TimesheetBundle/Repository/TimesheetRepository.php +++ b/src/TimesheetBundle/Repository/TimesheetRepository.php @@ -40,13 +40,7 @@ class TimesheetRepository extends AbstractRepository */ public function stopRecording(Timesheet $entry) { - $end = new DateTime(); - $begin = $entry->getBegin(); - - $entry->setEnd($end); - $entry->setDuration($end->getTimestamp() - $begin->getTimestamp()); - - // TODO calculate rate by users hourly rate + $entry->setEnd(new DateTime()); $entityManager = $this->getEntityManager(); $entityManager->persist($entry); diff --git a/src/TimesheetBundle/Resources/views/Navbar/active-entries.html.twig b/src/TimesheetBundle/Resources/views/Navbar/active-entries.html.twig index 5d58cb40..0663ae18 100644 --- a/src/TimesheetBundle/Resources/views/Navbar/active-entries.html.twig +++ b/src/TimesheetBundle/Resources/views/Navbar/active-entries.html.twig @@ -26,6 +26,6 @@ {% endfor %} -
  • {{ 'timesheet.start'|trans }}
    • +
  • {{ 'timesheet.start'|trans }}
    • diff --git a/src/TimesheetBundle/Resources/views/timesheet/edit.html.twig b/src/TimesheetBundle/Resources/views/timesheet/edit.html.twig index 923fb4d6..2fe4244b 100644 --- a/src/TimesheetBundle/Resources/views/timesheet/edit.html.twig +++ b/src/TimesheetBundle/Resources/views/timesheet/edit.html.twig @@ -7,7 +7,7 @@ {% block main %} {{ include('default/_form.html.twig', { - 'title': 'timesheet.edit'|trans, + 'title': (entry.id ? 'timesheet.edit'|trans : 'create'|trans), 'form': form, 'back': path('timesheet') }) }} diff --git a/src/TimesheetBundle/Resources/views/timesheet/index.html.twig b/src/TimesheetBundle/Resources/views/timesheet/index.html.twig index 2ca3c68a..2bf41f4b 100644 --- a/src/TimesheetBundle/Resources/views/timesheet/index.html.twig +++ b/src/TimesheetBundle/Resources/views/timesheet/index.html.twig @@ -20,7 +20,7 @@ 'label.activity': 'hidden-xs hidden-sm', 'label.description': 'hidden-xs hidden-sm', 'label.actions': '', - }, toolbarForm) }} + }, toolbarForm, {'plus-square': path('timesheet_create')}) }} {% for entry in entries %} diff --git a/src/TimesheetBundle/Voter/TimesheetVoter.php b/src/TimesheetBundle/Voter/TimesheetVoter.php index 4aa33f5a..295a3763 100644 --- a/src/TimesheetBundle/Voter/TimesheetVoter.php +++ b/src/TimesheetBundle/Voter/TimesheetVoter.php @@ -66,11 +66,6 @@ class TimesheetVoter extends AbstractVoter return false; } - // Customer cannot do anything with timesheet entries - if (!$this->hasRole('ROLE_USER', $token)) { - return false; - } - switch ($attribute) { case self::STOP: return $this->canStop($subject, $user, $token); diff --git a/tests/TimesheetBundle/Repository/Query/TimesheetQueryTest.php b/tests/TimesheetBundle/Repository/Query/TimesheetQueryTest.php index 6a23bdcb..2ae8825f 100644 --- a/tests/TimesheetBundle/Repository/Query/TimesheetQueryTest.php +++ b/tests/TimesheetBundle/Repository/Query/TimesheetQueryTest.php @@ -1,72 +1,42 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. */ namespace KimaiTest\TimesheetBundle\Repository\Query; use AppBundle\Repository\Query\BaseQuery; use \PHPUnit\Framework\TestCase; +use TimesheetBundle\Repository\Query\TimesheetQuery; +/** + * @covers \TimesheetBundle\Repository\Query\TimesheetQuery + * @author Kevin Papst + */ class TimesheetQueryTest extends TestCase { - public function testBaseQueryHasOverwrittenFields() + + public function testSetOrder() { $class = new \ReflectionClass(new BaseQuery()); $this->assertTrue($class->hasProperty('order')); $this->assertTrue($class->hasProperty('orderBy')); - } - public function testGetUser() - { - $this->markTestIncomplete(__METHOD__); - } + $sut = new TimesheetQuery(); - public function testSetUser() - { - $this->markTestIncomplete(__METHOD__); - } + $this->assertEquals(TimesheetQuery::ORDER_DESC, $sut->getOrder()); + $this->assertEquals('begin', $sut->getOrderBy()); - public function testGetActivity() - { - $this->markTestIncomplete(__METHOD__); - } + $sut->setOrder(TimesheetQuery::ORDER_ASC); + $sut->setOrderBy('id'); - public function testSetActivity() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testGetProject() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testSetProject() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testGetCustomer() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testSetCustomer() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testGetState() - { - $this->markTestIncomplete(__METHOD__); - } - - public function testSetState() - { - $this->markTestIncomplete(__METHOD__); + $this->assertEquals(TimesheetQuery::ORDER_ASC, $sut->getOrder()); + $this->assertEquals('id', $sut->getOrderBy()); } } diff --git a/tests/TimesheetBundle/Voter/TimesheetVoterTest.php b/tests/TimesheetBundle/Voter/TimesheetVoterTest.php new file mode 100644 index 00000000..5a367536 --- /dev/null +++ b/tests/TimesheetBundle/Voter/TimesheetVoterTest.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace KimaiTest\TimesheetBundle\Voter; + +use AppBundle\Entity\User; +use \PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use TimesheetBundle\Entity\Customer; +use TimesheetBundle\Entity\Timesheet; +use TimesheetBundle\Voter\TimesheetVoter; + +/** + * @covers \TimesheetBundle\Voter\TimesheetVoter + * @author Kevin Papst + */ +class TimesheetVoterTest extends TestCase +{ + + /** + * @dataProvider getTestData + */ + public function testCustomerIsDisallowed($user, $allow, $subject, $attributes, $result) + { + $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles()); + + $accessManager = $this->getMockBuilder(AccessDecisionManagerInterface::class)->getMock(); + $accessManager->method('decide')->willReturn($allow); + + $sut = new TimesheetVoter($accessManager); + + $this->assertEquals($result, $sut->vote($token, $subject, $attributes)); + } + + public function getTestData() + { + $user0 = $this->getUser(0, 'ROLE_CUSTOMER'); + $user1 = $this->getUser(1, 'ROLE_USER'); + $user2 = $this->getUser(1, 'ROLE_TEAMLEAD'); + + return [ + [$user0, false, new Customer(), ['edit'], VoterInterface::ACCESS_ABSTAIN], + [$user1, false, $this->getTimesheet($user1), ['edit'], VoterInterface::ACCESS_GRANTED], + [$user1, false, $this->getTimesheet($user0), ['edit'], VoterInterface::ACCESS_DENIED], + [$user2, true, $this->getTimesheet($user1), ['edit'], VoterInterface::ACCESS_GRANTED], + ]; + } + + protected function getTimesheet($user) + { + $timesheet = new Timesheet(); + $timesheet->setUser($user); + return $timesheet; + } + + protected function getUser($id, $role) + { + $user = $this->getMockBuilder(User::class)->getMock(); + $user->method('getId')->willReturn($id); + $user->method('getRoles')->willReturn([$role]); + return $user; + } +}