Configurable rate rounding (#5734)

* added invoice hydration of the issuer object * added "rate calculator" mode * new config to select rounding mode * replace static calls with dependency injection
2025-12-16 16:47:46 +01:00
parent 07ac6c308f
commit 595b5f4b25
43 changed files with 876 additions and 411 deletions
--- a/tests/Timesheet/Calculator/RateCalculatorTest.php
+++ b/tests/Timesheet/Calculator/RateCalculatorTest.php
@@ -18,7 +18,7 @@ use App\Entity\ProjectRate;
 use App\Entity\Timesheet;
 use App\Entity\User;
 use App\Entity\UserPreference;
-use App\Repository\TimesheetRepository;
+use App\Tests\Mocks\RateServiceFactory;
 use App\Timesheet\Calculator\RateCalculator;
 use App\Timesheet\RateService;
 use PHPUnit\Framework\Attributes\CoversClass;
@@ -28,14 +28,11 @@ use PHPUnit\Framework\TestCase;
 #[CoversClass(RateCalculator::class)]
 class RateCalculatorTest extends TestCase
 {
-    protected function getRateRepositoryMock(array $rates = []): TimesheetRepository
+    private function getRateService(array $rules = [], array $rates = []): RateService
    {
-        $mock = $this->getMockBuilder(TimesheetRepository::class)->disableOriginalConstructor()->getMock();
-        if (!empty($rates)) {
-            $mock->expects($this->any())->method('findMatchingRates')->willReturn($rates);
-        }
+        $factory = new RateServiceFactory($this);

-        return $mock;
+        return $factory->create($rules, $rates);
    }

    private function assertRateByTimesheetHourlyRate(int $duration, float $hourlyRate, float $rate): void
@@ -47,7 +44,7 @@ class RateCalculatorTest extends TestCase
        $record->setActivity(new Activity());
        $record->setUser($this->getTestUser());

-        $sut = new RateCalculator(new RateService([], $this->getRateRepositoryMock()));
+        $sut = new RateCalculator($this->getRateService());
        $sut->calculate($record, []);
        self::assertEquals($rate, $record->getRate());
    }
@@ -55,9 +52,9 @@ class RateCalculatorTest extends TestCase
    public function testCalculateWithTimesheetHourlyRate(): void
    {
        $this->assertRateByTimesheetHourlyRate(1800, 100, 50);
-        $this->assertRateByTimesheetHourlyRate(400, 100, 11);
-        $this->assertRateByTimesheetHourlyRate(1234, 100, 34);
-        $this->assertRateByTimesheetHourlyRate(2739, 100, 76);
+        $this->assertRateByTimesheetHourlyRate(400, 100, 11.1111);
+        $this->assertRateByTimesheetHourlyRate(1234, 100, 34.2778);
+        $this->assertRateByTimesheetHourlyRate(2739, 100, 76.0833);
    }

    public function testCalculateWithTimesheetFixedRate(): void
@@ -71,7 +68,7 @@ class RateCalculatorTest extends TestCase
        $record->setActivity(new Activity());
        $record->setUser($this->getTestUser());

-        $sut = new RateCalculator(new RateService([], $this->getRateRepositoryMock()));
+        $sut = new RateCalculator($this->getRateService());
        $sut->calculate($record, []);
        self::assertEquals(10, $record->getRate());
    }
@@ -112,22 +109,22 @@ class RateCalculatorTest extends TestCase

    #[DataProvider('getRateTestData')]
    public function testRates(
-        $expectedRate,
-        $expectedInternalRate,
-        $duration,
-        $userRate,
-        $userInternalRate,
-        $timesheetHourly,
-        $timesheetFixed,
-        $activityRate,
-        $activityInternal,
-        $activityIsFixed,
-        $projectRate,
-        $projectInternal,
-        $projectIsFixed,
-        $customerRate,
-        $customerInternal,
-        $customerIsFixed
+        float $expectedRate,
+        float $expectedInternalRate,
+        int $duration,
+        float $userRate,
+        ?float $userInternalRate,
+        ?float $timesheetHourly,
+        ?float $timesheetFixed,
+        ?float $activityRate,
+        ?float $activityInternal,
+        bool $activityIsFixed,
+        ?float $projectRate,
+        ?float $projectInternal,
+        bool $projectIsFixed,
+        ?float $customerRate,
+        ?float $customerInternal,
+        bool $customerIsFixed
    ) {
        $customer = new Customer('foo');

@@ -178,7 +175,7 @@ class RateCalculatorTest extends TestCase
            $rates[] = $rate;
        }

-        $sut = new RateCalculator(new RateService([], $this->getRateRepositoryMock($rates)));
+        $sut = new RateCalculator($this->getRateService([], $rates));
        $sut->calculate($timesheet, []);
        self::assertEquals($expectedRate, $timesheet->getRate());
        self::assertEquals($expectedInternalRate, $timesheet->getInternalRate());
@@ -207,7 +204,7 @@ class RateCalculatorTest extends TestCase

        self::assertEquals(0, $record->getRate());

-        $sut = new RateCalculator(new RateService([], $this->getRateRepositoryMock()));
+        $sut = new RateCalculator($this->getRateService());
        $sut->calculate($record, []);
        self::assertEquals(0, $record->getRate());
    }
@@ -216,7 +213,7 @@ class RateCalculatorTest extends TestCase
     * Uses the hourly rate from user_preferences to calculate the rate.
     */
    #[DataProvider('getRuleDefinitions')]
-    public function testCalculateWithRulesByUsersHourlyRate($duration, $rules, $expectedRate): void
+    public function testCalculateWithRulesByUsersHourlyRate(int $duration, array $rules, float $expectedRate): void
    {
        $end = new \DateTime('12:00:00', new \DateTimeZone('UTC'));
        $start = clone $end;
@@ -232,7 +229,7 @@ class RateCalculatorTest extends TestCase

        $record->setEnd($end);

-        $sut = new RateCalculator(new RateService($rules, $this->getRateRepositoryMock()));
+        $sut = new RateCalculator($this->getRateService($rules));
        $sut->calculate($record, []);

        self::assertEquals($expectedRate, $record->getRate());
@@ -247,7 +244,7 @@ class RateCalculatorTest extends TestCase
            [
                31837, // 31824 = 8,84
                [],
-                663
+                663.2708
            ],
            [
                31837, // 31824 = 8,84
@@ -261,7 +258,7 @@ class RateCalculatorTest extends TestCase
                        'factor' => 1.5
                    ],
                ],
-                1326 // 8,84 * 75 (see user) * 2
+                1326.5417
            ],
            [
                31837, // 31824 = 8,84
@@ -275,7 +272,7 @@ class RateCalculatorTest extends TestCase
                        'factor' => 1.5
                    ],
                ],
-                2320.5 // 75 * 8,84 * 3,5
+                2321.4479
            ],
        ];
    }
--- a/tests/Timesheet/RateCalculator/ClassicRateCalculatorTest.php
+++ b/tests/Timesheet/RateCalculator/ClassicRateCalculatorTest.php
@@ -0,0 +1,130 @@
+<?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\Timesheet\RateCalculator;
+
+use App\Timesheet\RateCalculator\ClassicRateCalculator;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(ClassicRateCalculator::class)]
+class ClassicRateCalculatorTest extends TestCase
+{
+    #[DataProvider('provideRates')]
+    public function testCalculateRate(float $hourlyRate, int $seconds, float $expected): void
+    {
+        $sut = new ClassicRateCalculator();
+
+        $result = $sut->calculateRate($hourlyRate, $seconds);
+
+        $this->assertEquals($expected, $result);
+    }
+
+    public static function provideRates(): array
+    {
+        return [
+            'zero duration' => [100.0, 0, 0.0],
+            'full hour' => [100.0, 3600, 100.0],
+            'half hour' => [100.0, 1800, 50.0],
+            'one minute with rounding' => [123.4567, 60, 2.0576],
+            'one second tiny amount' => [1.23, 1, 0.0003],
+        ];
+    }
+
+    public function testRoundDurationKeepsOriginalSeconds(): void
+    {
+        $sut = new ClassicRateCalculator();
+
+        $this->assertSame(0, $sut->roundDuration(0));
+        $this->assertSame(59, $sut->roundDuration(59));
+        $this->assertSame(3601, $sut->roundDuration(3601));
+    }
+
+    #[DataProvider('getRateCalculationData')]
+    public function testCalculateRates(float $hourlyRate, int $duration, float $expectedRate): void
+    {
+        $sut = new ClassicRateCalculator();
+        self::assertEquals($expectedRate, $sut->calculateRate($hourlyRate, $duration));
+    }
+
+    /**
+     * @return array<int, array<float, int, >>|\Generator
+     */
+    public static function getRateCalculationData()
+    {
+        yield [0, 0, 0];
+        yield [1, 100, 0.0278];
+        yield [1, 900, 0.25];
+        yield [1, 1800, 0.5];
+        yield [10000, 1, 2.7778];
+        yield [736, 123, 25.1467];
+        yield [7360, 1234, 2522.8444];
+        yield [7360.34, 1234, 2522.961];
+        yield [7360.01, 1234, 2522.8479];
+        yield [7360.99, 1234, 2523.1838];
+    }
+
+    public function testCalculateRateWithRounding(): void
+    {
+        $total = 0.00;
+        $seconds = 0;
+        $repeat = 130;
+        $sut = new ClassicRateCalculator();
+
+        for ($a = 0; $a < $repeat; $a++) {
+            $inputs = [
+                900,
+                1600,
+                4200,
+                8763,
+                3300,
+                600,
+                1300,
+                1837,
+                4217,
+                5400,
+                3283,
+                600,
+            ];
+
+            foreach ($inputs as $i) {
+                $seconds += $i;
+                $total += $sut->calculateRate(114.75, $i);
+            }
+        }
+
+        self::assertEquals(36000 * $repeat, $seconds);
+        self::assertEquals(1147.50 * $repeat, $total);
+    }
+
+    public function testDecimalDuration(): void
+    {
+        $inputs = [
+            900,
+            1600,
+            4200,
+            8763,
+            3300,
+            600,
+            1300,
+            1837,
+            4217,
+            5400,
+            3283,
+            600,
+            7200,
+        ];
+
+        $sut = new ClassicRateCalculator();
+        foreach ($inputs as $row) {
+            self::assertEquals($row, $sut->roundDuration($row));
+        }
+    }
+}
--- a/tests/Timesheet/RateCalculator/DecimalRateCalculatorTest.php
+++ b/tests/Timesheet/RateCalculator/DecimalRateCalculatorTest.php
@@ -0,0 +1,142 @@
+<?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\Timesheet\RateCalculator;
+
+use App\Timesheet\RateCalculator\DecimalRateCalculator;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(DecimalRateCalculator::class)]
+class DecimalRateCalculatorTest extends TestCase
+{
+    #[DataProvider('provideRates')]
+    public function testCalculateRate(float $hourlyRate, int $seconds, float $expected): void
+    {
+        $sut = new DecimalRateCalculator();
+
+        $result = $sut->calculateRate($hourlyRate, $seconds);
+
+        $this->assertEquals($expected, $result);
+    }
+
+    public static function provideRates(): array
+    {
+        return [
+            'zero duration' => [100.0, 0, 0.0],
+            'full hour' => [100.0, 3600, 100.0],
+            'half hour' => [100.0, 1800, 50.0],
+            'one minute with rounding' => [123.4567, 60, 2.47],
+            'one second tiny amount' => [1.23, 1, 0],
+        ];
+    }
+
+    public function testRoundDurationKeepsOriginalSeconds(): void
+    {
+        $sut = new DecimalRateCalculator();
+
+        $this->assertSame(0, $sut->roundDuration(0));
+        $this->assertSame(36, $sut->roundDuration(45));
+        $this->assertSame(72, $sut->roundDuration(59));
+        $this->assertSame(1224, $sut->roundDuration(1234));
+        $this->assertSame(3600, $sut->roundDuration(3601));
+    }
+
+    #[DataProvider('getRateCalculationData')]
+    public function testCalculateRates(float $hourlyRate, int $duration, float $expectedRate): void
+    {
+        $sut = new DecimalRateCalculator();
+        self::assertEquals($expectedRate, $sut->calculateRate($hourlyRate, $duration));
+    }
+
+    /**
+     * @return array<int, array<float, int, >>|\Generator
+     */
+    public static function getRateCalculationData()
+    {
+        yield [0.00, 0, 0.00];
+        yield [10.00, 7260, 20.2];
+        yield [1.00, 3600, 1.00];
+        yield [1.00, 100, 0.03];
+        yield [1.00, 900, 0.25];
+        yield [1.00, 1800, 0.5];
+        yield [10000.00, 60, 200.00];
+        yield [736.00, 123, 22.08];
+        yield [7360.00, 1234, 2502.4];
+        yield [7360.34, 1234, 2502.52];
+        yield [7360.01, 1234, 2502.4];
+        yield [7360.99, 1234, 2502.74];
+    }
+
+    public function testCalculateRateWithRounding(): void
+    {
+        $total = 0.00;
+        $seconds = 0;
+        $repeat = 130;
+
+        $inputs = [
+            [900, 28.69, 0],
+            [1600, 50.49, 0],
+            [4200, 134.26, 0],
+            [8763, 278.84, 0],
+            [3300, 105.57, 0],
+            [600, 19.51, 0],
+            [1300, 41.31, 0],
+            [1837, 58.52, 0],
+            [4217, 134.26, 0],
+            [5400, 172.13, 0],
+            [3283, 104.42, 0],
+            [600, 19.51, 0],
+        ];
+
+        $totalExpected = 0.00;
+        $sut = new DecimalRateCalculator();
+
+        for ($a = 0; $a < $repeat; $a++) {
+            foreach ($inputs as $row) {
+                [$duration, $rate] = $row;
+                $seconds += $duration;
+                $totalExpected += $rate;
+                $tmp = $sut->calculateRate(114.75, $duration);
+                self::assertEquals($rate, $tmp);
+                $total += $tmp;
+            }
+        }
+
+        self::assertEquals(36000 * $repeat, $seconds);
+        self::assertEquals($totalExpected, $total);
+        self::assertEqualsWithDelta(1147.51 * $repeat, $total, 0.00001);
+        self::assertEqualsWithDelta(149176.3, $total, 0.00001);
+    }
+
+    public function testDecimalDuration(): void
+    {
+        $inputs = [
+            [900, 900],
+            [1600, 1584],
+            [4200, 4212],
+            [8763, 8748],
+            [3300, 3312],
+            [600, 612],
+            [1300, 1296],
+            [1837, 1836],
+            [4217, 4212],
+            [5400, 5400],
+            [3283, 3276],
+            [600, 612],
+            [7200, 7200],
+        ];
+
+        $sut = new DecimalRateCalculator();
+        foreach ($inputs as $row) {
+            self::assertEquals($row[1], $sut->roundDuration($row[0]));
+        }
+    }
+}
--- a/tests/Timesheet/RateCalculator/RateCalculatorFactoryTest.php
+++ b/tests/Timesheet/RateCalculator/RateCalculatorFactoryTest.php
@@ -0,0 +1,48 @@
+<?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\Timesheet\RateCalculator;
+
+use App\Tests\Mocks\SystemConfigurationFactory;
+use App\Timesheet\RateCalculator\ClassicRateCalculator;
+use App\Timesheet\RateCalculator\DecimalRateCalculator;
+use App\Timesheet\RateCalculator\RateCalculatorFactory;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(RateCalculatorFactory::class)]
+class RateCalculatorFactoryTest extends TestCase
+{
+    private function assertCreatesClassicConfig(array $config): void
+    {
+        $config = SystemConfigurationFactory::createStub($config);
+        $sut = new RateCalculatorFactory($config);
+
+        $mode = $sut->getRateCalculatorMode();
+        self::assertInstanceOf(ClassicRateCalculator::class, $mode);
+    }
+
+    public function testCreatesClassic(): void
+    {
+        $this->assertCreatesClassicConfig([]);
+        $this->assertCreatesClassicConfig(['invoice' => ['rounding_mode' => 'classic']]);
+        $this->assertCreatesClassicConfig(['invoice' => ['rounding_mode' => '']]);
+        $this->assertCreatesClassicConfig(['invoice' => ['rounding_mode' => 'foo']]);
+        $this->assertCreatesClassicConfig(['invoice' => ['rounding_mode' => 'DECIMAL']]);
+    }
+
+    public function testCreateWithDecimalConfig(): void
+    {
+        $config = SystemConfigurationFactory::createStub(['invoice' => ['rounding_mode' => 'decimal']]);
+        $sut = new RateCalculatorFactory($config);
+
+        $mode = $sut->getRateCalculatorMode();
+        self::assertInstanceOf(DecimalRateCalculator::class, $mode);
+    }
+}
--- a/tests/Timesheet/RateServiceTest.php
+++ b/tests/Timesheet/RateServiceTest.php
@@ -18,7 +18,7 @@ use App\Entity\ProjectRate;
 use App\Entity\Timesheet;
 use App\Entity\User;
 use App\Entity\UserPreference;
-use App\Repository\TimesheetRepository;
+use App\Tests\Mocks\RateServiceFactory;
 use App\Timesheet\RateService;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
@@ -27,14 +27,11 @@ use PHPUnit\Framework\TestCase;
 #[CoversClass(RateService::class)]
 class RateServiceTest extends TestCase
 {
-    protected function getRateRepositoryMock(array $rates = []): TimesheetRepository
+    private function getSut(array $rules = [], array $rates = []): RateService
    {
-        $mock = $this->getMockBuilder(TimesheetRepository::class)->disableOriginalConstructor()->getMock();
-        if (!empty($rates)) {
-            $mock->expects($this->any())->method('findMatchingRates')->willReturn($rates);
-        }
+        $factory = new RateServiceFactory($this);

-        return $mock;
+        return $factory->create($rules, $rates);
    }

    private static function createDateTime(?string $datetime = null): \DateTime
@@ -51,7 +48,7 @@ class RateServiceTest extends TestCase
        $record->setActivity(new Activity());
        $record->setUser($this->getTestUser());

-        $sut = new RateService([], $this->getRateRepositoryMock());
+        $sut = $this->getSut();
        $rate = $sut->calculate($record);
        self::assertEquals(50, $rate->getRate());
    }
@@ -67,7 +64,7 @@ class RateServiceTest extends TestCase
        $record->setActivity(new Activity());
        $record->setUser($this->getTestUser());

-        $sut = new RateService([], $this->getRateRepositoryMock());
+        $sut = $this->getSut();
        $rate = $sut->calculate($record);
        self::assertEquals(10, $rate->getRate());
    }
@@ -108,22 +105,22 @@ class RateServiceTest extends TestCase

    #[DataProvider('getRateTestData')]
    public function testRates(
-        $expectedRate,
-        $expectedInternalRate,
-        $duration,
-        $userRate,
-        $userInternalRate,
-        $timesheetHourly,
-        $timesheetFixed,
-        $activityRate,
-        $activityInternal,
-        $activityIsFixed,
-        $projectRate,
-        $projectInternal,
-        $projectIsFixed,
-        $customerRate,
-        $customerInternal,
-        $customerIsFixed
+        float $expectedRate,
+        float $expectedInternalRate,
+        int $duration,
+        float $userRate,
+        ?float $userInternalRate,
+        ?float $timesheetHourly,
+        ?float $timesheetFixed,
+        ?float $activityRate,
+        ?float $activityInternal,
+        bool $activityIsFixed,
+        ?float $projectRate,
+        ?float $projectInternal,
+        bool $projectIsFixed,
+        ?float $customerRate,
+        ?float $customerInternal,
+        bool $customerIsFixed
    ): void {
        $customer = new Customer('foo');

@@ -174,7 +171,7 @@ class RateServiceTest extends TestCase
            $rates[] = $rate;
        }

-        $sut = new RateService([], $this->getRateRepositoryMock($rates));
+        $sut = $this->getSut([], $rates);
        $rate = $sut->calculate($timesheet);
        self::assertEquals($expectedRate, $rate->getRate());
        self::assertEquals($expectedInternalRate, $rate->getInternalRate());
@@ -203,7 +200,7 @@ class RateServiceTest extends TestCase

        self::assertEquals(0, $record->getRate());

-        $sut = new RateService([], $this->getRateRepositoryMock());
+        $sut = $this->getSut();
        $rate = $sut->calculate($record);
        self::assertEquals(0, $rate->getRate());
    }
@@ -212,7 +209,7 @@ class RateServiceTest extends TestCase
     * Uses the hourly rate from user_preferences to calculate the rate.
     */
    #[DataProvider('getRuleDefinitions')]
-    public function testCalculateWithRulesByUsersHourlyRate($duration, $rules, $expectedRate): void
+    public function testCalculateWithRulesByUsersHourlyRate(int $duration, array $rules, float $expectedRate): void
    {
        $end = self::createDateTime('12:00:00');
        $start = clone $end;
@@ -228,7 +225,7 @@ class RateServiceTest extends TestCase

        $record->setEnd($end);

-        $sut = new RateService($rules, $this->getRateRepositoryMock());
+        $sut = $this->getSut($rules);
        $rate = $sut->calculate($record);

        self::assertEquals($expectedRate, $rate->getRate());
@@ -243,7 +240,7 @@ class RateServiceTest extends TestCase
            [
                31837,
                [],
-                663
+                663.2708
            ],
            [
                31837,
@@ -257,7 +254,7 @@ class RateServiceTest extends TestCase
                        'factor' => 1.5
                    ],
                ],
-                1326 // 8,84 * 75 (see user) * 2
+                1326.5417
            ],
            [
                31837,
@@ -271,7 +268,7 @@ class RateServiceTest extends TestCase
                        'factor' => 1.5
                    ],
                ],
-                2320.5 // 75 * 8,84 * 3,5
+                2321.4479
            ],
        ];
    }
--- a/tests/Timesheet/UtilTest.php
+++ b/tests/Timesheet/UtilTest.php
@@ -12,11 +12,14 @@ namespace App\Tests\Timesheet;
 use App\Timesheet\Util;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\TestCase;

+#[Group('legacy')]
 #[CoversClass(Util::class)]
 class UtilTest extends TestCase
 {
+    #[Group('legacy')]
    #[DataProvider('getRateCalculationData')]
    public function testCalculateRate(float $hourlyRate, int $duration, float $expectedRate): void
    {
@@ -42,6 +45,7 @@ class UtilTest extends TestCase
        yield [7360.99, 1234, 2502.74];
    }

+    #[Group('legacy')]
    public function testCalculateRateWithRounding(): void
    {
        $total = 0.00;
@@ -82,6 +86,7 @@ class UtilTest extends TestCase
        self::assertEqualsWithDelta(149176.3, $total, 0.00001);
    }

+    #[Group('legacy')]
    public function testDecimalDuration(): void
    {
        $inputs = [