From 8c686b07142e90d3bf4ca7c749bf482a36b78031 Mon Sep 17 00:00:00 2001
From: Kevin Papst <kevinpapst@users.noreply.github.com>
Date: Wed, 20 Jun 2018 23:27:56 +0200
Subject: [PATCH] Added validation for end date before start date (#167)

* added validation for end date before start date
* added unit test for new validations
---
 src/Entity/Timesheet.php         |  18 +++++
 tests/Entity/TimesheetTest.php   | 128 +++++++++++++++++++++++++++++++
 translations/validators.de.xliff |   4 +
 translations/validators.en.xliff |   4 +
 translations/validators.ru.xliff |   4 +
 5 files changed, 158 insertions(+)
 create mode 100644 tests/Entity/TimesheetTest.php

diff --git a/src/Entity/Timesheet.php b/src/Entity/Timesheet.php
index 5d5e29a3..f13304e4 100644
--- a/src/Entity/Timesheet.php
+++ b/src/Entity/Timesheet.php
@@ -12,6 +12,7 @@ namespace App\Entity;
 use App\Entity\User;
 use Doctrine\ORM\Mapping as ORM;
 use Symfony\Component\Validator\Constraints as Assert;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 
 /**
  * Timesheet entity.
@@ -57,6 +58,7 @@ class Timesheet
      * @var integer
      *
      * @ORM\Column(name="duration", type="integer", nullable=true)
+     * @Assert\GreaterThanOrEqual(0)
      */
     private $duration = 0;
 
@@ -252,4 +254,20 @@ class Timesheet
     {
         return $this->rate;
     }
+
+    /**
+     * @param ExecutionContextInterface $context
+     * @param mixed $payload
+     *
+     * @Assert\Callback
+     */
+    public function validate(ExecutionContextInterface $context, $payload)
+    {
+        if ($this->getEnd() !== null && $this->getEnd()->getTimestamp() < $this->getBegin()->getTimestamp()) {
+            $context->buildViolation('End date must not be earlier then start date.')
+                ->atPath('end')
+                ->setTranslationDomain('validators')
+                ->addViolation();
+        }
+    }
 }
diff --git a/tests/Entity/TimesheetTest.php b/tests/Entity/TimesheetTest.php
new file mode 100644
index 00000000..1ca30409
--- /dev/null
+++ b/tests/Entity/TimesheetTest.php
@@ -0,0 +1,128 @@
+<?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\Twig;
+
+use App\Entity\Activity;
+use App\Entity\Customer;
+use App\Entity\Project;
+use App\Entity\Timesheet;
+use App\Entity\User;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Validator\ConstraintViolationInterface;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
+use Symfony\Component\Validator\Validation;
+
+/**
+ * @covers \App\Entity\Timesheet
+ */
+class TimesheetTest extends TestCase
+{
+
+    protected function getEntity()
+    {
+        $customer = new Customer();
+        $customer->setName('Test Customer');
+
+        $project = new Project();
+        $project->setName('Test Project');
+        $project->setCustomer($customer);
+
+        $activity = new Activity();
+        $activity->setName('Test');
+        $activity->setProject($project);
+
+        $entity = new Timesheet();
+        $entity->setUser(new User());
+        $entity->setActivity($activity);
+
+        return $entity;
+    }
+
+    public function testValidationEndNotEarlierThanBegin()
+    {
+        $entity = $this->getEntity();
+        $begin = new \DateTime();
+        $end = clone $begin;
+        $end = $end->modify('-1 second');
+        $entity->setBegin($begin);
+        $entity->setEnd($end);
+
+        $this->assertHasViolationForField($entity, 'end');
+
+        // allow same begin and end
+        $entity = $this->getEntity();
+        $begin = new \DateTime();
+        $end = clone $begin;
+        $entity->setBegin($begin);
+        $entity->setEnd($end);
+
+        $this->assertHasViolationForField($entity, []);
+    }
+
+    public function testDurationMustBeGreatorOrEqualThanZero()
+    {
+        $entity = $this->getEntity();
+        $begin = new \DateTime();
+        $end = clone $begin;
+        $entity->setBegin($begin);
+        $entity->setEnd($end);
+        $entity->setDuration(-1);
+
+        $this->assertHasViolationForField($entity, 'duration');
+
+        // allow zero duration
+        $entity = $this->getEntity();
+        $begin = new \DateTime();
+        $end = clone $begin;
+        $entity->setBegin($begin);
+        $entity->setEnd($end);
+        $entity->setDuration(0);
+
+        $this->assertHasViolationForField($entity, []);
+    }
+
+    /**
+     * @param $value
+     * @param array|string $fieldNames
+     */
+    protected function assertHasViolationForField($value, $fieldNames)
+    {
+        $validator = Validation::createValidatorBuilder()->enableAnnotationMapping()->getValidator();
+        $validations = $validator->validate($value);
+
+        if (!is_array($fieldNames)) {
+            $fieldNames = [$fieldNames];
+        }
+
+        $violatedFields = [];
+        /** @var ConstraintViolationInterface $validation */
+        foreach($validations as $validation) {
+            $violatedFields[] = $validation->getPropertyPath();
+        }
+
+
+        foreach($fieldNames as $id => $propertyPath) {
+            $foundField = false;
+            if (in_array($propertyPath, $violatedFields)) {
+                $foundField = true;
+                unset($violatedFields[$id]);
+            }
+
+            $this->assertTrue($foundField, 'Failed finding violation for field: ' . $propertyPath);
+        }
+
+        $this->assertEmpty($violatedFields, sprintf('Unexpected violations found: %s', implode(', ', $violatedFields)));
+
+        $expected = count($fieldNames);
+        $actual = $validations->count();
+
+        $this->assertEquals($expected, $actual, sprintf('Expected %s violations, found %s.', $expected, $actual));
+    }
+}
diff --git a/translations/validators.de.xliff b/translations/validators.de.xliff
index 34d566cf..e742ccee 100644
--- a/translations/validators.de.xliff
+++ b/translations/validators.de.xliff
@@ -6,6 +6,10 @@
                 <source>This value is not a valid role.</source>
                 <target>Dieser Wert ist keine gültige Rolle.</target>
             </trans-unit>
+            <trans-unit id="End date must not be earlier then start date.">
+                <source>End date must not be earlier then start date.</source>
+                <target>Das Enddatum darf nicht vor dem Startdatum liegen.</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>
diff --git a/translations/validators.en.xliff b/translations/validators.en.xliff
index 9c1b1473..b37d2bb1 100644
--- a/translations/validators.en.xliff
+++ b/translations/validators.en.xliff
@@ -6,6 +6,10 @@
                 <source>This value is not a valid role.</source>
                 <target>This value is not a valid role.</target>
             </trans-unit>
+            <trans-unit id="End date must not be earlier then start date.">
+                <source>End date must not be earlier then start date.</source>
+                <target>End date must not be earlier then start date.</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>
diff --git a/translations/validators.ru.xliff b/translations/validators.ru.xliff
index 2c530994..43a3df73 100644
--- a/translations/validators.ru.xliff
+++ b/translations/validators.ru.xliff
@@ -6,6 +6,10 @@
                 <source>This value is not a valid role.</source>
                 <target>Это значение - недействительная задача.</target>
             </trans-unit>
+            <trans-unit id="End date must not be earlier then start date.">
+                <source>End date must not be earlier then start date.</source>
+                <target>End date must not be earlier then start date.</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>