From 6b066046c6b878bab64af2e90cf3a08b351dabaa Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Wed, 13 Oct 2021 15:49:20 +0200 Subject: [PATCH] default value for billable flag and support in batch update (#2851) --- src/Configuration/SystemConfiguration.php | 5 ++ .../SystemConfigurationController.php | 4 ++ .../TimesheetAbstractController.php | 4 ++ src/DependencyInjection/Configuration.php | 6 +++ src/Form/MultiUpdate/TimesheetMultiUpdate.php | 12 +++++ .../MultiUpdate/TimesheetMultiUpdateDTO.php | 53 +++++++++---------- src/Timesheet/TimesheetService.php | 2 + .../timesheet/layout-multi-update.html.twig | 5 ++ .../DependencyInjection/AppExtensionTest.php | 3 ++ .../DependencyInjection/ConfigurationTest.php | 3 ++ .../TimesheetMultiUpdateDTOTest.php | 28 ++++++---- .../TimesheetMultiUpdateValidatorTest.php | 18 +++---- 12 files changed, 92 insertions(+), 51 deletions(-) diff --git a/src/Configuration/SystemConfiguration.php b/src/Configuration/SystemConfiguration.php index a4597515..7a4459b0 100644 --- a/src/Configuration/SystemConfiguration.php +++ b/src/Configuration/SystemConfiguration.php @@ -232,6 +232,11 @@ class SystemConfiguration implements SystemBundleConfiguration return (string) $this->find('timesheet.default_begin'); } + public function getTimesheetDefaultBillable(): bool + { + return (bool) $this->find('defaults.timesheet.billable'); + } + public function isTimesheetAllowFutureTimes(): bool { return (bool) $this->find('timesheet.rules.allow_future_times'); diff --git a/src/Controller/SystemConfigurationController.php b/src/Controller/SystemConfigurationController.php index 270ac554..22c283f9 100644 --- a/src/Controller/SystemConfigurationController.php +++ b/src/Controller/SystemConfigurationController.php @@ -382,6 +382,10 @@ final class SystemConfigurationController extends AbstractController ->setConstraints([ new GreaterThanOrEqual(['value' => 0]) ]), + (new Configuration()) + ->setName('defaults.timesheet.billable') + ->setType(YesNoType::class) + ->setOptions(['help' => 'default_value_new', 'label' => 'label.billable']), ]), (new SystemConfigurationModel()) ->setSection(SystemConfigurationModel::SECTION_LOCKDOWN) diff --git a/src/Controller/TimesheetAbstractController.php b/src/Controller/TimesheetAbstractController.php index 55a5b01a..a536a1f2 100644 --- a/src/Controller/TimesheetAbstractController.php +++ b/src/Controller/TimesheetAbstractController.php @@ -364,6 +364,10 @@ abstract class TimesheetAbstractController extends AbstractController $timesheet->setExported($dto->isExported()); $execute = true; } + if (null !== $dto->isBillable()) { + $timesheet->setBillable($dto->isBillable()); + $execute = true; + } if ($dto->isRecalculateRates()) { $timesheet->setFixedRate(null); diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index ebadf0b0..81d3ad3f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -635,6 +635,12 @@ class Configuration implements ConfigurationInterface ->scalarNode('currency')->defaultValue(Customer::DEFAULT_CURRENCY)->end() ->end() ->end() + ->arrayNode('timesheet') + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('billable')->defaultTrue()->end() + ->end() + ->end() ->arrayNode('user') ->addDefaultsIfNotSet() ->children() diff --git a/src/Form/MultiUpdate/TimesheetMultiUpdate.php b/src/Form/MultiUpdate/TimesheetMultiUpdate.php index 6297294e..45c89cd0 100644 --- a/src/Form/MultiUpdate/TimesheetMultiUpdate.php +++ b/src/Form/MultiUpdate/TimesheetMultiUpdate.php @@ -207,6 +207,17 @@ class TimesheetMultiUpdate extends AbstractType ]); } + if ($options['include_billable']) { + $builder->add('billable', ChoiceType::class, [ + 'label' => 'label.billable', + 'choices' => [ + '' => null, + 'yes' => true, + 'no' => false, + ], + ]); + } + if ($options['include_rate']) { $builder ->add('recalculateRates', YesNoType::class, [ @@ -279,6 +290,7 @@ class TimesheetMultiUpdate extends AbstractType 'include_user' => false, 'include_rate' => false, 'include_exported' => false, + 'include_billable' => true, ]); } } diff --git a/src/Form/MultiUpdate/TimesheetMultiUpdateDTO.php b/src/Form/MultiUpdate/TimesheetMultiUpdateDTO.php index 7b7fb083..a6f5f2f8 100644 --- a/src/Form/MultiUpdate/TimesheetMultiUpdateDTO.php +++ b/src/Form/MultiUpdate/TimesheetMultiUpdateDTO.php @@ -22,6 +22,7 @@ use Doctrine\Common\Collections\Collection; /** * @App\Validator\Constraints\TimesheetMultiUpdate + * @internal */ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithMetaFields { @@ -57,6 +58,10 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM * @var bool|null */ private $exported = null; + /** + * @var bool|null + */ + private $billable = null; /** * @var float|null */ @@ -84,11 +89,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->customer; } - public function setCustomer(Customer $customer): TimesheetMultiUpdateDTO + public function setCustomer(Customer $customer): void { $this->customer = $customer; - - return $this; } public function getProject(): ?Project @@ -96,11 +99,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->project; } - public function setProject(Project $project): TimesheetMultiUpdateDTO + public function setProject(Project $project): void { $this->project = $project; - - return $this; } public function getActivity(): ?Activity @@ -108,11 +109,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->activity; } - public function setActivity(Activity $activity): TimesheetMultiUpdateDTO + public function setActivity(Activity $activity): void { $this->activity = $activity; - - return $this; } /** @@ -123,11 +122,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->tags; } - public function setTags(iterable $tags): TimesheetMultiUpdateDTO + public function setTags(iterable $tags): void { $this->tags = $tags; - - return $this; } public function getUser(): ?User @@ -135,11 +132,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->user; } - public function setUser(User $user): TimesheetMultiUpdateDTO + public function setUser(User $user): void { $this->user = $user; - - return $this; } public function isExported(): ?bool @@ -147,11 +142,19 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->exported; } - public function setExported(bool $exported): TimesheetMultiUpdateDTO + public function setExported(?bool $exported): void { $this->exported = $exported; + } - return $this; + public function isBillable(): ?bool + { + return $this->billable; + } + + public function setBillable(?bool $billable): void + { + $this->billable = $billable; } public function isRecalculateRates(): bool @@ -159,11 +162,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->recalculateRates; } - public function setRecalculateRates(bool $recalculateRates): TimesheetMultiUpdateDTO + public function setRecalculateRates(bool $recalculateRates): void { $this->recalculateRates = $recalculateRates; - - return $this; } public function isReplaceTags(): bool @@ -171,11 +172,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->replaceTags; } - public function setReplaceTags(bool $replaceTags): TimesheetMultiUpdateDTO + public function setReplaceTags(bool $replaceTags): void { $this->replaceTags = $replaceTags; - - return $this; } public function getFixedRate(): ?float @@ -183,11 +182,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->fixedRate; } - public function setFixedRate(?float $fixedRate): TimesheetMultiUpdateDTO + public function setFixedRate(?float $fixedRate): void { $this->fixedRate = $fixedRate; - - return $this; } public function getHourlyRate(): ?float @@ -195,11 +192,9 @@ class TimesheetMultiUpdateDTO extends MultiUpdateTableDTO implements EntityWithM return $this->hourlyRate; } - public function setHourlyRate(?float $hourlyRate): TimesheetMultiUpdateDTO + public function setHourlyRate(?float $hourlyRate): void { $this->hourlyRate = $hourlyRate; - - return $this; } /** diff --git a/src/Timesheet/TimesheetService.php b/src/Timesheet/TimesheetService.php index ecacdb7a..44e80eff 100644 --- a/src/Timesheet/TimesheetService.php +++ b/src/Timesheet/TimesheetService.php @@ -110,6 +110,8 @@ final class TimesheetService $mode = $this->trackingModeService->getActiveMode(); $mode->create($timesheet, $request); + $timesheet->setBillable($this->configuration->getTimesheetDefaultBillable()); + return $timesheet; } diff --git a/templates/timesheet/layout-multi-update.html.twig b/templates/timesheet/layout-multi-update.html.twig index da6c2c29..797ade92 100644 --- a/templates/timesheet/layout-multi-update.html.twig +++ b/templates/timesheet/layout-multi-update.html.twig @@ -36,6 +36,11 @@ {{ form_row(form.exported) }} {% endif %} + {% if form.billable is defined %} +
+ {{ form_row(form.billable) }} +
+ {% endif %} {% if form.recalculateRates is defined %}
{{ form_row(form.recalculateRates) }} diff --git a/tests/DependencyInjection/AppExtensionTest.php b/tests/DependencyInjection/AppExtensionTest.php index cbcfec47..75898558 100644 --- a/tests/DependencyInjection/AppExtensionTest.php +++ b/tests/DependencyInjection/AppExtensionTest.php @@ -140,6 +140,9 @@ class AppExtensionTest extends TestCase 'templates/invoice/renderer/', ], 'kimai.defaults' => [ + 'timesheet' => [ + 'billable' => true, + ], 'customer' => [ 'timezone' => null, 'country' => 'DE', diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index dfb159d4..b08fda44 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -395,6 +395,9 @@ class ConfigurationTest extends TestCase 'dashboard' => [], 'widgets' => [], 'defaults' => [ + 'timesheet' => [ + 'billable' => true, + ], 'customer' => [ 'timezone' => null, 'country' => 'DE', diff --git a/tests/Form/MultiUpdate/TimesheetMultiUpdateDTOTest.php b/tests/Form/MultiUpdate/TimesheetMultiUpdateDTOTest.php index 777fc7d7..e9dfd218 100644 --- a/tests/Form/MultiUpdate/TimesheetMultiUpdateDTOTest.php +++ b/tests/Form/MultiUpdate/TimesheetMultiUpdateDTOTest.php @@ -32,6 +32,7 @@ class TimesheetMultiUpdateDTOTest extends TestCase self::assertNull($sut->getAction()); self::assertNull($sut->isExported()); + self::assertNull($sut->isBillable()); self::assertNull($sut->getProject()); self::assertNull($sut->getAction()); self::assertNull($sut->getCustomer()); @@ -70,37 +71,44 @@ class TimesheetMultiUpdateDTOTest extends TestCase self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setEntities($entities)); self::assertEquals($entities, $sut->getEntities()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setExported(true)); + self::assertNull($sut->isExported()); + $sut->setExported(true); self::assertTrue($sut->isExported()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setExported(false)); + $sut->setExported(false); self::assertFalse($sut->isExported()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setTags(['foo', '0815'])); + self::assertNull($sut->isBillable()); + $sut->setBillable(true); + self::assertTrue($sut->isBillable()); + $sut->setExported(false); + self::assertFalse($sut->isExported()); + + $sut->setTags(['foo', '0815']); self::assertEquals(['foo', '0815'], $sut->getTags()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setReplaceTags(true)); + $sut->setReplaceTags(true); self::assertTrue($sut->isReplaceTags()); $user = (new User())->setUsername('sdfsdfsd'); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setUser($user)); + $sut->setUser($user); self::assertSame($user, $sut->getUser()); $activity = (new Activity())->setName('sdfsdfsd'); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setActivity($activity)); + $sut->setActivity($activity); self::assertSame($activity, $sut->getActivity()); $project = (new Project())->setName('sdfsdfsd'); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setProject($project)); + $sut->setProject($project); self::assertSame($project, $sut->getProject()); $customer = (new Customer())->setName('sdfsdfsd'); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setCustomer($customer)); + $sut->setCustomer($customer); self::assertSame($customer, $sut->getCustomer()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setFixedRate(12.78)); + $sut->setFixedRate(12.78); self::assertEquals(12.78, $sut->getFixedRate()); - self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setHourlyRate(123.45)); + $sut->setHourlyRate(123.45); self::assertEquals(123.45, $sut->getHourlyRate()); self::assertInstanceOf(TimesheetMultiUpdateDTO::class, $sut->setUpdateMeta(['foo', 'bar'])); diff --git a/tests/Validator/Constraints/TimesheetMultiUpdateValidatorTest.php b/tests/Validator/Constraints/TimesheetMultiUpdateValidatorTest.php index 2c38580a..d526f701 100644 --- a/tests/Validator/Constraints/TimesheetMultiUpdateValidatorTest.php +++ b/tests/Validator/Constraints/TimesheetMultiUpdateValidatorTest.php @@ -44,10 +44,8 @@ class TimesheetMultiUpdateValidatorTest extends ConstraintValidatorTestCase $activity->setProject($project1); $timesheet = new TimesheetMultiUpdateDTO(); - $timesheet - ->setActivity($activity) - ->setProject($project2) - ; + $timesheet->setActivity($activity); + $timesheet->setProject($project2); $this->validator->validate($timesheet, new TimesheetMultiUpdateConstraint(['message' => 'myMessage'])); @@ -90,10 +88,8 @@ class TimesheetMultiUpdateValidatorTest extends ConstraintValidatorTestCase public function testHourlyRateAndFixedRateInParallelAreNotAllowed() { $timesheet = new TimesheetMultiUpdateDTO(); - $timesheet - ->setHourlyRate(10.12) - ->setFixedRate(123.45) - ; + $timesheet->setHourlyRate(10.12); + $timesheet->setFixedRate(123.45); $this->validator->validate($timesheet, new TimesheetMultiUpdateConstraint(['message' => 'myMessage'])); @@ -118,10 +114,8 @@ class TimesheetMultiUpdateValidatorTest extends ConstraintValidatorTestCase $activity->setProject($project); $timesheet = new TimesheetMultiUpdateDTO(); - $timesheet - ->setActivity($activity) - ->setProject($project) - ; + $timesheet->setActivity($activity); + $timesheet->setProject($project); $this->validator->validate($timesheet, new TimesheetMultiUpdateConstraint(['message' => 'myMessage']));