allow negative duration via internal API (#3573)

src/Entity/Timesheet.php

@@ -171,7 +171,6 @@ class Timesheet implements EntityWithMetaFields, ExportItemInterface
      * @Serializer\Groups({"Default"})
      *
      * @ORM\Column(name="duration", type="integer", nullable=true)
-     * @Assert\GreaterThanOrEqual(0)
      */
     private $duration = 0;
     /**

src/Utils/Duration.php

@@ -117,10 +117,6 @@ class Duration
                 throw new \InvalidArgumentException(sprintf('Unsupported duration format "%s"', $mode));
         }
 
-        if ($seconds < 0) {
-            return 0;
-        }
-
         return $seconds;
     }
 
@@ -155,13 +151,15 @@ class Duration
             );
         }
 
+        $i = 0;
         foreach ($parts as $part) {
             if (\strlen($part) === 0) {
                 throw new \InvalidArgumentException(
                     sprintf('Colon format cannot parse "%s"', $duration)
                 );
             }
-            if (((int) $part) < 0) {
+            // the entire time could be negative
+            if ($i++ > 0 && ((int) $part) < 0) {
                 throw new \InvalidArgumentException(
                     sprintf('Negative input is not allowed in "%s"', $duration)
                 );
@@ -175,7 +173,11 @@ class Duration
         }
 
         $seconds += (int) $parts[1] * 60;
-        $seconds += (int) $parts[0] * 3600;
+        $seconds += abs((int) $parts[0] * 3600);
+
+        if ($duration[0] === '-') {
+            $seconds = $seconds * -1;
+        }
 
         return $seconds;
     }

src/Utils/LocaleHelper.php

@@ -51,9 +51,6 @@ final class LocaleHelper
     public function durationDecimal(int $seconds)
     {
         $value = round($seconds / 3600, 2);
-        if ($value <= 0) {
-            $value = 0;
-        }
 
         return $this->getDurationFormatter()->format((float) $value);
     }

src/Validator/Constraints/Duration.php

@@ -21,11 +21,12 @@ class Duration extends Regex
     {
         $patterns = [
             // decimal times (can be separated by comma or dot, depending on the locale)
-            '[0-9]{1,}',
+            // negative times -? are allowed, because plugins could allow negative times
-            '[0-9]{1,}[,.]{1}[0-9]{1,}',
+            '-?[0-9]{1,}',
+            '-?[0-9]{1,}[,.]{1}[0-9]{1,}',
             // ASP.NET style time spans - https://momentjs.com/docs/#/durations/
-            '[0-9]{1,}:[0-9]{1,}:[0-9]{1,}',
+            '-?[0-9]{1,}:[0-9]{1,}:[0-9]{1,}',
-            '[0-9]{1,}:[0-9]{1,}',
+            '-?[0-9]{1,}:[0-9]{1,}',
             // https://en.wikipedia.org/wiki/ISO_8601#Time_intervals
             '[0-9]{1,}[hHmMsS]{1}',
             '[0-9]{1,}[hH]{1}[0-9]{1,}[mM]{1}',

src/Validator/Constraints/TimesheetBasicValidator.php

@@ -132,9 +132,6 @@ final class TimesheetBasicValidator extends ConstraintValidator
                 ->addViolation();
         }
 
-        $pathStart = 'begin';
-        $pathEnd = 'end';
-
         $projectBegin = $project->getStart();
         $projectEnd = $project->getEnd();
 
@@ -142,10 +139,13 @@ final class TimesheetBasicValidator extends ConstraintValidator
             return;
         }
 
+        $pathStart = 'begin';
+        $pathEnd = 'end';
+
         $timesheetStart = $timesheet->getBegin();
         $timesheetEnd = $timesheet->getEnd();
 
-        if (null !== $timesheetStart && $pathStart !== null) {
+        if (null !== $timesheetStart) {
             if (null !== $projectBegin && $timesheetStart->getTimestamp() < $projectBegin->getTimestamp()) {
                 $context->buildViolation('The project has not started at that time.')
                     ->atPath($pathStart)
@@ -161,7 +161,7 @@ final class TimesheetBasicValidator extends ConstraintValidator
             }
         }
 
-        if (null !== $timesheetEnd && $pathEnd !== null) {
+        if (null !== $timesheetEnd) {
             if (null !== $projectEnd && $timesheetEnd->getTimestamp() > $projectEnd->getTimestamp()) {
                 $context->buildViolation('The project is finished at that time.')
                     ->atPath($pathEnd)

tests/Entity/TimesheetValidationTest.php

@@ -246,26 +246,4 @@ class TimesheetValidationTest extends KernelTestCase
 
         $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, []);
-    }
 }

tests/Twig/LocaleFormatExtensionsTest.php

@@ -520,12 +520,23 @@ class LocaleFormatExtensionsTest extends TestCase
 
         // test negative duration
         $sut = $this->getSut($this->localeEn, 'en');
-        $this->assertEquals('0.00', $sut->durationDecimal(-1));
+        $this->assertEquals('-0.00', $sut->durationDecimal(-1));
+
+        // test negative duration
+        $sut = $this->getSut($this->localeEn, 'en');
+        $this->assertEquals('-0.01', $sut->durationDecimal(-50));
+
+        // test negative duration - with rounding issue
+        $sut = $this->getSut($this->localeEn, 'en');
+        $this->assertEquals('-0.02', $sut->durationDecimal(-60));
 
         // test zero duration
         $sut = $this->getSut($this->localeEn, 'en');
         $this->assertEquals('0.00', $sut->durationDecimal(0));
 
+        $sut = $this->getSut($this->localeEn, 'en');
+        $this->assertEquals('0.00', $sut->durationDecimal(-0));
+
         $sut = $this->getSut($this->localeEn, 'en');
 
         $this->assertEquals('0.00', $sut->durationDecimal(null));

tests/Utils/DurationTest.php

@@ -31,11 +31,11 @@ class DurationTest extends TestCase
     public function testParseDurationStringSpecials()
     {
         $sut = new Duration();
-        $this->assertEquals(0, $sut->parseDuration('-1', Duration::FORMAT_SECONDS));
+        $this->assertEquals(-1, $sut->parseDuration('-1', Duration::FORMAT_SECONDS));
         $this->assertEquals(0, $sut->parseDuration('0', Duration::FORMAT_SECONDS));
         $this->assertEquals(3600, $sut->parseDuration('3600', Duration::FORMAT_SECONDS));
         $this->assertEquals(0, $sut->parseDuration('', Duration::FORMAT_SECONDS));
-        $this->assertEquals(0, $sut->parseDuration('-12', Duration::FORMAT_SECONDS));
+        $this->assertEquals(-12, $sut->parseDuration('-12', Duration::FORMAT_SECONDS));
     }
 
     /**
@@ -64,6 +64,8 @@ class DurationTest extends TestCase
             [3600, '1', Duration::FORMAT_DECIMAL],
             [5400, '1.5', Duration::FORMAT_DECIMAL],
             [5400, '1,5', Duration::FORMAT_DECIMAL],
+            [-5400, '-1.5', Duration::FORMAT_DECIMAL],
+            [-5400, '-1,5', Duration::FORMAT_DECIMAL],
 
             [0, '', Duration::FORMAT_NATURAL],
             [0, 0, Duration::FORMAT_NATURAL],
@@ -82,10 +84,12 @@ class DurationTest extends TestCase
             [12420, '3h27m', Duration::FORMAT_NATURAL],
 
             [48420, '13:27', Duration::FORMAT_COLON],
+            [-48420, '-13:27', Duration::FORMAT_COLON],
             [48474, '13:27:54', Duration::FORMAT_COLON],
             [48474, '12:87:54', Duration::FORMAT_COLON],
             [11257200, '3127:00:00', Duration::FORMAT_COLON],
             [11257200, '3127:00', Duration::FORMAT_COLON],
+            [-11257277, '-3127:01:17', Duration::FORMAT_COLON],
         ];
     }
 
@@ -108,7 +112,6 @@ class DurationTest extends TestCase
             [':3127:00', Duration::FORMAT_COLON],
             ['::3127', Duration::FORMAT_COLON],
             ['3127:-01', Duration::FORMAT_COLON],
-            ['-3127:01:17', Duration::FORMAT_COLON],
         ];
     }
 

tests/Utils/LocaleHelperTest.php

@@ -211,7 +211,9 @@ class LocaleHelperTest extends TestCase
 
         // test negative duration
         $sut = $this->getSut('en');
-        $this->assertEquals('0.00', $sut->durationDecimal(-1));
+        $this->assertEquals('-0.00', $sut->durationDecimal(-1));
+        $this->assertEquals('0.00', $sut->durationDecimal(-0));
+        $this->assertEquals('-0.01', $sut->durationDecimal(-40));
 
         // test zero duration
         $sut = $this->getSut('en');