From 661c3897b0ee0ab5d38e1260b1103341da55365e Mon Sep 17 00:00:00 2001
From: Kevin Papst <kevinpapst@users.noreply.github.com>
Date: Mon, 17 Aug 2020 00:18:51 +0200
Subject: [PATCH] new form type to select a daytime in system-configuration
 (#1895)

---
 .../SystemConfigurationController.php         | 40 +++++----
 src/Form/Type/DayTimeType.php                 | 37 ++++++++
 src/Validator/Constraints/TimeFormat.php      | 28 ++++++
 .../Constraints/TimeFormatValidator.php       | 43 +++++++++
 .../SystemConfigurationControllerTest.php     |  2 +
 .../Constraints/TimeFormatValidatorTest.php   | 89 +++++++++++++++++++
 translations/validators.de.xlf                |  4 +
 translations/validators.en.xlf                |  4 +
 8 files changed, 230 insertions(+), 17 deletions(-)
 create mode 100644 src/Form/Type/DayTimeType.php
 create mode 100644 src/Validator/Constraints/TimeFormat.php
 create mode 100644 src/Validator/Constraints/TimeFormatValidator.php
 create mode 100644 tests/Validator/Constraints/TimeFormatValidatorTest.php

diff --git a/src/Controller/SystemConfigurationController.php b/src/Controller/SystemConfigurationController.php
index f39ba17f..a9422f4e 100644
--- a/src/Controller/SystemConfigurationController.php
+++ b/src/Controller/SystemConfigurationController.php
@@ -15,6 +15,7 @@ use App\Form\Model\Configuration;
 use App\Form\Model\SystemConfiguration as SystemConfigurationModel;
 use App\Form\SystemConfigurationForm;
 use App\Form\Type\DateTimeTextType;
+use App\Form\Type\DayTimeType;
 use App\Form\Type\LanguageType;
 use App\Form\Type\RoundingModeType;
 use App\Form\Type\SkinType;
@@ -23,6 +24,7 @@ use App\Form\Type\WeekDaysType;
 use App\Form\Type\YesNoType;
 use App\Repository\ConfigurationRepository;
 use App\Validator\Constraints\DateTimeFormat;
+use App\Validator\Constraints\TimeFormat;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
@@ -34,8 +36,8 @@ use Symfony\Component\Form\Extension\Core\Type\TimezoneType;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
-use Symfony\Component\Validator\Constraints\DateTime;
 use Symfony\Component\Validator\Constraints\GreaterThanOrEqual;
+use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\NotNull;
 use Symfony\Component\Validator\Constraints\Regex;
 
@@ -136,15 +138,19 @@ final class SystemConfigurationController extends AbstractController
         $form = $this->createConfigurationsForm($configModel);
         $form->handleRequest($request);
 
-        if ($form->isSubmitted() && $form->isValid()) {
-            try {
-                $this->repository->saveSystemConfiguration($form->getData());
-                $this->flashSuccess('action.update.success');
-            } catch (\Exception $ex) {
-                $this->flashError('action.update.error', ['%reason%' => $ex->getMessage()]);
-            }
+        if ($form->isSubmitted()) {
+            if ($form->isValid()) {
+                try {
+                    $this->repository->saveSystemConfiguration($form->getData());
+                    $this->flashSuccess('action.update.success');
+                } catch (\Exception $ex) {
+                    $this->flashError('action.update.error', ['%reason%' => $ex->getMessage()]);
+                }
 
-            return $this->redirectToRoute('system_configuration');
+                return $this->redirectToRoute('system_configuration');
+            } else {
+                $this->flashError('action.update.error', ['%reason%' => 'Validation problem']);
+            }
         }
 
         $configSettings = $this->getInitializedConfigurations();
@@ -393,23 +399,23 @@ final class SystemConfigurationController extends AbstractController
                     (new Configuration())
                         ->setName('calendar.businessHours.begin')
                         ->setTranslationDomain('system-configuration')
-                        ->setType(TextType::class)
-                        ->setConstraints([new DateTime(['format' => 'H:i']), new NotNull()]),
+                        ->setType(DayTimeType::class)
+                        ->setConstraints([new NotBlank(), new TimeFormat()]),
                     (new Configuration())
                         ->setName('calendar.businessHours.end')
                         ->setTranslationDomain('system-configuration')
-                        ->setType(TextType::class)
-                        ->setConstraints([new DateTime(['format' => 'H:i']), new NotNull()]),
+                        ->setType(DayTimeType::class)
+                        ->setConstraints([new NotBlank(), new TimeFormat()]),
                     (new Configuration())
                         ->setName('calendar.visibleHours.begin')
                         ->setTranslationDomain('system-configuration')
-                        ->setType(TextType::class)
-                        ->setConstraints([new DateTime(['format' => 'H:i']), new NotNull()]),
+                        ->setType(DayTimeType::class)
+                        ->setConstraints([new NotBlank(), new TimeFormat()]),
                     (new Configuration())
                         ->setName('calendar.visibleHours.end')
                         ->setTranslationDomain('system-configuration')
-                        ->setType(TextType::class)
-                        ->setConstraints([new DateTime(['format' => 'H:i']), new NotNull()]),
+                        ->setType(DayTimeType::class)
+                        ->setConstraints([new NotBlank(), new TimeFormat()]),
                     (new Configuration())
                         ->setName('calendar.slot_duration')
                         ->setTranslationDomain('system-configuration')
diff --git a/src/Form/Type/DayTimeType.php b/src/Form/Type/DayTimeType.php
new file mode 100644
index 00000000..aface0db
--- /dev/null
+++ b/src/Form/Type/DayTimeType.php
@@ -0,0 +1,37 @@
+<?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\Form\Type;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+class DayTimeType extends AbstractType
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefaults([
+            'attr' => [
+                'placeholder' => 'hh:mm'
+            ],
+        ]);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getParent()
+    {
+        return TextType::class;
+    }
+}
diff --git a/src/Validator/Constraints/TimeFormat.php b/src/Validator/Constraints/TimeFormat.php
new file mode 100644
index 00000000..f5f03812
--- /dev/null
+++ b/src/Validator/Constraints/TimeFormat.php
@@ -0,0 +1,28 @@
+<?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 Symfony\Component\Validator\Constraint;
+
+class TimeFormat extends Constraint
+{
+    public const INVALID_FORMAT = 'kimai-time-00';
+
+    protected static $errorNames = [
+        self::INVALID_FORMAT => 'The given value is not a valid time.',
+    ];
+
+    public $message = 'This time format is invalid.';
+
+    public function getTargets()
+    {
+        return self::PROPERTY_CONSTRAINT;
+    }
+}
diff --git a/src/Validator/Constraints/TimeFormatValidator.php b/src/Validator/Constraints/TimeFormatValidator.php
new file mode 100644
index 00000000..b6c2cbf7
--- /dev/null
+++ b/src/Validator/Constraints/TimeFormatValidator.php
@@ -0,0 +1,43 @@
+<?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 Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Exception\UnexpectedValueException;
+
+class TimeFormatValidator extends ConstraintValidator
+{
+    /**
+     * @param string|mixed $value
+     * @param Constraint $constraint
+     */
+    public function validate($value, Constraint $constraint)
+    {
+        if (!($constraint instanceof TimeFormat)) {
+            throw new UnexpectedTypeException($constraint, __NAMESPACE__ . '\TimeFormat');
+        }
+
+        if (!is_scalar($value) && !(\is_object($value) && method_exists($value, '__toString'))) {
+            throw new UnexpectedValueException($value, 'string');
+        }
+
+        $value = (string) $value;
+
+        if (preg_match('/^([01][0-9]|2[0-3]):([0-5][0-9])$/', $value) !== 1) {
+            $this->context->buildViolation('The given value is not a valid time.')
+                ->setTranslationDomain('validators')
+                ->setParameter('{{ value }}', $this->formatValue($value))
+                ->setCode(TimeFormat::INVALID_FORMAT)
+                ->addViolation();
+        }
+    }
+}
diff --git a/tests/Controller/SystemConfigurationControllerTest.php b/tests/Controller/SystemConfigurationControllerTest.php
index cd1180a8..dc5ffb7c 100644
--- a/tests/Controller/SystemConfigurationControllerTest.php
+++ b/tests/Controller/SystemConfigurationControllerTest.php
@@ -353,8 +353,10 @@ class SystemConfigurationControllerTest extends ControllerBaseTest
             [
                 '#system_configuration_form_calendar_configuration_2_value',
                 '#system_configuration_form_calendar_configuration_3_value',
+                '#system_configuration_form_calendar_configuration_3_value',
                 '#system_configuration_form_calendar_configuration_4_value',
                 '#system_configuration_form_calendar_configuration_5_value',
+                '#system_configuration_form_calendar_configuration_5_value',
             ],
             true
         );
diff --git a/tests/Validator/Constraints/TimeFormatValidatorTest.php b/tests/Validator/Constraints/TimeFormatValidatorTest.php
new file mode 100644
index 00000000..c7e8d92f
--- /dev/null
+++ b/tests/Validator/Constraints/TimeFormatValidatorTest.php
@@ -0,0 +1,89 @@
+<?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\Validator\Constraints\TimeFormat;
+use App\Validator\Constraints\TimeFormatValidator;
+use Symfony\Component\Validator\Constraints\NotBlank;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Exception\UnexpectedValueException;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+/**
+ * @covers \App\Validator\Constraints\TimeFormatValidator
+ */
+class TimeFormatValidatorTest extends ConstraintValidatorTestCase
+{
+    protected function createValidator()
+    {
+        return new TimeFormatValidator();
+    }
+
+    public function testConstraintIsInvalid()
+    {
+        $this->expectException(UnexpectedTypeException::class);
+
+        $this->validator->validate('foo', new NotBlank());
+    }
+
+    public function testWrongValueThrowsException()
+    {
+        $this->expectException(UnexpectedValueException::class);
+        $this->expectExceptionMessage('Expected argument of type "string", "stdClass" given');
+
+        $this->validator->validate(new \stdClass(), new TimeFormat());
+    }
+
+    /**
+     * @dataProvider getValidTimes
+     */
+    public function testValidationSucceeds(string $value)
+    {
+        $this->validator->validate($value, new TimeFormat());
+        $this->assertNoViolation();
+    }
+
+    public function getValidTimes()
+    {
+        return [
+            ['00:00'],
+            ['00:01'],
+            ['23:00'],
+            ['23:10'],
+            ['23:01'],
+            ['23:59'],
+        ];
+    }
+
+    /**
+     * @dataProvider getInvalidTimes
+     */
+    public function testValidationProblem(string $value)
+    {
+        $this->validator->validate($value, new TimeFormat());
+
+        $this->buildViolation('The given value is not a valid time.')
+            ->setParameter('{{ value }}', '"' . $value . '"')
+            ->setCode(TimeFormat::INVALID_FORMAT)
+            ->assertRaised();
+    }
+
+    public function getInvalidTimes()
+    {
+        return [
+            ['1:00'],
+            ['01:1'],
+            ['00:60'],
+            ['23:60'],
+            ['23:1'],
+            ['24:00'],
+        ];
+    }
+}
diff --git a/translations/validators.de.xlf b/translations/validators.de.xlf
index 0c1cdafd..e8ec463e 100644
--- a/translations/validators.de.xlf
+++ b/translations/validators.de.xlf
@@ -30,6 +30,10 @@
                 <source>You already have an entry for this time.</source>
                 <target>Es existiert bereits ein Eintrag für diesen Zeitpunkt.</target>
             </trans-unit>
+            <trans-unit id="The given value is not a valid time.">
+                <source>The given value is not a valid time.</source>
+                <target>Der eingetragene Wert ist keine gültige Uhrzeit.</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>
diff --git a/translations/validators.en.xlf b/translations/validators.en.xlf
index c4259aca..43e3a533 100644
--- a/translations/validators.en.xlf
+++ b/translations/validators.en.xlf
@@ -30,6 +30,10 @@
                 <source>You already have an entry for this time.</source>
                 <target>You already have an entry for this time.</target>
             </trans-unit>
+            <trans-unit id="The given value is not a valid time.">
+                <source>The given value is not a valid time.</source>
+                <target>The given value is not a valid time.</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>