From dcc52f1a9510be3dfe954255f2f87e4393a3b0a8 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Fri, 25 Oct 2024 10:47:58 +0200 Subject: [PATCH] API begin and end fields for Admins (#5134) --- src/Timesheet/TrackingMode/DefaultMode.php | 2 +- .../TrackingMode/DurationFixedBeginMode.php | 8 +++- src/Timesheet/TrackingMode/PunchInOutMode.php | 9 +++- .../TrackingMode/TrackingModeInterface.php | 21 +--------- src/Timesheet/TrackingModeService.php | 22 +++++++--- tests/API/APIControllerBaseTest.php | 14 ++++++- tests/API/TimesheetControllerTest.php | 42 +++++++++++++++++++ tests/Controller/TimesheetControllerTest.php | 27 ++++++++++++ tests/Mocks/TrackingModeServiceFactory.php | 6 ++- .../DurationFixedBeginModeTest.php | 20 ++++++++- .../TrackingMode/PunchInOutModeTest.php | 27 ++++++++++-- tests/phpstan.neon | 10 ----- 12 files changed, 161 insertions(+), 47 deletions(-) diff --git a/src/Timesheet/TrackingMode/DefaultMode.php b/src/Timesheet/TrackingMode/DefaultMode.php index 5117b633..0fe827ec 100644 --- a/src/Timesheet/TrackingMode/DefaultMode.php +++ b/src/Timesheet/TrackingMode/DefaultMode.php @@ -16,7 +16,7 @@ use Symfony\Component\HttpFoundation\Request; final class DefaultMode extends AbstractTrackingMode { - public function __construct(private RoundingService $rounding) + public function __construct(private readonly RoundingService $rounding) { } diff --git a/src/Timesheet/TrackingMode/DurationFixedBeginMode.php b/src/Timesheet/TrackingMode/DurationFixedBeginMode.php index d51d9083..de7be261 100644 --- a/src/Timesheet/TrackingMode/DurationFixedBeginMode.php +++ b/src/Timesheet/TrackingMode/DurationFixedBeginMode.php @@ -13,12 +13,16 @@ use App\Configuration\SystemConfiguration; use App\Entity\Timesheet; use DateTime; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; final class DurationFixedBeginMode implements TrackingModeInterface { use TrackingModeTrait; - public function __construct(private SystemConfiguration $configuration) + public function __construct( + private readonly SystemConfiguration $configuration, + private readonly AuthorizationCheckerInterface $authorizationChecker + ) { } @@ -39,7 +43,7 @@ final class DurationFixedBeginMode implements TrackingModeInterface public function canUpdateTimesWithAPI(): bool { - return false; + return $this->authorizationChecker->isGranted('view_other_timesheet'); } public function create(Timesheet $timesheet, ?Request $request = null): void diff --git a/src/Timesheet/TrackingMode/PunchInOutMode.php b/src/Timesheet/TrackingMode/PunchInOutMode.php index 6e75848f..90e2a7fb 100644 --- a/src/Timesheet/TrackingMode/PunchInOutMode.php +++ b/src/Timesheet/TrackingMode/PunchInOutMode.php @@ -12,11 +12,18 @@ namespace App\Timesheet\TrackingMode; use App\Entity\Timesheet; use DateTime; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; final class PunchInOutMode implements TrackingModeInterface { use TrackingModeTrait; + public function __construct( + private readonly AuthorizationCheckerInterface $authorizationChecker + ) + { + } + public function canEditBegin(): bool { return false; @@ -34,7 +41,7 @@ final class PunchInOutMode implements TrackingModeInterface public function canUpdateTimesWithAPI(): bool { - return false; + return $this->authorizationChecker->isGranted('view_other_timesheet'); } public function create(Timesheet $timesheet, ?Request $request = null): void diff --git a/src/Timesheet/TrackingMode/TrackingModeInterface.php b/src/Timesheet/TrackingMode/TrackingModeInterface.php index 4117db38..1b1896f3 100644 --- a/src/Timesheet/TrackingMode/TrackingModeInterface.php +++ b/src/Timesheet/TrackingMode/TrackingModeInterface.php @@ -23,61 +23,44 @@ use Symfony\Component\HttpFoundation\Request; interface TrackingModeInterface { /** - * Set default values on this new timesheet entity, - * before form data is rendered/processed. - * - * @param Timesheet $timesheet - * @param Request|null $request + * Set default values on this new timesheet entity, before form data is rendered/processed. */ public function create(Timesheet $timesheet, ?Request $request = null): void; /** * Whether the user can edit the begin datetime. - * - * @return bool */ public function canEditBegin(): bool; /** * Whether the user can edit the end datetime. - * - * @return bool */ public function canEditEnd(): bool; /** * Whether the user can edit the duration. - * If this is true, the result of canEditEnd() will be ignored. * - * @return bool + * If this is true, the result of canEditEnd() will be ignored. */ public function canEditDuration(): bool; /** * Whether the API can be used to manipulate the start and end times. - * - * @return bool */ public function canUpdateTimesWithAPI(): bool; /** * Returns the edit template path for this tracking mode for regular user mode. - * - * @return string */ public function getEditTemplate(): string; /** * Whether the real begin and end times are shown in the user timesheet. - * - * @return bool */ public function canSeeBeginAndEndTimes(): bool; /** * Returns a unique identifier for this tracking mode. - * - * @return string */ public function getId(): string; } diff --git a/src/Timesheet/TrackingModeService.php b/src/Timesheet/TrackingModeService.php index 5831cb65..4fd6ac73 100644 --- a/src/Timesheet/TrackingModeService.php +++ b/src/Timesheet/TrackingModeService.php @@ -16,8 +16,9 @@ use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; final class TrackingModeService { + private ?TrackingModeInterface $active = null; + /** - * @param SystemConfiguration $configuration * @param TrackingModeInterface[] $modes */ public function __construct( @@ -38,14 +39,23 @@ final class TrackingModeService public function getActiveMode(): TrackingModeInterface { - $trackingMode = $this->configuration->getTimesheetTrackingMode(); + // internal caching for the current request + // there is no use-case to change that during one requests lifetime + if ($this->active === null) { + $trackingMode = $this->configuration->getTimesheetTrackingMode(); - foreach ($this->getModes() as $mode) { - if ($mode->getId() === $trackingMode) { - return $mode; + foreach ($this->getModes() as $mode) { + if ($mode->getId() === $trackingMode) { + $this->active = $mode; + break; + } + } + + if ($this->active === null) { + throw new ServiceNotFoundException($trackingMode); } } - throw new ServiceNotFoundException($trackingMode); + return $this->active; } } diff --git a/tests/API/APIControllerBaseTest.php b/tests/API/APIControllerBaseTest.php index defc1223..2341b796 100644 --- a/tests/API/APIControllerBaseTest.php +++ b/tests/API/APIControllerBaseTest.php @@ -211,7 +211,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest * @param bool $extraFields test for the error "This form should not contain extra fields" * @param array|array $globalError */ - protected function assertApiCallValidationError(Response $response, array $failedFields, bool $extraFields = false, array $globalError = []): void + protected function assertApiCallValidationError(Response $response, array $failedFields, bool $extraFields = false, array $globalError = [], array $expectedFields = [], array $missingFields = []): void { self::assertFalse($response->isSuccessful()); $result = json_decode($response->getContent(), true); @@ -232,6 +232,18 @@ abstract class APIControllerBaseTest extends ControllerBaseTest self::assertArrayHasKey('children', $result['errors']); $data = $result['errors']['children']; + if (\count($expectedFields) > 0) { + foreach ($expectedFields as $expectedField) { + $this->assertArrayHasKey($expectedField, $data, 'Expected field is missing: ' . $expectedField); + } + } + + if (\count($missingFields) > 0) { + foreach ($missingFields as $missingField) { + $this->assertArrayNotHasKey($missingField, $data, 'Expected missing field is available: ' . $missingField); + } + } + $foundErrors = []; foreach ($failedFields as $key => $value) { diff --git a/tests/API/TimesheetControllerTest.php b/tests/API/TimesheetControllerTest.php index 735dea70..438cdcf4 100644 --- a/tests/API/TimesheetControllerTest.php +++ b/tests/API/TimesheetControllerTest.php @@ -752,6 +752,48 @@ class TimesheetControllerTest extends APIControllerBaseTest $this->assertTrue($result['billable']); } + public function getTrackingModeTestData(): array + { + return [ + ['duration_fixed_begin', User::ROLE_USER, false], + ['duration_fixed_begin', User::ROLE_ADMIN, true], + ['duration_fixed_begin', User::ROLE_SUPER_ADMIN, true], + ['punch', User::ROLE_USER, false], + ['punch', User::ROLE_ADMIN, true], + ['punch', User::ROLE_SUPER_ADMIN, true], + ['default', User::ROLE_USER, true], + ['default', User::ROLE_ADMIN, true], + ['default', User::ROLE_SUPER_ADMIN, true] + ]; + } + + /** + * @dataProvider getTrackingModeTestData + */ + public function testCreateActionWithTrackingModeHasFieldsForUser(string $trackingMode, string $user, bool $showTimes): void + { + $dateTime = new DateTimeFactory(new \DateTimeZone(self::TEST_TIMEZONE)); + $client = $this->getClientForAuthenticatedUser($user); + $this->setSystemConfiguration('timesheet.mode', $trackingMode); + $data = [ + 'activity' => 1, + 'project' => 1, + 'begin' => ($dateTime->createDateTime('-8 hours'))->format('Y-m-d H:m:0'), + 'end' => ($dateTime->createDateTime())->format('Y-m-d H:m:0'), + 'description' => 'foo', + ]; + $json = json_encode($data); + self::assertIsString($json); + $this->request($client, '/api/timesheets', 'POST', [], $json); + $response = $client->getResponse(); + + if ($showTimes) { + $this->assertTrue($response->isSuccessful()); + } else { + $this->assertApiCallValidationError($response, [], true, [], [], ['begin', 'end']); + } + } + public function testPatchAction(): void { $dateTime = new DateTimeFactory(new \DateTimeZone(self::TEST_TIMEZONE)); diff --git a/tests/Controller/TimesheetControllerTest.php b/tests/Controller/TimesheetControllerTest.php index 461abd06..135746cf 100644 --- a/tests/Controller/TimesheetControllerTest.php +++ b/tests/Controller/TimesheetControllerTest.php @@ -295,6 +295,33 @@ class TimesheetControllerTest extends ControllerBaseTest $this->assertFalse($form->has('fixedRate')); } + public function getTrackingModeTestData(): array + { + return [ + ['duration_fixed_begin', User::ROLE_USER, false, false], + ['duration_fixed_begin', User::ROLE_SUPER_ADMIN, false, false], + ['punch', User::ROLE_USER, false, false], + ['punch', User::ROLE_SUPER_ADMIN, false, false], + ['default', User::ROLE_USER, true, true], + ['default', User::ROLE_SUPER_ADMIN, true, true], + ]; + } + + /** + * @dataProvider getTrackingModeTestData + */ + public function testCreateActionWithTrackingModeHasFieldsForUser(string $trackingMode, string $user, bool $showBeginTime, bool $showEndTime): void + { + $client = $this->getClientForAuthenticatedUser($user); + $this->setSystemConfiguration('timesheet.mode', $trackingMode); + $this->request($client, '/timesheet/create'); + $this->assertTrue($client->getResponse()->isSuccessful()); + + $form = $client->getCrawler()->filter('form[name=timesheet_edit_form]')->form(); + $this->assertEquals($showBeginTime, $form->has('timesheet_edit_form[begin_time]')); + $this->assertEquals($showEndTime, $form->has('timesheet_edit_form[end_time]')); + } + public function testCreateActionWithFromAndToValues(): void { $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); diff --git a/tests/Mocks/TrackingModeServiceFactory.php b/tests/Mocks/TrackingModeServiceFactory.php index 1c1ab6f2..e3288e6c 100644 --- a/tests/Mocks/TrackingModeServiceFactory.php +++ b/tests/Mocks/TrackingModeServiceFactory.php @@ -14,6 +14,7 @@ use App\Timesheet\TrackingMode\DefaultMode; use App\Timesheet\TrackingMode\DurationFixedBeginMode; use App\Timesheet\TrackingMode\PunchInOutMode; use App\Timesheet\TrackingModeService; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; class TrackingModeServiceFactory extends AbstractMockFactory { @@ -26,12 +27,13 @@ class TrackingModeServiceFactory extends AbstractMockFactory $loader = new TestConfigLoader([]); $configuration = SystemConfigurationFactory::create($loader, ['timesheet' => ['mode' => $mode]]); + $auth = $this->createMock(AuthorizationCheckerInterface::class); if (null === $modes) { $modes = [ new DefaultMode((new RoundingServiceFactory($this->getTestCase()))->create()), - new PunchInOutMode(), - new DurationFixedBeginMode($configuration), + new PunchInOutMode($auth), + new DurationFixedBeginMode($configuration, $auth), ]; } diff --git a/tests/Timesheet/TrackingMode/DurationFixedBeginModeTest.php b/tests/Timesheet/TrackingMode/DurationFixedBeginModeTest.php index 9a4adfeb..5f0d802a 100644 --- a/tests/Timesheet/TrackingMode/DurationFixedBeginModeTest.php +++ b/tests/Timesheet/TrackingMode/DurationFixedBeginModeTest.php @@ -16,18 +16,22 @@ use App\Tests\Mocks\SystemConfigurationFactory; use App\Timesheet\TrackingMode\DurationFixedBeginMode; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; /** * @covers \App\Timesheet\TrackingMode\DurationFixedBeginMode */ class DurationFixedBeginModeTest extends TestCase { - protected function createSut($default = '13:47') + private function createSut(string $default = '13:47', bool $allowApiTimes = false): DurationFixedBeginMode { $loader = new TestConfigLoader([]); $configuration = SystemConfigurationFactory::create($loader, ['timesheet' => ['default_begin' => $default]]); - return new DurationFixedBeginMode($configuration); + $auth = $this->createMock(AuthorizationCheckerInterface::class); + $auth->method('isGranted')->willReturn($allowApiTimes); + + return new DurationFixedBeginMode($configuration, $auth); } public function testDefaultValues(): void @@ -42,6 +46,18 @@ class DurationFixedBeginModeTest extends TestCase self::assertEquals('duration_fixed_begin', $sut->getId()); } + public function testValuesForAdmin(): void + { + $sut = $this->createSut('now', true); + + self::assertFalse($sut->canEditBegin()); + self::assertFalse($sut->canEditEnd()); + self::assertTrue($sut->canEditDuration()); + self::assertTrue($sut->canUpdateTimesWithAPI()); + self::assertFalse($sut->canSeeBeginAndEndTimes()); + self::assertEquals('duration_fixed_begin', $sut->getId()); + } + public function testNow(): void { $seconds = (new \DateTime())->getTimestamp(); diff --git a/tests/Timesheet/TrackingMode/PunchInOutModeTest.php b/tests/Timesheet/TrackingMode/PunchInOutModeTest.php index bcf9afc1..3b45ef76 100644 --- a/tests/Timesheet/TrackingMode/PunchInOutModeTest.php +++ b/tests/Timesheet/TrackingMode/PunchInOutModeTest.php @@ -14,15 +14,24 @@ use App\Entity\User; use App\Timesheet\TrackingMode\PunchInOutMode; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; /** * @covers \App\Timesheet\TrackingMode\PunchInOutMode */ class PunchInOutModeTest extends TestCase { + private function createSut(bool $allowApiTimes = false): PunchInOutMode + { + $auth = $this->createMock(AuthorizationCheckerInterface::class); + $auth->method('isGranted')->willReturn($allowApiTimes); + + return new PunchInOutMode($auth); + } + public function testDefaultValues(): void { - $sut = new PunchInOutMode(); + $sut = $this->createSut(); self::assertFalse($sut->canEditBegin()); self::assertFalse($sut->canEditEnd()); @@ -32,6 +41,18 @@ class PunchInOutModeTest extends TestCase self::assertEquals('punch', $sut->getId()); } + public function testValuesForAdmin(): void + { + $sut = $this->createSut(true); + + self::assertFalse($sut->canEditBegin()); + self::assertFalse($sut->canEditEnd()); + self::assertFalse($sut->canEditDuration()); + self::assertTrue($sut->canUpdateTimesWithAPI()); + self::assertTrue($sut->canSeeBeginAndEndTimes()); + self::assertEquals('punch', $sut->getId()); + } + public function testCreate(): void { $startingTime = new \DateTime('22:54'); @@ -39,7 +60,7 @@ class PunchInOutModeTest extends TestCase $timesheet->setBegin($startingTime); $request = new Request(); - $sut = new PunchInOutMode(); + $sut = $this->createSut(); $sut->create($timesheet, $request); self::assertEquals($timesheet->getBegin(), $startingTime); } @@ -49,7 +70,7 @@ class PunchInOutModeTest extends TestCase $timesheet = (new Timesheet())->setUser(new User()); $request = new Request(); - $sut = new PunchInOutMode(); + $sut = $this->createSut(); $sut->create($timesheet, $request); self::assertInstanceOf(\DateTime::class, $timesheet->getBegin()); } diff --git a/tests/phpstan.neon b/tests/phpstan.neon index 5750efc4..3f5dbcdb 100644 --- a/tests/phpstan.neon +++ b/tests/phpstan.neon @@ -3349,16 +3349,6 @@ parameters: count: 1 path: Timesheet/TrackingMode/DurationFixedBeginModeTest.php - - - message: "#^Method App\\\\Tests\\\\Timesheet\\\\TrackingMode\\\\DurationFixedBeginModeTest\\:\\:createSut\\(\\) has no return type specified\\.$#" - count: 1 - path: Timesheet/TrackingMode/DurationFixedBeginModeTest.php - - - - message: "#^Method App\\\\Tests\\\\Timesheet\\\\TrackingMode\\\\DurationFixedBeginModeTest\\:\\:createSut\\(\\) has parameter \\$default with no type specified\\.$#" - count: 1 - path: Timesheet/TrackingMode/DurationFixedBeginModeTest.php - - message: "#^Method App\\\\Tests\\\\Timesheet\\\\UtilTest\\:\\:getRateCalculationData\\(\\) has no return type specified\\.$#" count: 1