Prevent bookings with same start / stop time (#3304)

src/Configuration/SystemConfiguration.php

@@ -237,6 +237,11 @@ class SystemConfiguration implements SystemBundleConfiguration
         return (bool) $this->find('timesheet.rules.allow_future_times');
     }
 
+    public function isTimesheetAllowZeroDuration(): bool
+    {
+        return (bool) $this->find('timesheet.rules.allow_zero_duration');
+    }
+
     public function isTimesheetAllowOverbookingBudget(): bool
     {
         return (bool) $this->find('timesheet.rules.allow_overbooking_budget');

src/Controller/SystemConfigurationController.php

@@ -338,6 +338,10 @@ final class SystemConfigurationController extends AbstractController
                         ->setName('timesheet.rules.allow_future_times')
                         ->setType(CheckboxType::class)
                         ->setTranslationDomain('system-configuration'),
+                    (new Configuration())
+                        ->setName('timesheet.rules.allow_zero_duration')
+                        ->setType(CheckboxType::class)
+                        ->setTranslationDomain('system-configuration'),
                     (new Configuration())
                         ->setName('timesheet.rules.allow_overlapping_records')
                         ->setType(CheckboxType::class)

src/DependencyInjection/Configuration.php

@@ -243,6 +243,9 @@ class Configuration implements ConfigurationInterface
                         ->booleanNode('allow_future_times')
                             ->defaultTrue()
                         ->end()
+                        ->booleanNode('allow_zero_duration')
+                            ->defaultTrue()
+                        ->end()
                         ->booleanNode('allow_overbooking_budget')
                             ->defaultTrue()
                         ->end()

src/Validator/Constraints/TimesheetZeroDuration.php

@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator\Constraints;
+
+final class TimesheetZeroDuration extends TimesheetConstraint
+{
+    public const ZERO_DURATION_ERROR = 'kimai-timesheet-zero-duration-01';
+
+    protected static $errorNames = [
+        self::ZERO_DURATION_ERROR => 'Duration cannot be zero.',
+    ];
+
+    public $message = 'Duration cannot be zero.';
+
+    public function getTargets()
+    {
+        return self::CLASS_CONSTRAINT;
+    }
+}

src/Validator/Constraints/TimesheetZeroDurationValidator.php

@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator\Constraints;
+
+use App\Configuration\SystemConfiguration;
+use App\Entity\Timesheet as TimesheetEntity;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+
+final class TimesheetZeroDurationValidator extends ConstraintValidator
+{
+    /**
+     * @var SystemConfiguration
+     */
+    private $configuration;
+
+    public function __construct(SystemConfiguration $configuration)
+    {
+        $this->configuration = $configuration;
+    }
+
+    /**
+     * @param TimesheetEntity $timesheet
+     * @param Constraint $constraint
+     */
+    public function validate($timesheet, Constraint $constraint)
+    {
+        if (!($constraint instanceof TimesheetZeroDuration)) {
+            throw new UnexpectedTypeException($constraint, TimesheetZeroDuration::class);
+        }
+
+        if (!\is_object($timesheet) || !($timesheet instanceof TimesheetEntity)) {
+            throw new UnexpectedTypeException($timesheet, TimesheetEntity::class);
+        }
+
+        if ($this->configuration->isTimesheetAllowZeroDuration()) {
+            return;
+        }
+
+        if ($timesheet->getDuration() == 0) {
+            $this->context->buildViolation($constraint->message)
+                ->atPath('duration')
+                ->setTranslationDomain('validators')
+                ->setCode(TimesheetZeroDuration::ZERO_DURATION_ERROR)
+                ->addViolation();
+        }
+    }
+}

tests/Configuration/SystemConfigurationTest.php

@@ -37,6 +37,7 @@ class SystemConfigurationTest extends TestCase
             'timesheet' => [
                 'rules' => [
                     'allow_future_times' => false,
+                    'allow_zero_duration' => true,
                     'lockdown_period_start' => null,
                     'lockdown_period_end' => null,
                     'lockdown_grace_period' => null,

tests/Controller/SystemConfigurationControllerTest.php

@@ -93,6 +93,7 @@ class SystemConfigurationControllerTest extends ControllerBaseTest
         $configService = static::$kernel->getContainer()->get(SystemConfiguration::class);
         $this->assertEquals('default', $configService->find('timesheet.mode'));
         $this->assertTrue($configService->find('timesheet.rules.allow_future_times'));
+        $this->assertTrue($configService->find('timesheet.rules.allow_zero_duration'));
         $this->assertEquals(1, $configService->find('timesheet.active_entries.hard_limit'));
 
         $form = $client->getCrawler()->filter('form[name=system_configuration_form_timesheet]')->form();
@@ -102,6 +103,7 @@ class SystemConfigurationControllerTest extends ControllerBaseTest
                     ['name' => 'timesheet.mode', 'value' => 'duration_only'],
                     ['name' => 'timesheet.active_entries.default_begin', 'value' => '23:59'],
                     ['name' => 'timesheet.rules.allow_future_times', 'value' => false],
+                    ['name' => 'timesheet.rules.allow_zero_duration', 'value' => true],
                     ['name' => 'timesheet.rules.allow_overlapping_records', 'value' => false],
                     ['name' => 'timesheet.rules.allow_overbooking_budget', 'value' => false],
                     ['name' => 'timesheet.active_entries.hard_limit', 'value' => 99],
@@ -168,6 +170,7 @@ class SystemConfigurationControllerTest extends ControllerBaseTest
                         ['name' => 'timesheet.mode', 'value' => 'foo'],
                         ['name' => 'timesheet.active_entries.default_begin', 'value' => '23:59'],
                         ['name' => 'timesheet.rules.allow_future_times', 'value' => 1],
+                        ['name' => 'timesheet.rules.allow_zero_duration', 'value' => 1],
                         ['name' => 'timesheet.rules.allow_overlapping_records', 'value' => 1],
                         ['name' => 'timesheet.rules.allow_overbooking_budget', 'value' => 1],
                         ['name' => 'timesheet.active_entries.hard_limit', 'value' => -1],
@@ -176,7 +179,7 @@ class SystemConfigurationControllerTest extends ControllerBaseTest
             ],
             [
                 '#system_configuration_form_timesheet_configuration_0_value', // mode
-                '#system_configuration_form_timesheet_configuration_5_value', // hard_limit
+                '#system_configuration_form_timesheet_configuration_6_value', // hard_limit
             ],
             true
         );

tests/Controller/TimesheetControllerTest.php

@@ -370,6 +370,7 @@ class TimesheetControllerTest extends ControllerBaseTest
                     ['name' => 'timesheet.mode', 'value' => 'default'],
                     ['name' => 'timesheet.active_entries.default_begin', 'value' => '08:00'],
                     ['name' => 'timesheet.rules.allow_future_times', 'value' => true],
+                    ['name' => 'timesheet.rules.allow_zero_duration', 'value' => true],
                     ['name' => 'timesheet.rules.allow_overlapping_records', 'value' => false],
                     ['name' => 'timesheet.rules.allow_overbooking_budget', 'value' => true],
                     ['name' => 'timesheet.active_entries.hard_limit', 'value' => 1],
@@ -458,6 +459,59 @@ class TimesheetControllerTest extends ControllerBaseTest
         );
     }
 
+    public function testCreateActionWithEmptyDuration()
+    {
+        $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
+
+        $fixture = new ActivityFixtures();
+        $fixture->setAmount(1);
+        $fixture->setIsGlobal(true);
+        $fixture->setIsVisible(true);
+        $fixture->setCallback(function (Activity $activity) {
+            $activity->setBudget(1000);
+            $activity->setTimeBudget(3600);
+        });
+        $activities = $this->importFixture($fixture);
+        /** @var Activity $activity */
+        $activity = $activities[0];
+
+        $fixture = new TimesheetFixtures();
+        $fixture->setAmount(1);
+        $fixture->setActivities([$activity]);
+        $fixture->setUser($this->getUserByRole(User::ROLE_USER));
+        $timesheets = $this->importFixture($fixture);
+        $id = $timesheets[0]->getId();
+
+        $this->request($client, '/timesheet/' . $id . '/edit');
+
+        $response = $client->getResponse();
+        $this->assertTrue($response->isSuccessful());
+
+        /** @var ConfigurationRepository $repository */
+        $repository = $this->getEntityManager()->getRepository(Configuration::class);
+        $config = new Configuration();
+        $config->setName('timesheet.rules.allow_zero_duration');
+        $config->setValue(false);
+        $repository->saveConfiguration($config);
+
+        $this->assertHasValidationError(
+            $client,
+            '/timesheet/' . $id . '/edit',
+            'form[name=timesheet_edit_form]',
+            [
+                'timesheet_edit_form' => [
+                    'hourlyRate' => 100,
+                    'begin' => '2020-02-18 01:00',
+                    'end' => '2020-02-18 01:00',
+                    'duration' => '00:00',
+                    'project' => 1,
+                    'activity' => $activity->getId(),
+                ]
+            ],
+            ['#timesheet_edit_form_duration']
+        );
+    }
+
     public function testCreateActionWithBeginAndEndAndTagValues()
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);

tests/DependencyInjection/AppExtensionTest.php

@@ -202,6 +202,7 @@ class AppExtensionTest extends TestCase
                 ],
                 'rules' => [
                     'allow_future_times' => true,
+                    'allow_zero_duration' => true,
                     'allow_overlapping_records' => true,
                     'lockdown_period_start' => null,
                     'lockdown_period_end' => null,

tests/DependencyInjection/ConfigurationTest.php

@@ -296,6 +296,7 @@ class ConfigurationTest extends TestCase
                 ],
                 'rules' => [
                     'allow_future_times' => true,
+                    'allow_zero_duration' => true,
                     'allow_overlapping_records' => true,
                     'lockdown_period_start' => null,
                     'lockdown_period_end' => null,

tests/Validator/Constraints/TimesheetZeroDurationValidatorTest.php

@@ -0,0 +1,95 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Validator\Constraints;
+
+use App\Configuration\ConfigLoaderInterface;
+use App\Configuration\SystemConfiguration;
+use App\Entity\Timesheet;
+use App\Validator\Constraints\TimesheetZeroDuration;
+use App\Validator\Constraints\TimesheetZeroDurationValidator;
+use Symfony\Component\Validator\Constraints\NotBlank;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+/**
+ * @covers \App\Validator\Constraints\TimesheetZeroDuration
+ * @covers \App\Validator\Constraints\TimesheetZeroDurationValidator
+ */
+class TimesheetZeroDurationValidatorTest extends ConstraintValidatorTestCase
+{
+    protected function createValidator()
+    {
+        return $this->createMyValidator(false);
+    }
+
+    protected function createMyValidator(bool $allowZeroDuration = false)
+    {
+        $loader = $this->createMock(ConfigLoaderInterface::class);
+        $config = new SystemConfiguration($loader, [
+            'timesheet' => [
+                'rules' => [
+                    'allow_zero_duration' => $allowZeroDuration,
+                ],
+            ]
+        ]);
+
+        return new TimesheetZeroDurationValidator($config);
+    }
+
+    public function testConstraintIsInvalid()
+    {
+        $this->expectException(UnexpectedTypeException::class);
+
+        $this->validator->validate(new Timesheet(), new NotBlank());
+    }
+
+    public function testInvalidValueThrowsException()
+    {
+        $this->expectException(UnexpectedTypeException::class);
+
+        $this->validator->validate(new NotBlank(), new TimesheetZeroDuration(['message' => 'Duration cannot be zero.']));
+    }
+
+    private function prepareTimesheet()
+    {
+        // creates Timesheet with same begin and endtime
+        $begin = new \DateTime();
+        $timesheet = new Timesheet();
+        $timesheet->setBegin(clone $begin);
+        $timesheet->setEnd(clone $begin);
+        $timesheet->setDuration(0);
+
+        return $timesheet;
+    }
+
+    public function testZeroDurationIsDisallowed()
+    {
+        $timesheet = $this->prepareTimesheet();
+
+        $this->validator->validate($timesheet, new TimesheetZeroDuration(['message' => 'Duration cannot be zero.']));
+
+        $this->buildViolation('Duration cannot be zero.')
+            ->atPath('property.path.duration')
+            ->setCode(TimesheetZeroDuration::ZERO_DURATION_ERROR)
+            ->assertRaised();
+    }
+
+    public function testZeroDurationIsAllowed()
+    {
+        $this->validator = $this->createMyValidator(true);
+        $this->validator->initialize($this->context);
+
+        $timesheet = $this->prepareTimesheet();
+
+        $this->validator->validate($timesheet, new TimesheetZeroDuration(['message' => 'Duration cannot be zero.']));
+
+        $this->assertNoViolation();
+    }
+}

translations/system-configuration.de.xlf

@@ -50,6 +50,10 @@
         <source>label.timesheet.rules.allow_future_times</source>
         <target>Erlaube Zeiteinträge in der Zukunft</target>
       </trans-unit>
+      <trans-unit id="z4tQEPb" resname="label.timesheet.rules.allow_zero_duration">
+        <source>label.timesheet.rules.allow_zero_duration</source>
+        <target>Erlaube Zeiteinträge mit einer leeren Dauer</target>
+      </trans-unit>
       <trans-unit id="VAAaL2t" resname="label.timesheet.rules.allow_overbooking_budget">
         <source>label.timesheet.rules.allow_overbooking_budget</source>
         <target>Überbuchung hinterlegter Budgets erlauben</target>

translations/system-configuration.en.xlf

@@ -50,6 +50,10 @@
         <source>label.timesheet.rules.allow_future_times</source>
         <target>Allow time entries in the future</target>
       </trans-unit>
+      <trans-unit id="z4tQEPb" resname="label.timesheet.rules.allow_zero_duration">
+        <source>label.timesheet.rules.allow_zero_duration</source>
+        <target>Allow time entries with an empty duration</target>
+      </trans-unit>
       <trans-unit id="VAAaL2t" resname="label.timesheet.rules.allow_overbooking_budget">
         <source>label.timesheet.rules.allow_overbooking_budget</source>
         <target>Allow overbooking of stored budgets</target>

translations/validators.de.xlf

@@ -38,6 +38,10 @@
         <source>The begin date cannot be in the future.</source>
         <target>Das Startdatum darf nicht in der Zukunft liegen.</target>
       </trans-unit>
+      <trans-unit id="ePrqiLN" resname="Duration cannot be zero.">
+        <source>Duration cannot be zero.</source>
+        <target>Eine leere Dauer ist nicht erlaubt.</target>
+      </trans-unit>
       <trans-unit id="1V6BsD_" resname="You must select at least one user or team.">
         <source>You must select at least one user or team.</source>
         <target>Sie müssen mindestens einen Benutzer oder ein Team auswählen.</target>

translations/validators.en.xlf

@@ -38,6 +38,10 @@
         <source>The begin date cannot be in the future.</source>
         <target>The begin date cannot be in the future.</target>
       </trans-unit>
+      <trans-unit id="ePrqiLN" resname="Duration cannot be zero.">
+        <source>Duration cannot be zero.</source>
+        <target>An empty duration is not allowed.</target>
+      </trans-unit>
       <trans-unit id="1V6BsD_" resname="You must select at least one user or team.">
         <source>You must select at least one user or team.</source>
         <target>You must select at least one user or team.</target>