From 8005679e6056868795d6f97e2eb8564e4138a868 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Thu, 14 Feb 2019 00:04:27 +0100 Subject: [PATCH] allow to query other users timesheets via api (#563) --- config/packages/nelmio_api_doc.yaml | 17 ++--- config/serializer/App/Entity.Timesheet.yml | 2 +- src/API/ActivityController.php | 5 +- src/API/CustomerController.php | 5 +- src/API/ProjectController.php | 5 +- src/API/TimesheetController.php | 38 ++++++++-- src/API/UserController.php | 5 +- src/Controller/Admin/TimesheetController.php | 5 +- .../Toolbar/TimesheetAdminToolbarForm.php | 33 -------- src/Form/Toolbar/TimesheetToolbarForm.php | 4 + src/Repository/Query/TimesheetQuery.php | 4 +- tests/API/TimesheetControllerTest.php | 76 +++++++++++++++++-- tests/DataFixtures/TimesheetFixtures.php | 26 +++++-- 13 files changed, 153 insertions(+), 72 deletions(-) delete mode 100644 src/Form/Toolbar/TimesheetAdminToolbarForm.php diff --git a/config/packages/nelmio_api_doc.yaml b/config/packages/nelmio_api_doc.yaml index 4db4bc66..fb1a876b 100644 --- a/config/packages/nelmio_api_doc.yaml +++ b/config/packages/nelmio_api_doc.yaml @@ -2,17 +2,12 @@ nelmio_api_doc: models: use_jms: true names: - - { alias: CustomerEntity, type: App\Entity\Customer, groups: [Default, Entity, Customer] } - - { alias: CustomerCollection, type: App\Entity\Customer, groups: [Default, Collection, Customer] } - - { alias: ProjectEntity, type: App\Entity\Project, groups: [Default, Entity, Project] } - - { alias: ProjectCollection, type: App\Entity\Project, groups: [Default, Collection, Project] } - - { alias: ActivityEntity, type: App\Entity\Activity, groups: [Default, Entity, Activity] } - - { alias: ActivityCollection, type: App\Entity\Activity, groups: [Default, Collection, Activity] } - - { alias: TimesheetFormEntity, type: App\Form\TimesheetEditForm, groups: [Default, Entity, Timesheet] } - - { alias: TimesheetEntity, type: App\Entity\Timesheet, groups: [Default, Entity, Timesheet] } - - { alias: TimesheetCollection, type: App\Entity\Timesheet, groups: [Default, Collection, Timesheet] } - - { alias: UserEntity, type: App\Entity\User, groups: [Default, Entity, User] } - - { alias: UserCollection, type: App\Entity\User, groups: [Default, Collection, User] } + - { alias: CustomerEntity, type: App\Entity\Customer, groups: [Default, Entity, Customer] } + - { alias: ProjectEntity, type: App\Entity\Project, groups: [Default, Entity, Project] } + - { alias: ActivityEntity, type: App\Entity\Activity, groups: [Default, Entity, Activity] } + - { alias: TimesheetEditForm, type: App\Form\TimesheetEditForm, groups: [Default, Entity, Timesheet] } + - { alias: TimesheetEntity, type: App\Entity\Timesheet, groups: [Default, Entity, Timesheet] } + - { alias: UserEntity, type: App\Entity\User, groups: [Default, Entity, User] } areas: path_patterns: - ^/api(?!/doc) diff --git a/config/serializer/App/Entity.Timesheet.yml b/config/serializer/App/Entity.Timesheet.yml index 9f880f21..6907c7fb 100644 --- a/config/serializer/App/Entity.Timesheet.yml +++ b/config/serializer/App/Entity.Timesheet.yml @@ -16,7 +16,7 @@ App\Entity\Timesheet: groups: [Default] description: include: true - groups: [Entity] + groups: [Default] rate: include: true groups: [Default] diff --git a/src/API/ActivityController.php b/src/API/ActivityController.php index 5ae95df8..600dec2d 100644 --- a/src/API/ActivityController.php +++ b/src/API/ActivityController.php @@ -53,7 +53,10 @@ class ActivityController extends BaseApiController * @SWG\Response( * response=200, * description="Returns the collection of all existing activities", - * @SWG\Schema(ref="#/definitions/ActivityCollection"), + * @SWG\Schema( + * type="array", + * @SWG\Items(ref="#/definitions/ActivityEntity") + * ) * ) * @Rest\QueryParam(name="project", requirements="\d+", strict=true, nullable=true, description="Project ID to filter activities. If none is provided, only global activities will be returned.") * @Rest\QueryParam(name="visible", requirements="\d+", strict=true, nullable=true, description="Visibility status to filter activities (1=visible, 2=hidden, 3=both)") diff --git a/src/API/CustomerController.php b/src/API/CustomerController.php index 6b7cacd2..aea059fa 100644 --- a/src/API/CustomerController.php +++ b/src/API/CustomerController.php @@ -53,7 +53,10 @@ class CustomerController extends BaseApiController * @SWG\Response( * response=200, * description="Returns the collection of all existing customer", - * @SWG\Schema(ref="#/definitions/CustomerCollection"), + * @SWG\Schema( + * type="array", + * @SWG\Items(ref="#/definitions/CustomerEntity") + * ) * ) * @Rest\QueryParam(name="visible", requirements="\d+", strict=true, nullable=true, description="Visibility status to filter activities (1=visible, 2=hidden, 3=both)") * @Rest\QueryParam(name="order", requirements="ASC|DESC", strict=true, nullable=true, description="The result order (allowed values: 'ASC', 'DESC')") diff --git a/src/API/ProjectController.php b/src/API/ProjectController.php index fd24d6bd..9b90fde0 100644 --- a/src/API/ProjectController.php +++ b/src/API/ProjectController.php @@ -53,7 +53,10 @@ class ProjectController extends BaseApiController * @SWG\Response( * response=200, * description="Returns the collection of all existing projects", - * @SWG\Schema(ref="#/definitions/ProjectCollection"), + * @SWG\Schema( + * type="array", + * @SWG\Items(ref="#/definitions/ProjectEntity") + * ) * ) * @Rest\QueryParam(name="customer", requirements="\d+", strict=true, nullable=true, description="Customer ID to filter projects") * @Rest\QueryParam(name="visible", requirements="\d+", strict=true, nullable=true, description="Visibility status to filter projects (1=visible, 2=hidden, 3=both)") diff --git a/src/API/TimesheetController.php b/src/API/TimesheetController.php index 80aa11bf..d12d8d74 100644 --- a/src/API/TimesheetController.php +++ b/src/API/TimesheetController.php @@ -70,8 +70,13 @@ class TimesheetController extends BaseApiController * @SWG\Response( * response=200, * description="Returns the collection of all existing timesheets for the user", - * @SWG\Schema(ref="#/definitions/TimesheetCollection"), + * @SWG\Schema( + * type="array", + * @SWG\Items(ref="#/definitions/TimesheetEntity") + * ) * ) + * + * @Rest\QueryParam(name="user", requirements="\d+|all", strict=true, nullable=true, description="User ID to filter timesheets (needs permission 'view_other_timesheet', pass 'all' to fetch data for all user)") * @Rest\QueryParam(name="customer", requirements="\d+", strict=true, nullable=true, description="Customer ID to filter timesheets") * @Rest\QueryParam(name="project", requirements="\d+", strict=true, nullable=true, description="Project ID to filter timesheets") * @Rest\QueryParam(name="activity", requirements="\d+", strict=true, nullable=true, description="Activity ID to filter timesheets") @@ -80,7 +85,7 @@ class TimesheetController extends BaseApiController * @Rest\QueryParam(name="order", requirements="ASC|DESC", strict=true, nullable=true, description="The result order (allowed values: 'ASC', 'DESC')") * @Rest\QueryParam(name="orderBy", requirements="id|begin|end|rate", strict=true, nullable=true, description="The field by which results will be ordered (allowed values: 'id', 'begin', 'end', 'rate')") * - * @Security("is_granted('view_own_timesheet')") + * @Security("is_granted('view_own_timesheet') or is_granted('view_other_timesheet')") * * @return Response */ @@ -90,6 +95,13 @@ class TimesheetController extends BaseApiController $query->setUser($this->getUser()); $query->setResultType(TimesheetQuery::RESULT_TYPE_PAGER); + if ($this->isGranted('view_other_timesheet') && null !== ($user = $paramFetcher->get('user'))) { + if ('all' === $user) { + $user = null; + } + $query->setUser($user); + } + if (null !== ($customer = $paramFetcher->get('customer'))) { $query->setCustomer($customer); } @@ -155,18 +167,26 @@ class TimesheetController extends BaseApiController /** * @SWG\Post( * description="Creates a new timesheet entry and returns it afterwards", - * @SWG\Schema(ref="#/definitions/TimesheetFormEntity"), * @SWG\Response( * response=200, * description="Returns the new created timesheet entry", * @SWG\Schema(ref="#/definitions/TimesheetEntity"), * ) * ) + * @SWG\Parameter( + * name="body", + * in="body", + * required=true, + * @SWG\Schema(ref="#/definitions/TimesheetEditForm") + * ) * * @Security("is_granted('create_own_timesheet')") * * @param Request $request * @return Response + * @throws \App\Repository\RepositoryException + * @throws \Doctrine\ORM\ORMException + * @throws \Doctrine\ORM\OptimisticLockException */ public function postAction(Request $request) { @@ -180,7 +200,6 @@ class TimesheetController extends BaseApiController 'include_exported' => $this->isGranted('edit_export', $timesheet), ]); - $form->setData($timesheet); $form->submit($request->request->all()); if ($form->isValid()) { @@ -227,15 +246,20 @@ class TimesheetController extends BaseApiController } /** - * @SWG\Post( + * @SWG\Patch( * description="Update an existing timesheet entry, you can pass all or just a subset of all attributes", - * @SWG\Schema(ref="#/definitions/TimesheetFormEntity"), * @SWG\Response( * response=200, * description="Returns the updated timesheet entry", - * @SWG\Schema(ref="#/definitions/TimesheetEntity"), + * @SWG\Schema(ref="#/definitions/TimesheetEntity") * ) * ) + * @SWG\Parameter( + * name="body", + * in="body", + * required=true, + * @SWG\Schema(ref="#/definitions/TimesheetEditForm") + * ) * * @param Request $request * @param string $id diff --git a/src/API/UserController.php b/src/API/UserController.php index b3b93b36..ff6c7e74 100644 --- a/src/API/UserController.php +++ b/src/API/UserController.php @@ -49,7 +49,10 @@ class UserController extends BaseApiController * @SWG\Response( * response=200, * description="Returns the collection of all registered users", - * @SWG\Schema(ref="#/definitions/UserCollection"), + * @SWG\Schema( + * type="array", + * @SWG\Items(ref="#/definitions/UserEntity") + * ) * ) * * @Security("is_granted('view_user')") diff --git a/src/Controller/Admin/TimesheetController.php b/src/Controller/Admin/TimesheetController.php index 90716cca..a0184935 100644 --- a/src/Controller/Admin/TimesheetController.php +++ b/src/Controller/Admin/TimesheetController.php @@ -13,7 +13,7 @@ use App\Controller\AbstractController; use App\Controller\TimesheetControllerTrait; use App\Entity\Timesheet; use App\Form\TimesheetEditForm; -use App\Form\Toolbar\TimesheetAdminToolbarForm; +use App\Form\Toolbar\TimesheetToolbarForm; use App\Repository\Query\TimesheetQuery; use Pagerfanta\Pagerfanta; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; @@ -206,11 +206,12 @@ class TimesheetController extends AbstractController */ protected function getToolbarForm(TimesheetQuery $query) { - return $this->createForm(TimesheetAdminToolbarForm::class, $query, [ + return $this->createForm(TimesheetToolbarForm::class, $query, [ 'action' => $this->generateUrl('admin_timesheet', [ 'page' => $query->getPage(), ]), 'method' => 'GET', + 'include_user' => true, ]); } } diff --git a/src/Form/Toolbar/TimesheetAdminToolbarForm.php b/src/Form/Toolbar/TimesheetAdminToolbarForm.php deleted file mode 100644 index e98c3cd2..00000000 --- a/src/Form/Toolbar/TimesheetAdminToolbarForm.php +++ /dev/null @@ -1,33 +0,0 @@ -addTimesheetStateChoice($builder); - $this->addPageSizeChoice($builder); - $this->addUserChoice($builder); - $this->addDateRangeChoice($builder); - $this->addCustomerChoice($builder); - $this->addProjectChoice($builder); - $this->addActivityChoice($builder); - $this->addHiddenPagination($builder); - } -} diff --git a/src/Form/Toolbar/TimesheetToolbarForm.php b/src/Form/Toolbar/TimesheetToolbarForm.php index 3d6cdfd7..563a5221 100644 --- a/src/Form/Toolbar/TimesheetToolbarForm.php +++ b/src/Form/Toolbar/TimesheetToolbarForm.php @@ -26,6 +26,9 @@ class TimesheetToolbarForm extends AbstractToolbarForm { $this->addTimesheetStateChoice($builder); $this->addPageSizeChoice($builder); + if ($options['include_user']) { + $this->addUserChoice($builder); + } $this->addDateRangeChoice($builder); $this->addCustomerChoice($builder); $this->addProjectChoice($builder); @@ -58,6 +61,7 @@ class TimesheetToolbarForm extends AbstractToolbarForm $resolver->setDefaults([ 'data_class' => TimesheetQuery::class, 'csrf_protection' => false, + 'include_user' => false, ]); } } diff --git a/src/Repository/Query/TimesheetQuery.php b/src/Repository/Query/TimesheetQuery.php index b6067017..a1958e45 100644 --- a/src/Repository/Query/TimesheetQuery.php +++ b/src/Repository/Query/TimesheetQuery.php @@ -69,10 +69,10 @@ class TimesheetQuery extends ActivityQuery } /** - * @param User $user + * @param User|int $user * @return TimesheetQuery */ - public function setUser(User $user = null) + public function setUser($user = null) { $this->user = $user; diff --git a/tests/API/TimesheetControllerTest.php b/tests/API/TimesheetControllerTest.php index a2aa997e..912e6387 100644 --- a/tests/API/TimesheetControllerTest.php +++ b/tests/API/TimesheetControllerTest.php @@ -33,6 +33,7 @@ class TimesheetControllerTest extends APIControllerBaseTest ->setAmount(10) ->setUser($this->getUserByRole($em, User::ROLE_USER)) ->setStartDate(new \DateTime('-10 days')) + ->setAllowEmptyDescriptions(false) ; $this->importFixture($em, $fixture); } @@ -48,12 +49,73 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertAccessIsGranted($client, '/api/timesheets'); $result = json_decode($client->getResponse()->getContent(), true); - $this->assertInternalType('array', $result); + $this->assertIsArray($result); $this->assertNotEmpty($result); $this->assertEquals(10, count($result)); $this->assertDefaultStructure($result[0], false); } + public function testGetCollectionForOtherUser() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + + $fixture = new TimesheetFixtures(); + $fixture + ->setFixedRate(true) + ->setHourlyRate(true) + ->setAmount(7) + ->setUser($this->getUserByRole($em, User::ROLE_ADMIN)) + ->setStartDate(new \DateTime('-10 days')) + ; + $this->importFixture($em, $fixture); + + $query = ['user' => 2]; + $this->assertAccessIsGranted($client, '/api/timesheets', 'GET', $query); + $result = json_decode($client->getResponse()->getContent(), true); + + $this->assertIsArray($result); + $this->assertNotEmpty($result); + $this->assertEquals(10, count($result)); + $this->assertDefaultStructure($result[0], false); + } + + public function testGetCollectionForAllUser() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + $em = $client->getContainer()->get('doctrine.orm.entity_manager'); + + $fixture = new TimesheetFixtures(); + $fixture + ->setFixedRate(true) + ->setHourlyRate(true) + ->setAmount(7) + ->setUser($this->getUserByRole($em, User::ROLE_ADMIN)) + ->setStartDate(new \DateTime('-10 days')) + ; + $this->importFixture($em, $fixture); + + $query = ['user' => 'all']; + $this->assertAccessIsGranted($client, '/api/timesheets', 'GET', $query); + $result = json_decode($client->getResponse()->getContent(), true); + + $this->assertIsArray($result); + $this->assertNotEmpty($result); + $this->assertEquals(17, count($result)); + $this->assertDefaultStructure($result[0], false); + } + + public function testGetCollectionForEmptyResult() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + + $this->assertAccessIsGranted($client, '/api/timesheets'); + $result = json_decode($client->getResponse()->getContent(), true); + + $this->assertIsArray($result); + $this->assertEmpty($result); + } + public function testGetCollectionWithQuery() { $query = ['customer' => 1, 'project' => 1, 'page' => 2, 'size' => 5, 'order' => 'DESC', 'orderBy' => 'rate']; @@ -61,7 +123,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertAccessIsGranted($client, '/api/timesheets', 'GET', $query); $result = json_decode($client->getResponse()->getContent(), true); - $this->assertInternalType('array', $result); + $this->assertIsArray($result); $this->assertNotEmpty($result); $this->assertEquals(5, count($result)); $this->assertDefaultStructure($result[0], false); @@ -73,7 +135,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertAccessIsGranted($client, '/api/timesheets/1'); $result = json_decode($client->getResponse()->getContent(), true); - $this->assertInternalType('array', $result); + $this->assertIsArray($result); $this->assertDefaultStructure($result); } @@ -93,7 +155,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertTrue($client->getResponse()->isSuccessful()); $result = json_decode($client->getResponse()->getContent(), true); - $this->assertInternalType('array', $result); + $this->assertIsArray($result); $this->assertDefaultStructure($result); $this->assertNotEmpty($result['id']); $this->assertEquals(28800, $result['duration']); @@ -194,7 +256,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertTrue($client->getResponse()->isSuccessful()); $result = json_decode($client->getResponse()->getContent(), true); - $this->assertInternalType('array', $result); + $this->assertIsArray($result); $this->assertDefaultStructure($result); $this->assertNotEmpty($result['id']); $this->assertEquals(25200, $result['duration']); @@ -221,12 +283,12 @@ class TimesheetControllerTest extends APIControllerBaseTest protected function assertDefaultStructure(array $result, $full = true) { $expectedKeys = [ - 'id', 'begin', 'end', 'duration', 'rate', 'activity', 'project', 'user' + 'id', 'begin', 'end', 'duration', 'description', 'rate', 'activity', 'project', 'user' ]; if ($full) { $expectedKeys = array_merge($expectedKeys, [ - 'exported', 'description', 'fixed_rate', 'hourly_rate' + 'exported', 'fixed_rate', 'hourly_rate' ]); } diff --git a/tests/DataFixtures/TimesheetFixtures.php b/tests/DataFixtures/TimesheetFixtures.php index c2415f42..e2fb7c59 100644 --- a/tests/DataFixtures/TimesheetFixtures.php +++ b/tests/DataFixtures/TimesheetFixtures.php @@ -51,6 +51,20 @@ class TimesheetFixtures extends Fixture * @var bool */ protected $hourlyRate = false; + /** + * @var bool + */ + protected $allowEmptyDescriptions = true; + + /** + * @param bool $allowEmptyDescriptions + * @return TimesheetFixtures + */ + public function setAllowEmptyDescriptions(bool $allowEmptyDescriptions) + { + $this->allowEmptyDescriptions = $allowEmptyDescriptions; + return $this; + } /** * @param bool $fixedRate @@ -148,11 +162,13 @@ class TimesheetFixtures extends Fixture $user = $this->user; for ($i = 0; $i < $this->amount; $i++) { - $description = null; - if ($i % 3 == 0) { - $description = $faker->text; - } elseif ($i % 2 == 0) { - $description = ''; + $description = $faker->text; + if ($this->allowEmptyDescriptions) { + if ($i % 3 == 0) { + $description = null; + } elseif ($i % 2 == 0) { + $description = ''; + } } $activity = $activities[array_rand($activities)];