From 0cefcf0e16385253a97704fcae626511fb1ad246 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Mon, 10 Oct 2022 17:47:11 +0200 Subject: [PATCH] allow negative duration via internal API (#3573) --- src/Entity/Timesheet.php | 1 - src/Utils/Duration.php | 14 +++++++----- src/Utils/LocaleHelper.php | 3 --- src/Validator/Constraints/Duration.php | 9 ++++---- .../Constraints/TimesheetBasicValidator.php | 10 ++++----- tests/Entity/TimesheetValidationTest.php | 22 ------------------- tests/Twig/LocaleFormatExtensionsTest.php | 13 ++++++++++- tests/Utils/DurationTest.php | 9 +++++--- tests/Utils/LocaleHelperTest.php | 4 +++- 9 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/Entity/Timesheet.php b/src/Entity/Timesheet.php index 74292f50..8dc8eeb2 100644 --- a/src/Entity/Timesheet.php +++ b/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; /** diff --git a/src/Utils/Duration.php b/src/Utils/Duration.php index eb907120..c29054a2 100644 --- a/src/Utils/Duration.php +++ b/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; } diff --git a/src/Utils/LocaleHelper.php b/src/Utils/LocaleHelper.php index 17a98558..1316e96f 100644 --- a/src/Utils/LocaleHelper.php +++ b/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); } diff --git a/src/Validator/Constraints/Duration.php b/src/Validator/Constraints/Duration.php index e96233ab..39bb1331 100644 --- a/src/Validator/Constraints/Duration.php +++ b/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,}', - '[0-9]{1,}[,.]{1}[0-9]{1,}', + // negative times -? are allowed, because plugins could allow negative times + '-?[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}', diff --git a/src/Validator/Constraints/TimesheetBasicValidator.php b/src/Validator/Constraints/TimesheetBasicValidator.php index d37ef2ad..5e45ee7e 100644 --- a/src/Validator/Constraints/TimesheetBasicValidator.php +++ b/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) diff --git a/tests/Entity/TimesheetValidationTest.php b/tests/Entity/TimesheetValidationTest.php index 04bee22a..4e234ba2 100644 --- a/tests/Entity/TimesheetValidationTest.php +++ b/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, []); - } } diff --git a/tests/Twig/LocaleFormatExtensionsTest.php b/tests/Twig/LocaleFormatExtensionsTest.php index ea5515fb..e49044c4 100644 --- a/tests/Twig/LocaleFormatExtensionsTest.php +++ b/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)); diff --git a/tests/Utils/DurationTest.php b/tests/Utils/DurationTest.php index 07653be2..13c23e59 100644 --- a/tests/Utils/DurationTest.php +++ b/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], ]; } diff --git a/tests/Utils/LocaleHelperTest.php b/tests/Utils/LocaleHelperTest.php index 0aa535b6..5c594285 100644 --- a/tests/Utils/LocaleHelperTest.php +++ b/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');