diff --git a/src/API/TimesheetController.php b/src/API/TimesheetController.php index 55b13e25..91f6ff0a 100644 --- a/src/API/TimesheetController.php +++ b/src/API/TimesheetController.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace App\API; +use App\Entity\Timesheet; use App\Entity\User; use App\Event\RecentActivityEvent; use App\Event\TimesheetDuplicatePostEvent; @@ -36,7 +37,6 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Swagger\Annotations as SWG; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\Validator\Constraints; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -280,20 +280,13 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('view', id)") */ - public function getAction(int $id): Response + public function getAction(Timesheet $id): Response { - $data = $this->repository->find($id); - - if (null === $data) { - throw new NotFoundException(); - } - - if (!$this->isGranted('view', $data)) { - throw new AccessDeniedHttpException('You are not allowed to view this timesheet'); - } - - $view = new View($data, 200); + $timesheet = $id; // cannot be changed due to BC reasons, routes use 'id' + $view = new View($timesheet, 200); $view->getContext()->setGroups(self::GROUPS_ENTITY); return $this->viewHandler->handle($view); @@ -384,34 +377,27 @@ class TimesheetController extends BaseApiController * ) * ) * @SWG\Parameter( - * name="body", - * in="body", - * required=true, - * @SWG\Schema(ref="#/definitions/TimesheetEditForm") - * ) - * @SWG\Parameter( * name="id", * in="path", * type="integer", * description="Timesheet record ID to update", * required=true, * ) + * @SWG\Parameter( + * name="body", + * in="body", + * required=true, + * @SWG\Schema(ref="#/definitions/TimesheetEditForm") + * ) * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('edit', id)") */ - public function patchAction(Request $request, int $id): Response + public function patchAction(Request $request, Timesheet $id): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('edit', $timesheet)) { - throw new AccessDeniedHttpException('You are not allowed to update this timesheet'); - } - + $timesheet = $id; $event = new TimesheetMetaDefinitionEvent($timesheet); $this->dispatcher->dispatch($event); @@ -463,20 +449,12 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('delete', id)") */ - public function deleteAction(int $id): Response + public function deleteAction(Timesheet $id): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('delete', $timesheet)) { - throw $this->createAccessDeniedException('You are not allowed to delete this timesheet'); - } - - $this->service->deleteTimesheet($timesheet); + $this->service->deleteTimesheet($id); $view = new View(null, Response::HTTP_NO_CONTENT); @@ -585,22 +563,14 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('stop', id)") */ - public function stopAction(int $id): Response + public function stopAction(Timesheet $id): Response { - $timesheet = $this->repository->find($id); + $this->service->stopTimesheet($id); - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('stop', $timesheet)) { - throw new AccessDeniedHttpException('You are not allowed to stop this timesheet'); - } - - $this->service->stopTimesheet($timesheet); - - $view = new View($timesheet, 200); + $view = new View($id, 200); $view->getContext()->setGroups(self::GROUPS_ENTITY); return $this->viewHandler->handle($view); @@ -627,19 +597,12 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('start', id)") */ - public function restartAction(int $id, ParamFetcherInterface $paramFetcher): Response + public function restartAction(Timesheet $id, ParamFetcherInterface $paramFetcher): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('start', $timesheet)) { - throw new AccessDeniedHttpException('You are not allowed to re-start this timesheet'); - } - + $timesheet = $id; /** @var User $user */ $user = $this->getUser(); @@ -714,25 +677,16 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('duplicate', id)") */ - public function duplicateAction(int $id): Response + public function duplicateAction(Timesheet $id): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('duplicate', $timesheet)) { - throw new AccessDeniedHttpException('You are not allowed to duplicate this timesheet'); - } - + $timesheet = $id; $copyTimesheet = clone $timesheet; $this->dispatcher->dispatch(new TimesheetDuplicatePreEvent($copyTimesheet, $timesheet)); - $this->service->saveNewTimesheet($copyTimesheet); - $this->dispatcher->dispatch(new TimesheetDuplicatePostEvent($copyTimesheet, $timesheet)); $view = new View($copyTimesheet, 200); @@ -759,21 +713,12 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('edit_export', id)") */ - public function exportAction(int $id): Response + public function exportAction(Timesheet $id): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('edit_export', $timesheet)) { - throw new AccessDeniedHttpException( - sprintf('You are not allowed to %s this timesheet', ($timesheet->isExported() ? 'unlock' : 'lock')) - ); - } - + $timesheet = $id; $timesheet->setExported(!$timesheet->isExported()); $this->service->updateTimesheet($timesheet); @@ -804,19 +749,12 @@ class TimesheetController extends BaseApiController * * @ApiSecurity(name="apiUser") * @ApiSecurity(name="apiToken") + * + * @Security("is_granted('edit', id)") */ - public function metaAction(int $id, ParamFetcherInterface $paramFetcher): Response + public function metaAction(Timesheet $id, ParamFetcherInterface $paramFetcher): Response { - $timesheet = $this->repository->find($id); - - if (null === $timesheet) { - throw new NotFoundException(); - } - - if (!$this->isGranted('edit', $timesheet)) { - throw new AccessDeniedHttpException('You are not allowed to update this timesheet'); - } - + $timesheet = $id; $event = new TimesheetMetaDefinitionEvent($timesheet); $this->dispatcher->dispatch($event); diff --git a/src/Controller/TimesheetAbstractController.php b/src/Controller/TimesheetAbstractController.php index 23f397f8..af67bc89 100644 --- a/src/Controller/TimesheetAbstractController.php +++ b/src/Controller/TimesheetAbstractController.php @@ -13,6 +13,8 @@ use App\Configuration\SystemConfiguration; use App\Entity\MetaTableTypeInterface; use App\Entity\Tag; use App\Entity\Timesheet; +use App\Event\TimesheetDuplicatePostEvent; +use App\Event\TimesheetDuplicatePreEvent; use App\Event\TimesheetMetaDefinitionEvent; use App\Event\TimesheetMetaDisplayEvent; use App\Export\ServiceExport; @@ -208,6 +210,32 @@ abstract class TimesheetAbstractController extends AbstractController ]); } + protected function duplicate(Timesheet $timesheet, Request $request, string $renderTemplate): Response + { + $copyTimesheet = clone $timesheet; + + $form = $this->getDuplicateForm($timesheet); + $form->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + try { + $this->dispatcher->dispatch(new TimesheetDuplicatePreEvent($copyTimesheet, $timesheet)); + $this->service->saveNewTimesheet($copyTimesheet); + $this->dispatcher->dispatch(new TimesheetDuplicatePostEvent($copyTimesheet, $timesheet)); + $this->flashSuccess('action.update.success'); + + return $this->redirectToRoute($this->getTimesheetRoute()); + } catch (\Exception $ex) { + $this->flashUpdateException($ex); + } + } + + return $this->render($renderTemplate, [ + 'timesheet' => $copyTimesheet, + 'form' => $form->createView(), + ]); + } + protected function export(Request $request, string $exporterId): Response { $query = new TimesheetQuery(); @@ -425,12 +453,12 @@ abstract class TimesheetAbstractController extends AbstractController ]); } - protected function getCreateForm(Timesheet $entry): FormInterface + protected function generateCreateForm(Timesheet $entry, string $formClass, string $action): FormInterface { $mode = $this->getTrackingMode(); - return $this->createForm($this->getCreateFormClassName(), $entry, [ - 'action' => $this->generateUrl($this->getCreateRoute()), + return $this->createForm($formClass, $entry, [ + 'action' => $action, 'include_rate' => $this->isGranted('edit_rate', $entry), 'include_exported' => $this->isGranted('edit_export', $entry), 'include_user' => $this->includeUserInForms('create'), @@ -495,11 +523,6 @@ abstract class TimesheetAbstractController extends AbstractController return 'edit_rate_own_timesheet'; } - protected function getCreateFormClassName(): string - { - return TimesheetEditForm::class; - } - protected function getEditFormClassName(): string { return TimesheetEditForm::class; @@ -525,11 +548,6 @@ abstract class TimesheetAbstractController extends AbstractController return 'timesheet_edit'; } - protected function getCreateRoute(): string - { - return 'timesheet_create'; - } - protected function getMultiUpdateRoute(): string { return 'timesheet_multi_update'; @@ -544,4 +562,8 @@ abstract class TimesheetAbstractController extends AbstractController { return $this->getTrackingMode()->canSeeBeginAndEndTimes(); } + + abstract protected function getDuplicateForm(Timesheet $entry): FormInterface; + + abstract protected function getCreateForm(Timesheet $entry): FormInterface; } diff --git a/src/Controller/TimesheetController.php b/src/Controller/TimesheetController.php index 6dd4e474..d767d065 100644 --- a/src/Controller/TimesheetController.php +++ b/src/Controller/TimesheetController.php @@ -11,11 +11,13 @@ namespace App\Controller; use App\Entity\Timesheet; use App\Event\TimesheetMetaDisplayEvent; +use App\Form\TimesheetEditForm; use App\Repository\ActivityRepository; use App\Repository\ProjectRepository; use App\Repository\Query\TimesheetQuery; use App\Repository\TagRepository; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; +use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -30,12 +32,8 @@ class TimesheetController extends TimesheetAbstractController * @Route(path="/", defaults={"page": 1}, name="timesheet", methods={"GET"}) * @Route(path="/page/{page}", requirements={"page": "[1-9]\d*"}, name="timesheet_paginated", methods={"GET"}) * @Security("is_granted('view_own_timesheet')") - * - * @param int $page - * @param Request $request - * @return Response */ - public function indexAction($page, Request $request) + public function indexAction(int $page, Request $request): Response { $query = new TimesheetQuery(); $query->setPage($page); @@ -47,12 +45,8 @@ class TimesheetController extends TimesheetAbstractController /** * @Route(path="/export/{exporter}", name="timesheet_export", methods={"GET"}) * @Security("is_granted('export_own_timesheet')") - * - * @param Request $request - * @param string $exporter - * @return Response */ - public function exportAction(Request $request, string $exporter) + public function exportAction(Request $request, string $exporter): Response { return $this->export($request, $exporter); } @@ -60,21 +54,26 @@ class TimesheetController extends TimesheetAbstractController /** * @Route(path="/{id}/edit", name="timesheet_edit", methods={"GET", "POST"}) * @Security("is_granted('edit', entry)") - * - * @param Timesheet $entry - * @param Request $request - * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response */ - public function editAction(Timesheet $entry, Request $request) + public function editAction(Timesheet $entry, Request $request): Response { return $this->edit($entry, $request, 'timesheet/edit.html.twig'); } + /** + * @Route(path="/{id}/duplicate", name="timesheet_duplicate", methods={"GET", "POST"}) + * @Security("is_granted('duplicate', entry)") + */ + public function duplicateAction(Timesheet $entry, Request $request): Response + { + return $this->duplicate($entry, $request, 'timesheet/edit.html.twig'); + } + /** * @Route(path="/multi-update", name="timesheet_multi_update", methods={"POST"}) * @Security("is_granted('edit_own_timesheet')") */ - public function multiUpdateAction(Request $request) + public function multiUpdateAction(Request $request): Response { return $this->multiUpdate($request, 'timesheet/multi-update.html.twig'); } @@ -83,7 +82,7 @@ class TimesheetController extends TimesheetAbstractController * @Route(path="/multi-delete", name="timesheet_multi_delete", methods={"POST"}) * @Security("is_granted('delete_own_timesheet')") */ - public function multiDeleteAction(Request $request) + public function multiDeleteAction(Request $request): Response { return $this->multiDelete($request); } @@ -91,14 +90,19 @@ class TimesheetController extends TimesheetAbstractController /** * @Route(path="/create", name="timesheet_create", methods={"GET", "POST"}) * @Security("is_granted('create_own_timesheet')") - * - * @param Request $request - * @param ProjectRepository $projectRepository - * @param ActivityRepository $activityRepository - * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response */ - public function createAction(Request $request, ProjectRepository $projectRepository, ActivityRepository $activityRepository, TagRepository $tagRepository) + public function createAction(Request $request, ProjectRepository $projectRepository, ActivityRepository $activityRepository, TagRepository $tagRepository): Response { return $this->create($request, 'timesheet/edit.html.twig', $projectRepository, $activityRepository, $tagRepository); } + + protected function getCreateForm(Timesheet $entry): FormInterface + { + return $this->generateCreateForm($entry, TimesheetEditForm::class, $this->generateUrl('timesheet_create')); + } + + protected function getDuplicateForm(Timesheet $entry): FormInterface + { + return $this->generateCreateForm($entry, TimesheetEditForm::class, $this->generateUrl('timesheet_duplicate', ['id' => $entry->getId()])); + } } diff --git a/src/Controller/TimesheetTeamController.php b/src/Controller/TimesheetTeamController.php index ce086ac4..03634da1 100644 --- a/src/Controller/TimesheetTeamController.php +++ b/src/Controller/TimesheetTeamController.php @@ -23,7 +23,6 @@ use App\Repository\TagRepository; use Doctrine\Common\Collections\ArrayCollection; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Component\Form\FormInterface; -use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -43,7 +42,7 @@ class TimesheetTeamController extends TimesheetAbstractController * @param Request $request * @return Response */ - public function indexAction($page, Request $request) + public function indexAction(int $page, Request $request): Response { $query = new TimesheetQuery(); $query->setPage($page); @@ -54,12 +53,8 @@ class TimesheetTeamController extends TimesheetAbstractController /** * @Route(path="/export/{exporter}", name="admin_timesheet_export", methods={"GET"}) - * - * @param Request $request - * @param string $exporter - * @return Response */ - public function exportAction(Request $request, string $exporter) + public function exportAction(Request $request, string $exporter): Response { return $this->export($request, $exporter); } @@ -67,26 +62,26 @@ class TimesheetTeamController extends TimesheetAbstractController /** * @Route(path="/{id}/edit", name="admin_timesheet_edit", methods={"GET", "POST"}) * @Security("is_granted('edit', entry)") - * - * @param Timesheet $entry - * @param Request $request - * @return RedirectResponse|Response */ - public function editAction(Timesheet $entry, Request $request) + public function editAction(Timesheet $entry, Request $request): Response { return $this->edit($entry, $request, 'timesheet-team/edit.html.twig'); } + /** + * @Route(path="/{id}/duplicate", name="admin_timesheet_duplicate", methods={"GET", "POST"}) + * @Security("is_granted('duplicate', entry)") + */ + public function duplicateAction(Timesheet $entry, Request $request): Response + { + return $this->duplicate($entry, $request, 'timesheet-team/edit.html.twig'); + } + /** * @Route(path="/create", name="admin_timesheet_create", methods={"GET", "POST"}) * @Security("is_granted('create_other_timesheet')") - * - * @param Request $request - * @param ProjectRepository $projectRepository - * @param ActivityRepository $activityRepository - * @return RedirectResponse|Response */ - public function createAction(Request $request, ProjectRepository $projectRepository, ActivityRepository $activityRepository, TagRepository $tagRepository) + public function createAction(Request $request, ProjectRepository $projectRepository, ActivityRepository $activityRepository, TagRepository $tagRepository): Response { return $this->create($request, 'timesheet-team/edit.html.twig', $projectRepository, $activityRepository, $tagRepository); } @@ -94,11 +89,8 @@ class TimesheetTeamController extends TimesheetAbstractController /** * @Route(path="/create_mu", name="admin_timesheet_create_multiuser", methods={"GET", "POST"}) * @Security("is_granted('create_other_timesheet')") - * - * @param Request $request - * @return RedirectResponse|Response */ - public function createForMultiUserAction(Request $request) + public function createForMultiUserAction(Request $request): Response { $entry = new MultiUserTimesheet(); $entry->setUser($this->getUser()); @@ -176,7 +168,7 @@ class TimesheetTeamController extends TimesheetAbstractController * @Route(path="/multi-update", name="admin_timesheet_multi_update", methods={"POST"}) * @Security("is_granted('edit_other_timesheet')") */ - public function multiUpdateAction(Request $request) + public function multiUpdateAction(Request $request): Response { return $this->multiUpdate($request, 'timesheet-team/multi-update.html.twig'); } @@ -185,7 +177,7 @@ class TimesheetTeamController extends TimesheetAbstractController * @Route(path="/multi-delete", name="admin_timesheet_multi_delete", methods={"POST"}) * @Security("is_granted('delete_other_timesheet')") */ - public function multiDeleteAction(Request $request) + public function multiDeleteAction(Request $request): Response { return $this->multiDelete($request); } @@ -195,6 +187,16 @@ class TimesheetTeamController extends TimesheetAbstractController $query->setCurrentUser($this->getUser()); } + protected function getCreateForm(Timesheet $entry): FormInterface + { + return $this->generateCreateForm($entry, TimesheetAdminEditForm::class, $this->generateUrl('admin_timesheet_create')); + } + + protected function getDuplicateForm(Timesheet $entry): FormInterface + { + return $this->generateCreateForm($entry, TimesheetAdminEditForm::class, $this->generateUrl('admin_timesheet_duplicate', ['id' => $entry->getId()])); + } + protected function getPermissionEditExport(): string { return 'edit_export_other_timesheet'; @@ -205,11 +207,6 @@ class TimesheetTeamController extends TimesheetAbstractController return 'edit_rate_other_timesheet'; } - protected function getCreateFormClassName(): string - { - return TimesheetAdminEditForm::class; - } - protected function getEditFormClassName(): string { return TimesheetAdminEditForm::class; @@ -234,11 +231,6 @@ class TimesheetTeamController extends TimesheetAbstractController return 'admin_timesheet_edit'; } - protected function getCreateRoute(): string - { - return 'admin_timesheet_create'; - } - protected function getMultiUpdateRoute(): string { return 'admin_timesheet_multi_update'; diff --git a/src/EventSubscriber/Actions/AbstractTimesheetSubscriber.php b/src/EventSubscriber/Actions/AbstractTimesheetSubscriber.php index 96ffad34..fa9ea1e9 100644 --- a/src/EventSubscriber/Actions/AbstractTimesheetSubscriber.php +++ b/src/EventSubscriber/Actions/AbstractTimesheetSubscriber.php @@ -12,9 +12,12 @@ namespace App\EventSubscriber\Actions; use App\Entity\Timesheet; use App\Event\PageActionsEvent; +/** + * @internal + */ abstract class AbstractTimesheetSubscriber extends AbstractActionsSubscriber { - protected function timesheetActions(PageActionsEvent $event, string $routeListing, string $routeEdit): void + protected function timesheetActions(PageActionsEvent $event, string $routeEdit, string $routeDuplicate): void { $payload = $event->getPayload(); @@ -35,7 +38,8 @@ abstract class AbstractTimesheetSubscriber extends AbstractActionsSubscriber } if ($this->isGranted('duplicate', $timesheet)) { - $event->addAction('copy', ['url' => $this->path('duplicate_timesheet', ['id' => $timesheet->getId()]), 'class' => 'api-link', 'attr' => ['data-payload' => '{"copy": "all"}', 'data-event' => 'kimai.timesheetStart kimai.timesheetUpdate', 'data-method' => 'PATCH', 'data-msg-error' => 'action.update.error', 'data-msg-success' => 'action.update.success']]); + $class = $event->isView('edit') ? '' : 'modal-ajax-form'; + $event->addAction('copy', ['url' => $this->path($routeDuplicate, ['id' => $timesheet->getId()]), 'class' => $class]); } if ($event->countActions() > 0) { diff --git a/src/EventSubscriber/Actions/TimesheetSubscriber.php b/src/EventSubscriber/Actions/TimesheetSubscriber.php index 69c667db..b3fad5c2 100644 --- a/src/EventSubscriber/Actions/TimesheetSubscriber.php +++ b/src/EventSubscriber/Actions/TimesheetSubscriber.php @@ -20,6 +20,6 @@ class TimesheetSubscriber extends AbstractTimesheetSubscriber public function onActions(PageActionsEvent $event): void { - $this->timesheetActions($event, 'timesheet', 'timesheet_edit'); + $this->timesheetActions($event, 'timesheet_edit', 'timesheet_duplicate'); } } diff --git a/src/EventSubscriber/Actions/TimesheetTeamSubscriber.php b/src/EventSubscriber/Actions/TimesheetTeamSubscriber.php index 9f510c4a..ced6cb47 100644 --- a/src/EventSubscriber/Actions/TimesheetTeamSubscriber.php +++ b/src/EventSubscriber/Actions/TimesheetTeamSubscriber.php @@ -20,6 +20,6 @@ class TimesheetTeamSubscriber extends AbstractTimesheetSubscriber public function onActions(PageActionsEvent $event): void { - $this->timesheetActions($event, 'admin_timesheet', 'admin_timesheet_edit'); + $this->timesheetActions($event, 'admin_timesheet_edit', 'admin_timesheet_duplicate'); } } diff --git a/src/Form/Type/SystemConfigurationType.php b/src/Form/Type/SystemConfigurationType.php index 1111f35b..c9b7c02a 100644 --- a/src/Form/Type/SystemConfigurationType.php +++ b/src/Form/Type/SystemConfigurationType.php @@ -45,7 +45,7 @@ class SystemConfigurationType extends AbstractType } $required = $preference->isRequired(); - if (CheckboxType::class == $preference->getType()) { + if (CheckboxType::class == $preference->getType() || YesNoType::class == $preference->getType()) { $required = false; } diff --git a/src/Voter/TimesheetVoter.php b/src/Voter/TimesheetVoter.php index fac6ee6e..0c3b12c6 100644 --- a/src/Voter/TimesheetVoter.php +++ b/src/Voter/TimesheetVoter.php @@ -9,7 +9,6 @@ namespace App\Voter; -use App\Configuration\SystemConfiguration; use App\Entity\Timesheet; use App\Entity\User; use App\Security\RolePermissionManager; @@ -48,9 +47,6 @@ final class TimesheetVoter extends Voter 'duplicate' ]; - private $configuration; - private $cacheAllowCopy; - private $permissionManager; private $lockdownService; @@ -59,11 +55,10 @@ final class TimesheetVoter extends Voter private $editExported; private $now; - public function __construct(RolePermissionManager $permissionManager, LockdownService $lockdownService, SystemConfiguration $configuration) + public function __construct(RolePermissionManager $permissionManager, LockdownService $lockdownService) { $this->permissionManager = $permissionManager; $this->lockdownService = $lockdownService; - $this->configuration = $configuration; } /** @@ -209,17 +204,6 @@ final class TimesheetVoter extends Voter protected function canDuplicate(User $user, Timesheet $timesheet): bool { - if ($this->cacheAllowCopy === null) { - $this->cacheAllowCopy = $this->configuration->isTimesheetAllowOverlappingRecords(); - } - - // This is a quickfix, because the API cannot open dialogs. if the method is copied to the timesheet controller, - // we could open the edit dialog instead of directly saving the copied entry. - // Probably add a new permission is required to differentiate between API and UI. - if (!$this->cacheAllowCopy) { - return false; - } - if (!$this->isAllowedInLockdown($user, $timesheet)) { return false; } diff --git a/templates/customer/index.html.twig b/templates/customer/index.html.twig index 410c46b1..2e5ac706 100644 --- a/templates/customer/index.html.twig +++ b/templates/customer/index.html.twig @@ -79,7 +79,7 @@ {% endif %} - {% if entry.hasBudget() %} + {% if entry.hasTimeBudget() %} {{ entry.timeBudget|duration }} {% else %} – diff --git a/tests/API/APIControllerBaseTest.php b/tests/API/APIControllerBaseTest.php index a583fb2f..cb7fc1f1 100644 --- a/tests/API/APIControllerBaseTest.php +++ b/tests/API/APIControllerBaseTest.php @@ -120,13 +120,13 @@ abstract class APIControllerBaseTest extends ControllerBaseTest return $client->request($method, $this->createUrl($url), $parameters, [], $server, $content); } - protected function assertEntityNotFound(string $role, string $url, string $method = 'GET') + protected function assertEntityNotFound(string $role, string $url, string $method = 'GET', ?string $message = null) { $client = $this->getClientForAuthenticatedUser($role); $this->request($client, $url, $method); $this->assertApiException($client->getResponse(), [ 'code' => 404, - 'message' => 'Not found' + 'message' => $message ?? 'Not found' ]); } @@ -138,19 +138,19 @@ abstract class APIControllerBaseTest extends ControllerBaseTest ]); } - protected function assertEntityNotFoundForDelete(string $role, string $url) + protected function assertEntityNotFoundForDelete(string $role, string $url, ?string $message = null) { $this->assertExceptionForDeleteAction($role, $url, [], [ 'code' => 404, - 'message' => 'Not found' + 'message' => $message ?? 'Not found' ]); } - protected function assertEntityNotFoundForPatch(string $role, string $url, array $data) + protected function assertEntityNotFoundForPatch(string $role, string $url, array $data, ?string $message = null) { $this->assertExceptionForPatchAction($role, $url, $data, [ 'code' => 404, - 'message' => 'Not found' + 'message' => $message ?? 'Not found', ]); } @@ -158,7 +158,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest { $this->assertExceptionForPostAction($role, $url, $data, [ 'code' => 404, - 'message' => $message ?? 'Not found' + 'message' => $message ?? 'Not found', ]); } diff --git a/tests/API/TimesheetControllerTest.php b/tests/API/TimesheetControllerTest.php index 92a73f0d..d254c10e 100644 --- a/tests/API/TimesheetControllerTest.php +++ b/tests/API/TimesheetControllerTest.php @@ -363,7 +363,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $timesheets = $this->importFixtureForUser(User::ROLE_ADMIN); $this->assertCount(10, $timesheets); - $this->assertApiAccessDenied($client, '/api/timesheets/' . $timesheets[0]->getId(), 'You are not allowed to view this timesheet'); + $this->assertApiAccessDenied($client, '/api/timesheets/' . $timesheets[0]->getId(), 'Access denied.'); } public function testGetEntityAccessAllowedForAdmin() @@ -379,7 +379,7 @@ class TimesheetControllerTest extends APIControllerBaseTest public function testGetEntityNotFound() { - $this->assertEntityNotFound(User::ROLE_USER, '/api/timesheets/' . PHP_INT_MAX); + $this->assertEntityNotFound(User::ROLE_USER, '/api/timesheets/' . PHP_INT_MAX, 'GET', 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testPostAction() @@ -569,12 +569,12 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertFalse($response->isSuccessful()); $this->assertEquals(Response::HTTP_FORBIDDEN, $response->getStatusCode()); $json = json_decode($response->getContent(), true); - $this->assertEquals('You are not allowed to update this timesheet', $json['message']); + $this->assertEquals('Access denied.', $json['message']); } public function testPatchActionWithUnknownTimesheet() { - $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/255', []); + $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/255', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testInvalidPatchAction() @@ -618,7 +618,7 @@ class TimesheetControllerTest extends APIControllerBaseTest public function testDeleteActionWithUnknownTimesheet() { - $this->assertEntityNotFoundForDelete(User::ROLE_ADMIN, '/api/timesheets/255'); + $this->assertEntityNotFoundForDelete(User::ROLE_ADMIN, '/api/timesheets/255', 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testDeleteActionForDifferentUser() @@ -644,7 +644,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertFalse($response->isSuccessful()); $this->assertEquals(Response::HTTP_FORBIDDEN, $response->getStatusCode()); $json = json_decode($response->getContent(), true); - $this->assertEquals('You are not allowed to delete this timesheet', $json['message']); + $this->assertEquals('Access denied.', $json['message']); } public function testDeleteActionForExportedRecordIsNotAllowed() @@ -661,7 +661,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $em->flush(); $this->request($client, '/api/timesheets/' . $id, 'DELETE'); - $this->assertApiResponseAccessDenied($client->getResponse(), 'You are not allowed to delete this timesheet'); + $this->assertApiResponseAccessDenied($client->getResponse(), 'Access denied.'); } public function testDeleteActionForExportedRecordIsAllowedForAdmin() @@ -784,7 +784,7 @@ class TimesheetControllerTest extends APIControllerBaseTest public function testStopThrowsNotFound() { - $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/11/stop', []); + $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/11/stop', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testStopNotAllowedForUser() @@ -807,7 +807,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $id = $timesheets[3]->getId(); $this->request($client, '/api/timesheets/' . $id . '/stop', 'PATCH'); - $this->assertApiResponseAccessDenied($client->getResponse(), 'You are not allowed to stop this timesheet'); + $this->assertApiResponseAccessDenied($client->getResponse(), 'Access denied.'); } public function testGetCollectionWithTags() @@ -980,12 +980,12 @@ class TimesheetControllerTest extends APIControllerBaseTest $id = $timesheets[0]->getId(); $this->request($client, '/api/timesheets/' . $id . '/restart', 'PATCH'); - $this->assertApiResponseAccessDenied($client->getResponse(), 'You are not allowed to re-start this timesheet'); + $this->assertApiResponseAccessDenied($client->getResponse(), 'Access denied.'); } public function testRestartThrowsNotFound() { - $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/42/restart', []); + $this->assertEntityNotFoundForPatch(User::ROLE_USER, '/api/timesheets/42/restart', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testDuplicateAction() @@ -1024,7 +1024,7 @@ class TimesheetControllerTest extends APIControllerBaseTest public function testDuplicateThrowsNotFound() { - $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/11/duplicate', []); + $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/11/duplicate', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testExportAction() @@ -1063,19 +1063,17 @@ class TimesheetControllerTest extends APIControllerBaseTest $id = $timesheets[0]->getId(); $this->request($client, '/api/timesheets/' . $id . '/export', 'PATCH'); - $this->assertApiResponseAccessDenied($client->getResponse(), 'You are not allowed to lock this timesheet'); + $this->assertApiResponseAccessDenied($client->getResponse(), 'Access denied.'); } public function testExportThrowsNotFound() { - $id = PHP_INT_MAX; - $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/' . $id . '/export', []); + $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/' . PHP_INT_MAX . '/export', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testMetaActionThrowsNotFound() { - $id = PHP_INT_MAX; - $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/' . $id . '/meta', []); + $this->assertEntityNotFoundForPatch(User::ROLE_ADMIN, '/api/timesheets/' . PHP_INT_MAX . '/meta', [], 'App\\Entity\\Timesheet object not found by the @ParamConverter annotation.'); } public function testMetaActionThrowsExceptionOnMissingName() @@ -1084,7 +1082,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $timesheets = $this->importFixtureForUser(User::ROLE_USER); $id = $timesheets[0]->getId(); - return $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['value' => 'X'], [ + $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['value' => 'X'], [ 'code' => 400, 'message' => 'Parameter "name" of value "NULL" violated a constraint "This value should not be null."' ]); @@ -1096,7 +1094,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $timesheets = $this->importFixtureForUser(User::ROLE_USER); $id = $timesheets[0]->getId(); - return $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['name' => 'X'], [ + $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['name' => 'X'], [ 'code' => 400, 'message' => 'Parameter "value" of value "NULL" violated a constraint "This value should not be null."' ]); @@ -1108,7 +1106,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $timesheets = $this->importFixtureForUser(User::ROLE_USER); $id = $timesheets[0]->getId(); - return $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['name' => 'X', 'value' => 'Y'], [ + $this->assertExceptionForMethod($client, '/api/timesheets/' . $id . '/meta', 'PATCH', ['name' => 'X', 'value' => 'Y'], [ 'code' => 500, 'message' => 'Unknown meta-field requested' ]); diff --git a/tests/Controller/TimesheetControllerTest.php b/tests/Controller/TimesheetControllerTest.php index 2f59098b..29f10b69 100644 --- a/tests/Controller/TimesheetControllerTest.php +++ b/tests/Controller/TimesheetControllerTest.php @@ -19,6 +19,7 @@ use App\Repository\ConfigurationRepository; use App\Tests\DataFixtures\ActivityFixtures; use App\Tests\DataFixtures\TimesheetFixtures; use App\Tests\Mocks\TimesheetTestMetaFieldSubscriberMock; +use App\Timesheet\DateTimeFactory; /** * @group integration @@ -634,4 +635,51 @@ class TimesheetControllerTest extends ControllerBaseTest self::assertEquals(13, $timesheet->getFixedRate()); } } + + public function testDuplicateAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $dateTime = new DateTimeFactory(new \DateTimeZone('Europe/London')); + + $fixture = new TimesheetFixtures(); + $fixture->setAmount(1); + $fixture->setAmountRunning(0); + $fixture->setUser($this->getUserByRole(User::ROLE_USER)); + $fixture->setStartDate($dateTime->createDateTime()); + $fixture->setCallback(function (Timesheet $timesheet) { + $timesheet->setDescription('Testing is fun!'); + $end = clone $timesheet->getBegin(); + $end->modify('+ 16 hours'); + $timesheet->setEnd($end); + $timesheet->setFixedRate(2016); + $timesheet->setHourlyRate(127); + }); + + /** @var Timesheet[] $ids */ + $ids = $this->importFixture($fixture); + $newId = $ids[0]->getId(); + + $this->request($client, '/timesheet/' . $newId . '/duplicate'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=timesheet_edit_form]')->form(); + $client->submit($form, $form->getPhpValues()); + + $this->assertIsRedirect($client, $this->createUrl('/timesheet/')); + $client->followRedirect(); + $this->assertTrue($client->getResponse()->isSuccessful()); + $this->assertHasFlashSuccess($client); + + $em = $this->getEntityManager(); + /** @var Timesheet $timesheet */ + $timesheet = $em->getRepository(Timesheet::class)->find($newId++); + $this->assertInstanceOf(\DateTime::class, $timesheet->getBegin()); + $this->assertEquals('Europe/London', $timesheet->getBegin()->getTimezone()->getName()); + $this->assertEquals('Testing is fun!', $timesheet->getDescription()); + $this->assertEquals(2016, $timesheet->getRate()); + $this->assertEquals(127, $timesheet->getHourlyRate()); + $this->assertEquals(2016, $timesheet->getFixedRate()); + $this->assertTrue($timesheet->getDuration() == 57600 || $timesheet->getDuration() == 57660); // 1 minute rounding might be applied + $this->assertEquals(2016, $timesheet->getRate()); + } } diff --git a/tests/Controller/TimesheetTeamControllerTest.php b/tests/Controller/TimesheetTeamControllerTest.php index de727bde..f30af226 100644 --- a/tests/Controller/TimesheetTeamControllerTest.php +++ b/tests/Controller/TimesheetTeamControllerTest.php @@ -14,6 +14,7 @@ use App\Entity\TimesheetMeta; use App\Entity\User; use App\Form\Type\DateRangeType; use App\Tests\DataFixtures\TimesheetFixtures; +use App\Timesheet\DateTimeFactory; use App\Timesheet\Util; /** @@ -405,4 +406,51 @@ class TimesheetTeamControllerTest extends ControllerBaseTest self::assertEquals(Util::calculateRate(13.78, $timesheet->getDuration()), $timesheet->getRate()); } } + + public function testDuplicateAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $dateTime = new DateTimeFactory(new \DateTimeZone('Europe/London')); + + $fixture = new TimesheetFixtures(); + $fixture->setAmount(1); + $fixture->setAmountRunning(0); + $fixture->setUser($this->getUserByRole(User::ROLE_USER)); + $fixture->setStartDate($dateTime->createDateTime()); + $fixture->setCallback(function (Timesheet $timesheet) { + $timesheet->setDescription('Testing is fun!'); + $end = clone $timesheet->getBegin(); + $end->modify('+ 16 hours'); + $timesheet->setEnd($end); + $timesheet->setFixedRate(2016); + $timesheet->setHourlyRate(127); + }); + + /** @var Timesheet[] $ids */ + $ids = $this->importFixture($fixture); + $newId = $ids[0]->getId(); + + $this->request($client, '/team/timesheet/' . $newId . '/duplicate'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=timesheet_admin_edit_form]')->form(); + $client->submit($form, $form->getPhpValues()); + + $this->assertIsRedirect($client, $this->createUrl('/team/timesheet/')); + $client->followRedirect(); + $this->assertTrue($client->getResponse()->isSuccessful()); + $this->assertHasFlashSuccess($client); + + $em = $this->getEntityManager(); + /** @var Timesheet $timesheet */ + $timesheet = $em->getRepository(Timesheet::class)->find($newId++); + $this->assertInstanceOf(\DateTime::class, $timesheet->getBegin()); + $this->assertEquals('Europe/London', $timesheet->getBegin()->getTimezone()->getName()); + $this->assertEquals('Testing is fun!', $timesheet->getDescription()); + $this->assertEquals(2016, $timesheet->getRate()); + $this->assertEquals(127, $timesheet->getHourlyRate()); + $this->assertEquals(2016, $timesheet->getFixedRate()); + $this->assertTrue($timesheet->getDuration() == 57600 || $timesheet->getDuration() == 57660); // 1 minute rounding might be applied + $this->assertEquals(2016, $timesheet->getRate()); + } } diff --git a/tests/DataFixtures/TimesheetFixtures.php b/tests/DataFixtures/TimesheetFixtures.php index 6543e377..c26b4edc 100644 --- a/tests/DataFixtures/TimesheetFixtures.php +++ b/tests/DataFixtures/TimesheetFixtures.php @@ -45,9 +45,9 @@ final class TimesheetFixtures implements TestFixture */ private $projects = []; /** - * @var string + * @var \DateTime */ - private $startDate = '2018-04-01'; + private $startDate; /** * @var bool */ @@ -121,8 +121,8 @@ final class TimesheetFixtures implements TestFixture */ public function setStartDate($date): TimesheetFixtures { - if ($date instanceof \DateTime) { - $date = $date->format('Y-m-d'); + if (!($date instanceof \DateTime)) { + $date = new \DateTime($date); } $this->startDate = $date; @@ -312,7 +312,11 @@ final class TimesheetFixtures implements TestFixture private function getDateTime(int $i): \DateTime { - $start = \DateTime::createFromFormat('Y-m-d', $this->startDate); + if ($this->startDate === null) { + $this->startDate = new \DateTime('2018-04-01'); + } + + $start = clone $this->startDate; $start->modify("+ $i days"); $start->modify('+ ' . rand(1, 172800) . ' seconds'); // up to 2 days diff --git a/tests/Voter/TimesheetVoterTest.php b/tests/Voter/TimesheetVoterTest.php index 603cca6b..e981855c 100644 --- a/tests/Voter/TimesheetVoterTest.php +++ b/tests/Voter/TimesheetVoterTest.php @@ -210,12 +210,11 @@ class TimesheetVoterTest extends AbstractVoterTest 'lockdown_period_start' => $lockdownBegin, 'lockdown_period_end' => $lockdownEnd, 'lockdown_grace_period' => $lockdownGrace, - 'allow_overlapping_records' => true, ], ] ]); - $voter = new TimesheetVoter($this->getRolePermissionManager(), new LockdownService($config), $config); + $voter = new TimesheetVoter($this->getRolePermissionManager(), new LockdownService($config)); self::assertInstanceOf(Voter::class, $voter); return $voter;