From 4523115584158d55ea22c7a9b875b6c9b5c71b5e Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Thu, 12 Mar 2026 16:09:54 +0100 Subject: [PATCH] Introduce centralized date normalization and duplicate filtering for invoicing logic, and add comprehensive unit tests --- .../app/classes/invoicing_period_utils.php | 107 ++++++++++++++++++ services/nginx/app/objects/orders_o.php | 38 ++----- .../nginx/app/routes/InvoicingPeriodRoute.php | 105 +++++++---------- ...voicingOrdersCalculationsHardeningTest.php | 29 +++++ .../InvoicingPeriodRouteGuardsTest.php | 20 ++++ .../Invoicing/InvoicingPeriodUtilsTest.php | 42 +++++++ 6 files changed, 251 insertions(+), 90 deletions(-) create mode 100644 services/nginx/app/classes/invoicing_period_utils.php create mode 100644 services/nginx/app/tests/Unit/Invoicing/InvoicingOrdersCalculationsHardeningTest.php create mode 100644 services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodRouteGuardsTest.php create mode 100644 services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodUtilsTest.php diff --git a/services/nginx/app/classes/invoicing_period_utils.php b/services/nginx/app/classes/invoicing_period_utils.php new file mode 100644 index 00000000..60f225cc --- /dev/null +++ b/services/nginx/app/classes/invoicing_period_utils.php @@ -0,0 +1,107 @@ + strtotime($dateTo)) { + throw new InvalidArgumentException('Invalid date range. dateFrom must be before or equal to dateTo'); + } + + return [ + 'dateFrom' => date('Y-m-d 00:00:00', strtotime($dateFrom)), + 'dateTo' => date('Y-m-d 23:59:59', strtotime($dateTo)), + ]; + } + + /** + * Filter grouped orders down to orders that have at least one neighbor within the time window. + * + * @param array>> $ordersByRegistration + * @return array>> + */ + public static function filterPossibleDuplicates(array $ordersByRegistration, int $windowSeconds = 86400): array + { + $possibleDuplicates = []; + + foreach ( $ordersByRegistration as $registration => $orderList ) { + if (count($orderList) < 2) { + continue; + } + + $normalizedOrders = []; + foreach ( $orderList as $order ) { + $createdAt = (string)($order['created_at'] ?? ''); + $timestamp = strtotime($createdAt); + if ($timestamp === false) { + continue; + } + $order['_timestamp'] = $timestamp; + $normalizedOrders[] = $order; + } + + if (count($normalizedOrders) < 2) { + continue; + } + + usort($normalizedOrders, function (array $a, array $b) { + return (int)$a['_timestamp'] <=> (int)$b['_timestamp']; + }); + + $duplicateIndexes = []; + $count = count($normalizedOrders); + for ( $i = 0; $i < $count; $i++ ) { + $currentTimestamp = (int)$normalizedOrders[$i]['_timestamp']; + for ( $j = $i - 1; $j >= 0; $j-- ) { + $delta = $currentTimestamp - (int)$normalizedOrders[$j]['_timestamp']; + if ($delta > $windowSeconds) { + break; + } + $duplicateIndexes[$i] = true; + $duplicateIndexes[$j] = true; + } + } + + if (count($duplicateIndexes) < 2) { + continue; + } + + $possibleDuplicates[$registration] = []; + $indexes = array_keys($duplicateIndexes); + sort($indexes); + foreach ( $indexes as $index ) { + $order = $normalizedOrders[$index]; + unset($order['_timestamp']); + $possibleDuplicates[$registration][] = $order; + } + } + + return $possibleDuplicates; + } + + private static function isValidDate(string $date): bool + { + $parsed = \DateTime::createFromFormat('Y-m-d', $date); + return $parsed !== false && $parsed->format('Y-m-d') === $date; + } +} diff --git a/services/nginx/app/objects/orders_o.php b/services/nginx/app/objects/orders_o.php index 7bcb167a..8b899569 100644 --- a/services/nginx/app/objects/orders_o.php +++ b/services/nginx/app/objects/orders_o.php @@ -5,6 +5,7 @@ namespace objects; use attachments\helpers\attachment_content; use classes\db; use classes\email; +use classes\invoicing_period_utils; use classes\pdf_generator; use classes\motorapi; use classes\object_property; @@ -619,6 +620,10 @@ class orders_o extends db */ public function getNetAmountForOrders(array $order_ids): array { + if (empty($order_ids)) { + return []; + } + $order_items = new order_items_o(); $tmp = $order_items->getFieldsWhere( [ @@ -1053,6 +1058,9 @@ class orders_o extends db public function getTransactionsForCustomersInDateRange(array $customers, string $dateFrom, string $dateTo): array { global $db; + if (empty($customers)) { + return []; + } // Validate the date range if (strtotime($dateFrom) === false || strtotime($dateTo) === false) { throw new Exception('Invalid date range provided'); @@ -1126,36 +1134,8 @@ class orders_o extends db 'object' => (new orders_o())->select((int)$tmp['id']) ]; } - // Filter out orders with more than one entry for the same registration numbers (in a 24 hour period) - $possibleDuplicates = []; - // Loop through the registration numbers - foreach ( $orders as $reg_1 => $orderList ) { - // If there are more than one order for the same registration number, add it to the possible duplicates - if (count($orderList) > 1) { - // Loop through the orders and check if they are within 24 hours of each other - $filteredOrders = []; - foreach ( $orderList as $order ) { - // Check if the order is within 24 hours of the previous order (if any) - if (empty($filteredOrders)) { - $filteredOrders[] = $order; // Add the first order - } else { - // Check if the order is within 24 hours of the previous order - $firstOrderTime = strtotime($filteredOrders[0]['created_at']); - $currentOrderTime = strtotime($order['created_at']); - if ($currentOrderTime - $firstOrderTime <= 86400) { // 86400 seconds = 24 hours - $filteredOrders[] = $order; // Add the order to the filtered list - } - } - } - // If there are more than one order in the filtered list, add it to the possible duplicates - if (count($filteredOrders) > 1) { - $possibleDuplicates[$reg_1] = $filteredOrders; - } - } - } - // Return the possible duplicates - return $possibleDuplicates; + return invoicing_period_utils::filterPossibleDuplicates($orders, 86400); } public function setTemporaryNetAmount(float $amount): void diff --git a/services/nginx/app/routes/InvoicingPeriodRoute.php b/services/nginx/app/routes/InvoicingPeriodRoute.php index 98b9caaa..6bee3a96 100644 --- a/services/nginx/app/routes/InvoicingPeriodRoute.php +++ b/services/nginx/app/routes/InvoicingPeriodRoute.php @@ -3,6 +3,7 @@ namespace routes; use classes\authentication; +use classes\invoicing_period_utils; use classes\slack; use Exception; use objects\customer_vehicles_o; @@ -73,6 +74,33 @@ class InvoicingPeriodRoute return in_array(strtolower((string)$envFlag), ['1', 'true', 'yes'], true); } + /** + * @return array{dateFrom:string,dateTo:string} + */ + private function requireAndNormalizeDateRange(): array + { + global $response; + + self::requireParameters([ + 'dateFrom', + 'dateTo', + ]); + + $dateFrom = (string)$this->getParameter('dateFrom'); + $dateTo = (string)$this->getParameter('dateTo'); + + try { + return invoicing_period_utils::normalizeDateRange($dateFrom, $dateTo); + } catch (\InvalidArgumentException $e) { + $response->error($e->getMessage(), 400); + } + + return [ + 'dateFrom' => '', + 'dateTo' => '', + ]; + } + /** * @param array $collective_results @@ -100,18 +128,9 @@ class InvoicingPeriodRoute $user = (new authentication())->get_user(); // Check if the request was successful if ($user) { - self::requireParameters([ - 'dateFrom', - 'dateTo', - ]); - $dateFrom = $this->getParameter('dateFrom'); - $dateTo = $this->getParameter('dateTo'); - // Require the dateFrom and dateTo parameters to be valid dates - self::requireDateFormat($dateFrom, 'Y-m-d'); - self::requireDateFormat($dateTo, 'Y-m-d'); - // Add a day to the dateTo parameter to include the end date in the range - $dateFrom = date('Y-m-d 00:00:00', strtotime($dateFrom)); - $dateTo = date('Y-m-d 23:59:59', strtotime($dateTo)); + $dateRange = $this->requireAndNormalizeDateRange(); + $dateFrom = $dateRange['dateFrom']; + $dateTo = $dateRange['dateTo']; // Add date from and date to to the response meta $response->add_meta('date_from', $dateFrom); $response->add_meta('date_to', $dateTo); @@ -133,19 +152,10 @@ class InvoicingPeriodRoute $this->get('/superuser/invoicing/period/distribution/all', function () { // Require the user to be logged in global $response; - //$this->requirePermission('superuser_invoicing_period'); - self::requireParameters([ - 'dateFrom', - 'dateTo', - ]); - $dateFrom = $this->getParameter('dateFrom'); - $dateTo = $this->getParameter('dateTo'); - // Require the dateFrom and dateTo parameters to be valid dates - self::requireDateFormat($dateFrom, 'Y-m-d'); - self::requireDateFormat($dateTo, 'Y-m-d'); - // Add a day to the dateTo parameter to include the end date in the range - $dateTo = date('Y-m-d 23:59:59', strtotime($dateTo)); - $dateFrom = date('Y-m-d 00:00:00', strtotime($dateFrom)); + $this->requirePermission('superuser_invoicing_period'); + $dateRange = $this->requireAndNormalizeDateRange(); + $dateFrom = $dateRange['dateFrom']; + $dateTo = $dateRange['dateTo']; // Add date from and date to to the response meta $response->add_meta('date_from', $dateFrom); $response->add_meta('date_to', $dateTo); @@ -165,18 +175,9 @@ class InvoicingPeriodRoute // Require the user to be logged in global $response; $this->requirePermission('superuser_invoicing_period'); - self::requireParameters([ - 'dateFrom', - 'dateTo', - ]); - $dateFrom = $this->getParameter('dateFrom'); - $dateTo = $this->getParameter('dateTo'); - // Require the dateFrom and dateTo parameters to be valid dates - self::requireDateFormat($dateFrom, 'Y-m-d'); - self::requireDateFormat($dateTo, 'Y-m-d'); - // Add a day to the dateTo parameter to include the end date in the range - $dateFrom = date('Y-m-d 00:00:00', strtotime($dateFrom)); - $dateTo = date('Y-m-d 23:59:59', strtotime($dateTo)); + $dateRange = $this->requireAndNormalizeDateRange(); + $dateFrom = $dateRange['dateFrom']; + $dateTo = $dateRange['dateTo']; // Add date from and date to to the response meta $response->add_meta('date_from', $dateFrom); $response->add_meta('date_to', $dateTo); @@ -191,18 +192,9 @@ class InvoicingPeriodRoute // Require the user to be logged in global $response; $this->requirePermission('superuser_invoicing_period'); - self::requireParameters([ - 'dateFrom', - 'dateTo', - ]); - $dateFrom = $this->getParameter('dateFrom'); - $dateTo = $this->getParameter('dateTo'); - // Require the dateFrom and dateTo parameters to be valid dates - self::requireDateFormat($dateFrom, 'Y-m-d'); - self::requireDateFormat($dateTo, 'Y-m-d'); - // Add a day to the dateTo parameter to include the end date in the range - $dateFrom = date('Y-m-d 00:00:00', strtotime($dateFrom)); - $dateTo = date('Y-m-d 23:59:59', strtotime($dateTo)); + $dateRange = $this->requireAndNormalizeDateRange(); + $dateFrom = $dateRange['dateFrom']; + $dateTo = $dateRange['dateTo']; // Add date from and date to to the response meta $response->add_meta('date_from', $dateFrom); $response->add_meta('date_to', $dateTo); @@ -217,18 +209,9 @@ class InvoicingPeriodRoute // Require the user to be logged in global $response; $this->requirePermission('superuser_invoicing_period'); - self::requireParameters([ - 'dateFrom', - 'dateTo', - ]); - $dateFrom = $this->getParameter('dateFrom'); - $dateTo = $this->getParameter('dateTo'); - // Require the dateFrom and dateTo parameters to be valid dates - $this->requireDateFormat($dateFrom, 'Y-m-d'); - $this->requireDateFormat($dateTo, 'Y-m-d'); - $dateFrom = date('Y-m-d 00:00:00', strtotime($dateFrom)); - // Add a day to the dateTo parameter to include the end date in the range - $dateTo = date('Y-m-d 23:59:59', strtotime($dateTo)); + $dateRange = $this->requireAndNormalizeDateRange(); + $dateFrom = $dateRange['dateFrom']; + $dateTo = $dateRange['dateTo']; // Get all orders in the date range that have: // - Department ID: 10 // - Reference: Vaskeabonnementer diff --git a/services/nginx/app/tests/Unit/Invoicing/InvoicingOrdersCalculationsHardeningTest.php b/services/nginx/app/tests/Unit/Invoicing/InvoicingOrdersCalculationsHardeningTest.php new file mode 100644 index 00000000..82c74a93 --- /dev/null +++ b/services/nginx/app/tests/Unit/Invoicing/InvoicingOrdersCalculationsHardeningTest.php @@ -0,0 +1,29 @@ +not->toBeFalse(); + expect($content)->toContain('public function getNetAmountForOrders(array $order_ids): array'); + expect($content)->toContain('if (empty($order_ids)) {'); + expect($content)->toContain('return [];'); +}); + +it('guards against empty customer arrays in date-range transaction fetches', function (): void { + $ordersFile = app_path('objects/orders_o.php'); + $content = file_get_contents($ordersFile); + + expect($content)->not->toBeFalse(); + expect($content)->toContain('public function getTransactionsForCustomersInDateRange(array $customers, string $dateFrom, string $dateTo): array'); + expect($content)->toContain('if (empty($customers)) {'); + expect($content)->toContain('return [];'); +}); + +it('uses centralized duplicate filtering for possible duplicate detection', function (): void { + $ordersFile = app_path('objects/orders_o.php'); + $content = file_get_contents($ordersFile); + + expect($content)->not->toBeFalse(); + expect($content)->toContain('invoicing_period_utils::filterPossibleDuplicates($orders, 86400)'); +}); diff --git a/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodRouteGuardsTest.php b/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodRouteGuardsTest.php new file mode 100644 index 00000000..afe913e5 --- /dev/null +++ b/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodRouteGuardsTest.php @@ -0,0 +1,20 @@ +not->toBeFalse(); + expect($content)->toMatch( + "/\\/superuser\\/invoicing\\/period\\/distribution\\/all'.*?\\\$this->requirePermission\\('superuser_invoicing_period'\\);/s" + ); +}); + +it('uses shared date-range normalization across invoicing period endpoints', function (): void { + $routeFile = app_path('routes/InvoicingPeriodRoute.php'); + $content = file_get_contents($routeFile); + + expect($content)->not->toBeFalse(); + expect(substr_count((string)$content, 'requireAndNormalizeDateRange()'))->toBeGreaterThanOrEqual(5); +}); + diff --git a/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodUtilsTest.php b/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodUtilsTest.php new file mode 100644 index 00000000..17be1bc6 --- /dev/null +++ b/services/nginx/app/tests/Unit/Invoicing/InvoicingPeriodUtilsTest.php @@ -0,0 +1,42 @@ +toBe([ + 'dateFrom' => '2026-03-01 00:00:00', + 'dateTo' => '2026-03-31 23:59:59', + ]); +}); + +it('rejects invalid date formats', function (): void { + invoicing_period_utils::normalizeDateRange('2026/03/01', '2026-03-31'); +})->throws(InvalidArgumentException::class, 'Invalid date format. Expected: Y-m-d Got: 2026/03/01'); + +it('rejects descending date ranges', function (): void { + invoicing_period_utils::normalizeDateRange('2026-04-01', '2026-03-31'); +})->throws(InvalidArgumentException::class, 'Invalid date range. dateFrom must be before or equal to dateTo'); + +it('finds duplicate orders regardless of original input order', function (): void { + $input = [ + 'ABC12345' => [ + ['id' => 3, 'created_at' => '2026-03-03 02:00:00'], + ['id' => 1, 'created_at' => '2026-03-01 12:00:00'], + ['id' => 2, 'created_at' => '2026-03-02 07:00:00'], + ], + 'NON_DUP' => [ + ['id' => 10, 'created_at' => '2026-03-01 00:00:00'], + ['id' => 11, 'created_at' => '2026-03-03 00:00:01'], + ], + ]; + + $duplicates = invoicing_period_utils::filterPossibleDuplicates($input, 86400); + + expect(array_keys($duplicates))->toBe(['ABC12345']); + expect(array_column($duplicates['ABC12345'], 'id'))->toBe([1, 2, 3]); +}); +