Fix issue #1302 (kimai#1303)

* Fix issue #1302
jimlucas · Oct 26, 2019 · d5a5959 · d5a5959
1 parent e77a3cc
commit d5a5959
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 91 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,15 +1,15 @@
 language: php
 
 php:
-  - 5.5
   - 5.6
   - 7.0
   - 7.1
-  - 7.2
+# due to incompatible phpunit framework
+#  - 7.2
 
-matrix:
-  allow_failures:
-    - php: 7.2
+#matrix:
+#  allow_failures:
+#    - php: 7.2
 
 before_script:
   - ./.travis.install.sh

diff --git a/composer.json b/composer.json
@@ -11,7 +11,7 @@
         { "name": "Simon Schaufelberger", "homepage": "https://github.com/simonschaufi" }
     ],
     "require": {
-        "php": ">=5.5.0",
+        "php": ">=5.6.0",
         "phpoffice/phpexcel": "1.8.*",
         "tecnickcom/tcpdf": "^6.2.12",
         "tinybutstrong/tinybutstrong": "^3.10",

diff --git a/extensions/ki_timesheets/processor.php b/extensions/ki_timesheets/processor.php
@@ -624,8 +624,8 @@ function($activity) {
             $outDate = new Zend_Date($edit_out);
 
             $rounded = Kimai_Rounding::roundTimespan(
-                $inDate->getTimestamp(),
-                $outDate->getTimestamp(),
+                (int)$inDate->getTimestamp(),
+                (int)$outDate->getTimestamp(),
                 $kga->getRoundPrecisionRecorderTimes(),
                 $kga->isRoundDownRecorderTimes()
             );

diff --git a/libraries/Kimai/Rounding.php b/libraries/Kimai/Rounding.php
@@ -7,96 +7,134 @@ class Kimai_Rounding
 {
     /**
      * Find a beginning and end time whose timespan is as close to
-     * the real timepsan as possible while being a multiple of $steps (in minutes).
+     * the real timespan as possible while being a multiple of $minutes.
      *
      * e.g.: 16:07:31 - 17:15:16 is "rounded" to 16:00:00 - 17:15:00
-     *       with steps set to 15
+     *       with steps set to 15 min
      *
      * @param int $start the beginning of the timespan
-     * @param int $end   the end of the timespan
-     * @param int $steps the steps in minutes (has to divide an hour, e.g. 5 is valid while 7 is not)
-     * @param $allowRoundDown
+     * @param int $end the end of the timespan
+     * @param int $minutes the steps in minutes (has to divide an hour, e.g. 5 is valid while 7 is not)
+     * @param bool $allowRoundDown
+     *
      * @return array
      */
-    public static function roundTimespan($start, $end, $steps, $allowRoundDown)
+    public static function roundTimespan($start, $end, $minutes, $allowRoundDown)
     {
-        // calculate how long a steps is (e.g. 15 second steps are 900 seconds long)
-        $stepWidth = $steps * 60;
-
-        if ($steps == 0) {
-            $bestTime = [];
-            $bestTime['start'] = $start;
-            $bestTime['end'] = $end;
-            $bestTime['duration'] = $end - $start;
-            return $bestTime;
+        if ($allowRoundDown) {
+            return self::closestRounding($start, $end, $minutes);
         }
 
-        // calculate how many seconds we are over the previous full step
-        $startSecondsOver = $start % $stepWidth;
-        $endSecondsOver = $end % $stepWidth;
+        return self::ceilRounding($start, $end, $minutes);
+    }
 
-        // calculate earlier and later times of full step width
-        $earlierStart = $start - $startSecondsOver;
-        $earlierEnd = $end - $endSecondsOver;
-        $laterStart = $start + ($stepWidth - $startSecondsOver);
-        $laterEnd = $end + ($stepWidth - $endSecondsOver);
+    private static function closestRounding($start, $end, $minutes)
+    {
+        $roundedStart = self::getClosestRoundingStart($start, $minutes);
+        $roundedEnd = self::getClosestRoundingEnd($end, $minutes);
 
+        if ($roundedStart === null || $roundedEnd === null) {
+            return [
+                'start' => $start,
+                'end' => $end,
+            ];
+        }
 
-        // assuming the earlier start end end time are the best (likely not always true)
-        $bestTime = [];
-        $bestTime['start'] = $earlierStart;
-        $bestTime['end'] = $earlierEnd;
-        $bestTime['duration'] = $earlierEnd - $earlierStart;
-        $bestTime['totalDeviation'] = abs($start - $earlierStart) + abs($end - $earlierEnd);
+        return [
+            'start' => $roundedStart,
+            'end' => $roundedEnd,
+        ];
+    }
 
-        // check for better start and end times
-        self::roundTimespanCheckIfBetter($bestTime, $earlierStart, $laterEnd, $start, $end, $allowRoundDown);
-        self::roundTimespanCheckIfBetter($bestTime, $laterStart, $earlierEnd, $start, $end, $allowRoundDown);
-        self::roundTimespanCheckIfBetter($bestTime, $laterStart, $laterEnd, $start, $end, $allowRoundDown);
+    private static function getClosestRoundingStart($start, $minutes)
+    {
+        if ($minutes <= 0) {
+            return null;
+        }
+
+        $timestamp = $start;
+        $seconds = $minutes * 60;
+        $diff = $timestamp % $seconds;
+
+        if (0 === $diff) {
+            return null;
+        }
 
-        return $bestTime;
+        if ($diff > ($seconds / 2)) {
+            return $timestamp - $diff + $seconds;
+        }
+        return $timestamp - $diff;
     }
 
-    /**
-     * Check if the new time values are better than the old once in the array.
-     *
-     * @param $bestTime (called by reference)
-     *                  Array containing the, until now, best time data
-     * @param int $newStart suggestion for a better start time
-     * @param int $newEnd   suggestion for a better end time
-     * @param int $realStart the real start time
-     * @param int $realEnd   the real end time
-     * @param $allowRoundDown
-     */
-    private static function roundTimespanCheckIfBetter(&$bestTime, $newStart, $newEnd, $realStart, $realEnd, $allowRoundDown)
+    private static function getClosestRoundingEnd($end, $minutes)
     {
-        $realDuration = $realEnd - $realStart;
-        $newDuration = $newEnd - $newStart;
+        if ($minutes <= 0) {
+            return null;
+        }
 
-        if ($allowRoundDown) {
-            // new times are definitely worse, as the timespan is furher away from the real duration
-            if (abs($realDuration - $newDuration) > abs($realDuration - $bestTime['duration'])) {
-                return;
-            }
-
-            // still, this might be closer to the real time
-            if (abs($realStart - $newStart) + abs($realEnd - $newEnd) >= $bestTime['totalDeviation']) {
-                return;
-            }
-        } else {
-            if ($newDuration < $realDuration) {
-                return;
-            }
-
-            if ($newDuration > $bestTime['duration'] && $bestTime['duration'] > $realDuration) {
-                return;
-            }
+        $timestamp = $end;
+        $seconds = $minutes * 60;
+        $diff = $timestamp % $seconds;
+
+        if (0 === $diff) {
+            return null;
+        }
+
+        if ($diff > ($seconds / 2)) {
+            return $timestamp - $diff + $seconds;
+        }
+        return $timestamp - $diff;
+    }
+
+    private static function ceilRounding($start, $end, $minutes)
+    {
+        $roundedStart = self::getCeilRoundingStart($start, $minutes);
+        $roundedEnd = self::getCeilRoundingEnd($end, $minutes);
+
+        if ($roundedStart === null || $roundedEnd === null) {
+            return [
+                'start' => $start,
+                'end' => $end,
+            ];
+        }
+
+        return [
+            'start' => $roundedStart,
+            'end' => $roundedEnd,
+        ];
+    }
+
+    private static function getCeilRoundingStart($start, $minutes)
+    {
+        if ($minutes <= 0) {
+            return null;
+        }
+
+        $timestamp = $start;
+        $seconds = $minutes * 60;
+        $diff = $timestamp % $seconds;
+
+        if (0 === $diff) {
+            return null;
+        }
+
+        return $timestamp - $diff + $seconds;
+    }
+
+    private static function getCeilRoundingEnd($end, $minutes)
+    {
+        if ($minutes <= 0) {
+            return null;
+        }
+
+        $timestamp = $end;
+        $seconds = $minutes * 60;
+        $diff = $timestamp % $seconds;
+
+        if (0 === $diff) {
+            return null;
         }
 
-        // new time is better, update array
-        $bestTime['start'] = $newStart;
-        $bestTime['end'] = $newEnd;
-        $bestTime['duration'] = $newEnd - $newStart;
-        $bestTime['totalDeviation'] = abs($realStart - $newStart) + abs($realEnd - $newEnd);
+        return $timestamp - $diff + $seconds;
     }
 }
diff --git a/tests/library/Kimai/RoundingTest.php b/tests/library/Kimai/RoundingTest.php
@@ -26,7 +26,6 @@
  */
 class RoundingTest extends TestCase
 {
-
     /**
      * @covers ::roundTimespan
      */
@@ -39,16 +38,64 @@ public function testRoundTimespanWithStepsZero()
         $this->assertInternalType('array', $actual);
         $this->assertArrayHasKey('start', $actual);
         $this->assertArrayHasKey('end', $actual);
-        $this->assertArrayHasKey('duration', $actual);
 
-        $this->assertEquals($actual['start'], $start);
-        $this->assertEquals($actual['end'], $end);
-        $this->assertEquals($actual['duration'], $end - $start);
+        $this->assertEquals($start, $actual['start']);
+        $this->assertEquals($end, $actual['end']);
+    }
+
+    /**
+     * @covers ::roundTimespan
+     */
+    public function testRoundTimespanWithSteps15MinAndNoRoundDownOnExactStep()
+    {
+        $start = 1572109200;
+        $end = 1572110100;
+        $actual = Kimai_Rounding::roundTimespan($start, $end, 15, false);
+
+        $this->assertInternalType('array', $actual);
+        $this->assertArrayHasKey('start', $actual);
+        $this->assertArrayHasKey('end', $actual);
+
+        $this->assertEquals($start, $actual['start']);
+        $this->assertEquals($end, $actual['end']);
+    }
+
+    /**
+     * @covers ::roundTimespan
+     */
+    public function testRoundTimespanWithSteps15MinAndNoRoundDownOnAnyMinute()
+    {
+        $start = 1572109500;
+        $end = 1572110400;
+        $actual = Kimai_Rounding::roundTimespan($start, $end, 15, false);
+
+        $this->assertInternalType('array', $actual);
+        $this->assertArrayHasKey('start', $actual);
+        $this->assertArrayHasKey('end', $actual);
+
+        $this->assertEquals(1572110100, $actual['start']);
+        $this->assertEquals(1572111000, $actual['end']);
+    }
+
+    /**
+     * @covers ::roundTimespan
+     */
+    public function testRoundTimespanWithSteps15MinAndRoundDown(){
+
+        $start = 1572109200;
+        $end = 1572110100;
+        $actual = Kimai_Rounding::roundTimespan($start, $end, 15, true);
+
+        $this->assertInternalType('array', $actual);
+        $this->assertArrayHasKey('start', $actual);
+        $this->assertArrayHasKey('end', $actual);
+
+        $this->assertEquals($start, $actual['start']);
+        $this->assertEquals($end, $actual['end']);
     }
 
     /**
      * @covers ::roundTimespan
-     * @covers ::roundTimespanCheckIfBetter
      */
     public function testRoundTimespan()
     {
@@ -59,12 +106,8 @@ public function testRoundTimespan()
         $this->assertInternalType('array', $actual);
         $this->assertArrayHasKey('start', $actual);
         $this->assertArrayHasKey('end', $actual);
-        $this->assertArrayHasKey('duration', $actual);
-        $this->assertArrayHasKey('totalDeviation', $actual);
 
-        $this->assertEquals($actual['start'], 1458405900);
-        $this->assertEquals($actual['end'], 1458413100);
-        $this->assertEquals($actual['duration'], 7200);
-        $this->assertEquals($actual['totalDeviation'], 530);
+        $this->assertEquals(1458405900, $actual['start']);
+        $this->assertEquals(1458413100, $actual['end']);
     }
 }