fix(economic): audit and sanitize additional export fields (TRU-193) (#393)
## Summary Audit and (where needed) fix additional fields in the e-conomic export path. PR #391 covered the main order.* and order_item.* fields; this PR covers the remaining fields that could carry special characters. ## Changes 1. Pre-flight validation (defense in depth): 5 rules per line throw on violation. 2. addTextLine() and addProductLine() now sanitize at insertion (defense in depth). 3. Recipient block sanitization in add(): name/address/zip/city via sanitizeTextLine, EAN via preg_replace. 4. Audit document: documentation/economic/export-field-audit.md. 5. Tests: 94 tests / 171 assertions (14 + 19 + 6 + 24 new tests). ## Refs - TRU-193, TRU-188, TRU-194, PR #391 --------- Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: OpenClaw <openclaw@copenhagentruckwash.io> Co-authored-by: Bugfix Subagent <bugfix@subagent.local>
This commit is contained in:
co-authored by
openhands
OpenClaw
Bugfix Subagent
parent
b16b2cfe44
commit
ea9bdbe12c
@@ -0,0 +1,262 @@
|
||||
# E-conomic Export Field Audit (TRU-193)
|
||||
|
||||
**Status:** Complete
|
||||
**Date:** 2026-08-17
|
||||
**Scope:** All user-input fields that flow into e-conomic API payloads from
|
||||
the `copenhagentruckwash/api` backend.
|
||||
**Primary files audited:**
|
||||
- `services/nginx/app/modules/economic/helpers/economic_invoice_draft.php`
|
||||
- `services/nginx/app/modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php`
|
||||
- `services/nginx/app/modules/economic/endpoints/invoices/draft/economic_invoices_draft_endpoint.php`
|
||||
- `services/nginx/app/classes/economic_export_sanitizer.php` (the sanitizer itself)
|
||||
|
||||
## Summary
|
||||
|
||||
| Category | Count |
|
||||
|----------|-------|
|
||||
| User-input fields audited | 17 |
|
||||
| Fields already sanitized (covered by PR #391 or preflight) | 14 |
|
||||
| Fields newly sanitized in TRU-193 | 3 (`recipient.name`, `recipient.address`, `recipient.zip/city`, `recipient.ean`) |
|
||||
| Fields that are controlled input (no sanitization needed) | 4 |
|
||||
| Fields not present in any e-conomic export path (out of scope) | 3 |
|
||||
|
||||
All user-input fields flowing to e-conomic are now either sanitized via
|
||||
`economic_export_sanitizer` or verified to be controlled input.
|
||||
|
||||
## Sanitizer methods used
|
||||
|
||||
| Method | Purpose | Length cap |
|
||||
|--------|---------|------------|
|
||||
| `sanitizeTextLine($value, $maxLength=250)` | Plain text lines (PO, ref, notes, recipient fields) | 250 (configurable) |
|
||||
| `sanitizeProductNumber($value)` | Product identifiers | 50 |
|
||||
| `sanitizeProductDescription($value)` | Product-line descriptions | 500 |
|
||||
| `sanitizeForEconApi($value)` | Catch-all alias of `sanitizeTextLine` | 250 |
|
||||
|
||||
Rules applied:
|
||||
- `/` replaced with `-` (the reported 400 trigger, TRU-188)
|
||||
- Control characters (`\x00-\x1F` except `\t` and `\n`, plus `\x7F`) stripped
|
||||
- Tab + newline characters collapse to a single space
|
||||
- Whitespace normalized and trimmed
|
||||
- Length capped with `...` suffix if too long
|
||||
|
||||
## Audit by field
|
||||
|
||||
### 1. `order.po` (purchase order)
|
||||
- **Source:** `orders_o::po` (user input)
|
||||
- **Flows to:** Text line in draft invoice (`addNewTransactionHeader`)
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine()`
|
||||
- **Sensitive to:** `/`, newlines, control chars, length
|
||||
|
||||
### 2. `order.reference`
|
||||
- **Source:** `orders_o::reference` (user input)
|
||||
- **Flows to:** Text lines in draft invoice (multiple `Reference:` lines)
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine()`
|
||||
- **Sensitive to:** `/` (PRIMARY TRU-188 trigger), newlines, control chars
|
||||
|
||||
### 3. `order.notes`
|
||||
- **Source:** `orders_o::notes` (user input)
|
||||
- **Flows to:** Text lines in draft invoice (multiple `Notat:` lines)
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine()`
|
||||
- **Sensitive to:** `/`, newlines, control chars, length
|
||||
|
||||
### 4. `order.reg_1`, `order.reg_2`, `order.reg_3`
|
||||
- **Source:** `orders_o::reg_1/2/3` (user input — vehicle registration numbers)
|
||||
- **Flows to:** Concatenated `Reg 1: ... Reg 2: ... Reg 3: ...` line
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine(..., 50)` then `strtoupper()`
|
||||
- **Sensitive to:** `/`, special chars, length (capped at 50)
|
||||
|
||||
### 5. `department.name`
|
||||
- **Source:** `departments_o::getDepartmentName()` (admin input)
|
||||
- **Flows to:** Transaction header line `[ date department_name #order_id ]`
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine(..., 100)`
|
||||
- **Sensitive to:** `/` (e.g. "Roskilde/Ølstykke"), special chars, length
|
||||
|
||||
### 6. `order.created_at` (formatted date)
|
||||
- **Source:** `orders_o::created_at` (server-generated timestamp)
|
||||
- **Flows to:** Transaction header line date prefix
|
||||
- **Status:** ✅ Controlled input — formatted by `date('d/m/Y H:i', strtotime(...))`
|
||||
- **Sensitive to:** None (formatted as digits + slashes; `/` is added by date format
|
||||
but the sanitizer does not run on the formatted string — verified by inspection
|
||||
that the slashes in `dd/mm/YYYY` are safe; this is a known, accepted pattern)
|
||||
|
||||
### 7. `order.id` (integer)
|
||||
- **Source:** Database auto-increment
|
||||
- **Flows to:** Transaction header line `#{id}` suffix
|
||||
- **Status:** ✅ Controlled input — integer
|
||||
- **Sensitive to:** None
|
||||
|
||||
### 8. `order_item.reference`
|
||||
- **Source:** Per-item reference (user input)
|
||||
- **Flows to:** Text lines under each order item (`Reference:` + `# ...`)
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine()`
|
||||
- **Sensitive to:** `/`, newlines, control chars, length
|
||||
|
||||
### 9. `order_item.notes`
|
||||
- **Source:** Per-item notes (user input)
|
||||
- **Flows to:** Text lines under each order item (`Notat:` + `# ...`)
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeTextLine()`
|
||||
- **Sensitive to:** `/`, newlines, control chars, length
|
||||
|
||||
### 10. `order_item.product.economic_product_id`
|
||||
- **Source:** `products_o::economic_product_id` (admin-set)
|
||||
- **Flows to:** `product.productNumber` in the e-conomic line payload
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeProductNumber()`
|
||||
- **Sensitive to:** Path separators, illegal chars
|
||||
|
||||
### 11. `order_item.product.name`
|
||||
- **Source:** `products_o::name` (admin-set product name)
|
||||
- **Flows to:** `description` in the e-conomic line payload
|
||||
- **Status:** ✅ Already sanitized
|
||||
- **Sanitizer:** `sanitizeProductDescription()` (called inside `addProductLine()`)
|
||||
- **Sensitive to:** `/`, newlines, control chars, length (capped at 500)
|
||||
|
||||
### 12. `order_item.quantity`, `order_item.price`, `order_item.product.price`
|
||||
- **Source:** Numeric fields (calculated or admin-set)
|
||||
- **Flows to:** `quantity`, `unitNetPrice`, `discountPercentage` numeric fields
|
||||
- **Status:** ✅ Controlled input — numeric types; cast to float/int before use
|
||||
- **Sensitive to:** None
|
||||
|
||||
### 13. Currency (`DKK`, `EUR`, etc.)
|
||||
- **Source:** Admin-set on the department / invoice
|
||||
- **Flows to:** `'currency' => $currency` in the invoice payload
|
||||
- **Status:** ✅ Controlled input — ISO 4217 codes, validated by `strtoupper`
|
||||
- **Sensitive to:** None
|
||||
|
||||
### 14. `recipient.name` (in `economic_invoices_drafts_endpoint::add()`)
|
||||
- **Source:** `economic_customer::getName()` (e-conomic customer data — controlled input)
|
||||
- **Flows to:** `recipient.name` in the create-invoice payload
|
||||
- **Status:** 🆕 Newly sanitized in TRU-193
|
||||
- **Sanitizer:** `sanitizeTextLine(..., 100)`
|
||||
- **Sensitive to (defense in depth):** `/`, newlines, control chars, length
|
||||
- **Rationale:** Although this comes from e-conomic (so e-conomic already has
|
||||
it), we sanitize defensively in case e-conomic later rejects a value it
|
||||
previously accepted, or in case the API contract changes. Cap of 100 chars
|
||||
matches the e-conomic recipient `name` field limit.
|
||||
|
||||
### 15. `recipient.address` (in `economic_invoices_drafts_endpoint::add()`)
|
||||
- **Source:** `economic_customer::getAddress()` (e-conomic customer data — controlled input)
|
||||
- **Flows to:** `recipient.address` in the create-invoice payload
|
||||
- **Status:** 🆕 Newly sanitized in TRU-193
|
||||
- **Sanitizer:** `sanitizeTextLine(..., 250)`
|
||||
- **Sensitive to (defense in depth):** Newlines (postal format), `/` (some
|
||||
countries use `/` in street names), control chars, length
|
||||
- **Rationale:** Same as `recipient.name` — defense in depth.
|
||||
|
||||
### 16. `recipient.zip`, `recipient.city`
|
||||
- **Source:** `economic_customer::getZipCode()`, `getCity()` (e-conomic data)
|
||||
- **Flows to:** `recipient.zip`, `recipient.city` in the create-invoice payload
|
||||
- **Status:** 🆕 Newly sanitized in TRU-193
|
||||
- **Sanitizer:** `sanitizeTextLine(..., 20)` for zip, `(..., 100)` for city
|
||||
- **Sensitive to (defense in depth):** Special chars, length
|
||||
- **Rationale:** Defense in depth — same as above.
|
||||
|
||||
### 17. `recipient.ean` (in `economic_invoices_drafts_endpoint::add()`)
|
||||
- **Source:** `economic_customer::getEan()` (e-conomic data)
|
||||
- **Flows to:** `recipient.ean` + `recipient.nemHandelType = 'ean'`
|
||||
- **Status:** 🆕 Newly sanitized in TRU-193
|
||||
- **Sanitizer:** `preg_replace('/[^0-9]/', '', $ean)` — strip non-digits
|
||||
- **Sensitive to:** Non-digit chars; EAN must be numeric per NemHandel spec
|
||||
- **Rationale:** If the sanitized value is empty, we omit the EAN key entirely
|
||||
rather than sending an empty string (which e-conomic may reject).
|
||||
|
||||
## Fields audited but not present in this export path
|
||||
|
||||
These fields were mentioned in the TRU-193 ticket but are **not used in any
|
||||
e-conomic export code path** in this backend. Documenting them for
|
||||
completeness:
|
||||
|
||||
| Field | Why not in scope |
|
||||
|-------|------------------|
|
||||
| `customer.email` | Email is fetched from e-conomic via `economic_customer::getEmail()` and never sent back in the create-invoice payload. The email field is used only for read operations. |
|
||||
| `customer.address` (full multi-line) | `recipient.address` is the e-conomic-controlled single-line address; the multi-line address (used for HTML rendering) is not sent to e-conomic. |
|
||||
| `subscription.name` | Subscription names are not sent to e-conomic; the e-conomic invoice export only includes order items, not subscription data. |
|
||||
|
||||
## Other controlled inputs (no sanitization needed)
|
||||
|
||||
| Field | Why safe |
|
||||
|-------|----------|
|
||||
| `external_id` | Generated UUID (`bin2hex(random_bytes(16))`); only `[0-9a-f-]` |
|
||||
| `layout.layoutNumber` | Admin-set integer from e-conomic config |
|
||||
| `paymentTerms.paymentTermsNumber` | Integer from e-conomic |
|
||||
| `vatZone.vatZoneNumber` | Integer from e-conomic |
|
||||
| `customer.customerNumber` | Integer from e-conomic |
|
||||
| `attention` reference | E-conomic nested object (`customerContactNumber`) |
|
||||
| `customerContact` / `salesPerson` / `deliveryLocation` | E-conomic nested objects |
|
||||
| `departmentalDistributionNumber` / `dimension` | Integer IDs |
|
||||
| `TotDiscount` (productNumber for discount line) | Literal string constant |
|
||||
| `'Rabat'` (description for discount line) | Literal string constant |
|
||||
|
||||
## Defense in depth: preflight validation
|
||||
|
||||
In addition to the field-level sanitizers, `economic_invoice_draft::addLines()`
|
||||
now runs a **preflight validation** before sending to e-conomic. The preflight
|
||||
checks 5 rules per line and throws `RuntimeException` on the first violation:
|
||||
|
||||
1. `description` must be non-empty after `trim()`
|
||||
2. `description` must be ≤ 250 chars
|
||||
3. `productNumber` (if present) must match `/^[A-Za-z0-9._-]{1,50}$/`
|
||||
4. `quantity` (if present) must be a positive number
|
||||
5. `unitNetPrice` (if present) must be a number ≥ 0
|
||||
|
||||
Even if a sanitizer is bypassed or a new field is added without sanitization,
|
||||
the preflight catches the most common 400-error triggers and fails loudly
|
||||
before the request goes out.
|
||||
|
||||
## Test coverage
|
||||
|
||||
- `EconomicExportSanitizerTest` (PHPUnit) — 45 tests / ~80 assertions
|
||||
- Original 31: slash replacement, control chars, tab/newline handling,
|
||||
whitespace collapse, length cap with ellipsis, multibyte safety,
|
||||
null/empty input, integer/float input, product number rules
|
||||
- New 14 (TRU-193): recipient name/address/zip/city length caps,
|
||||
recipient address newlines + slashes, Danish/UK postal formats,
|
||||
Danish special chars (København Ø), ampersand + quotes, CRLF
|
||||
normalization, empty-field handling, EAN digit preservation
|
||||
- `EconomicInvoiceDraftPreflightTest` (PHPUnit) — 19 tests / 37 assertions
|
||||
- Covers: all 5 preflight rules + the disabled-flag bypass path
|
||||
- `EconomicInvoiceDraftRecipientSanitizationTest` (PHPUnit) — 6 tests
|
||||
- Verifies the recipient-block wiring in `economic_invoices_drafts_endpoint.php`
|
||||
(sanitize calls for name/address/zip/city, preg_replace for EAN,
|
||||
empty-EAN unsets the key)
|
||||
- `EconomicDraftSanitizationIntegrationTest` (PHPUnit, integration) — 24 tests / 51 assertions
|
||||
- End-to-end: addTextLine sanitization, addProductLine sanitization + empty-skip,
|
||||
preflight catches all 5 rules, mixed text + product flow works
|
||||
|
||||
Total: 94 tests, 171 assertions, all passing.
|
||||
|
||||
## What changed in TRU-193
|
||||
|
||||
1. **Pre-flight validation** added to `economic_invoice_draft.php`
|
||||
(separate atomic commit) — defense in depth.
|
||||
2. **Recipient block sanitization** added in
|
||||
`economic_invoices_drafts_endpoint.php`:
|
||||
- `customer_name`, `customer_address`, `customer_zip`, `customer_city`
|
||||
now go through `sanitizeTextLine()` with field-appropriate length caps.
|
||||
- `customer_ean` is stripped to digits only; if empty, the `ean` key is
|
||||
removed from the payload (and `nemHandelType` is not set).
|
||||
3. **Defense-in-depth at insertion** in `economic_invoice_draft.php`:
|
||||
- `addTextLine()` now sanitizes at insertion time (was: sanitization only
|
||||
happened in the calling methods). Catches any new caller that forgets
|
||||
to sanitize.
|
||||
- `addProductLine()` sanitizes at insertion and skips the line entirely
|
||||
if sanitization produced an empty product number or description
|
||||
(was: would have passed empty strings to e-conomic and triggered a 400).
|
||||
4. **No changes to already-sanitized fields** (PO, reference, notes,
|
||||
reg_*, department name, product name, product number) — PR #391
|
||||
already covered them correctly.
|
||||
|
||||
## Refs
|
||||
|
||||
- TRU-188 — Reported 400 on `/` in order reference (the original trigger)
|
||||
- TRU-189 through TRU-196 — Related issues covered by PR #391
|
||||
- TRU-194 — Pre-flight validation (separate workstream)
|
||||
- PR #391 — Initial fix for `order.*` and `order_item.*` fields
|
||||
- PR #392 — Pre-flight validation defense in depth
|
||||
+17
-7
@@ -125,10 +125,15 @@ class economic_invoices_drafts_endpoint
|
||||
$customer = (new economic())->getCustomer($customer_number);
|
||||
|
||||
// Set the recipient details
|
||||
$customer_name = $customer->getName() ?? 'Ukendt';
|
||||
$customer_address = $customer->getAddress() ?? 'Ukendt';
|
||||
$customer_zip = $customer->getZipCode() ?? 'Ukendt';
|
||||
$customer_city = $customer->getCity() ?? 'Ukendt';
|
||||
// Note: customer_* fields come from e-conomic itself (controlled input),
|
||||
// but we sanitize them defensively to avoid 400s if e-conomic ever stores
|
||||
// a value with chars e-conomic later rejects in the recipient block.
|
||||
// Each field uses an appropriate length cap to match the corresponding
|
||||
// e-conomic recipient field limits.
|
||||
$customer_name = \classes\economic_export_sanitizer::sanitizeTextLine($customer->getName() ?? 'Ukendt', 100);
|
||||
$customer_address = \classes\economic_export_sanitizer::sanitizeTextLine($customer->getAddress() ?? 'Ukendt', 250);
|
||||
$customer_zip = \classes\economic_export_sanitizer::sanitizeTextLine($customer->getZipCode() ?? 'Ukendt', 20);
|
||||
$customer_city = \classes\economic_export_sanitizer::sanitizeTextLine($customer->getCity() ?? 'Ukendt', 100);
|
||||
$recipient = [
|
||||
'name' => $customer_name,
|
||||
'address' => $customer_address,
|
||||
@@ -139,9 +144,14 @@ class economic_invoices_drafts_endpoint
|
||||
],
|
||||
];
|
||||
$customer_ean = $customer->getEan();
|
||||
if ($customer_ean !== null) {
|
||||
$recipient['ean'] = $customer_ean;
|
||||
$recipient['nemHandelType'] = 'ean';
|
||||
if ($customer_ean !== null && $customer_ean !== '') {
|
||||
// EAN should be digits only; sanitize to strip anything that slipped through
|
||||
$recipient['ean'] = preg_replace('/[^0-9]/', '', $customer_ean);
|
||||
if ($recipient['ean'] !== '') {
|
||||
$recipient['nemHandelType'] = 'ean';
|
||||
} else {
|
||||
unset($recipient['ean']);
|
||||
}
|
||||
}
|
||||
$public_entry_number = $customer->getPublicEntryNumber();
|
||||
if ($public_entry_number !== null) {
|
||||
|
||||
@@ -42,6 +42,13 @@ class economic_invoice_draft
|
||||
*/
|
||||
protected float $conversion_rate = 1.0;
|
||||
|
||||
/**
|
||||
* Whether pre-flight validation runs inside addLines() before sending to e-conomic.
|
||||
* Defense in depth — even after sanitization, a final check catches anything that slips through.
|
||||
* @var bool $preflight_enabled
|
||||
*/
|
||||
protected bool $preflight_enabled = true;
|
||||
|
||||
|
||||
/**
|
||||
* Construct a new Economic draft invoice object
|
||||
@@ -116,9 +123,142 @@ class economic_invoice_draft
|
||||
*/
|
||||
public function addLines(): void
|
||||
{
|
||||
if ($this->preflight_enabled) {
|
||||
$this->preflightValidate($this->draft_lines, null);
|
||||
}
|
||||
$this->flushLinesInBatches();
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-flight validation: defense in depth before sending to e-conomic.
|
||||
* Validates each line against 5 rules and throws RuntimeException on the first violation.
|
||||
*
|
||||
* Rules (in order, per line):
|
||||
* 1. description — must be non-empty after trim()
|
||||
* 2. description — must be <= 250 chars
|
||||
* 3. productNumber (if present in product.productNumber) — must match /^[A-Za-z0-9._-]{1,50}$/
|
||||
* 4. quantity — must be a positive number (> 0)
|
||||
* 5. unitNetPrice — must be a number (>= 0)
|
||||
*
|
||||
* @param array<int,array<string,mixed>> $lines
|
||||
* @param int|null $orderId Optional order id for log context
|
||||
* @throws \RuntimeException on any rule violation
|
||||
*/
|
||||
public function preflightValidate(array $lines, ?int $orderId = null): void
|
||||
{
|
||||
foreach ($lines as $i => $line) {
|
||||
if (!is_array($line)) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'line is not an array',
|
||||
$line
|
||||
);
|
||||
}
|
||||
|
||||
// Rule 1 + 2: description
|
||||
$description = $line['description'] ?? null;
|
||||
if ($description === null) {
|
||||
$description = '';
|
||||
}
|
||||
if (!is_scalar($description)) {
|
||||
$description = (string)$description;
|
||||
} else {
|
||||
$description = (string)$description;
|
||||
}
|
||||
$descriptionTrimmed = trim($description);
|
||||
if ($descriptionTrimmed === '') {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'description is empty',
|
||||
$description
|
||||
);
|
||||
}
|
||||
if (mb_strlen($descriptionTrimmed) > 250) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'description exceeds 250 chars (length=' . mb_strlen($descriptionTrimmed) . ')',
|
||||
$description
|
||||
);
|
||||
}
|
||||
|
||||
// Rule 3: productNumber (only if present in product.productNumber)
|
||||
if (isset($line['product']) && is_array($line['product']) && array_key_exists('productNumber', $line['product'])) {
|
||||
$productNumber = $line['product']['productNumber'];
|
||||
if ($productNumber === null) {
|
||||
$productNumber = '';
|
||||
} else {
|
||||
$productNumber = (string)$productNumber;
|
||||
}
|
||||
if (!preg_match('/^[A-Za-z0-9._-]{1,50}$/', $productNumber)) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'productNumber does not match /^[A-Za-z0-9._-]{1,50}$/',
|
||||
$productNumber
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Rule 4: quantity — only required if present in the line (text lines omit it)
|
||||
if (array_key_exists('quantity', $line)) {
|
||||
$quantity = $line['quantity'];
|
||||
if (!is_numeric($quantity) || (float)$quantity <= 0) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'quantity is not a positive number',
|
||||
$quantity
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Rule 5: unitNetPrice — only required if present in the line
|
||||
if (array_key_exists('unitNetPrice', $line)) {
|
||||
$unitNetPrice = $line['unitNetPrice'];
|
||||
if (!is_numeric($unitNetPrice) || (float)$unitNetPrice < 0) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'unitNetPrice is not a number >= 0',
|
||||
$unitNetPrice
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Log the offending line and throw a RuntimeException.
|
||||
*/
|
||||
private function logAndThrow(int $lineIndex, ?int $orderId, string $rule, mixed $value): never
|
||||
{
|
||||
$valueTruncated = is_scalar($value) ? (string)$value : json_encode($value);
|
||||
if ($valueTruncated === false) {
|
||||
$valueTruncated = '[unserializable]';
|
||||
}
|
||||
if (mb_strlen($valueTruncated) > 200) {
|
||||
$valueTruncated = mb_substr($valueTruncated, 0, 200) . '...';
|
||||
}
|
||||
$orderContext = $orderId === null ? 'order=n/a' : 'order=' . $orderId;
|
||||
error_log(sprintf(
|
||||
'[preflight] validation failed: %s | line=%d | %s | value=%s',
|
||||
$rule,
|
||||
$lineIndex,
|
||||
$orderContext,
|
||||
$valueTruncated
|
||||
));
|
||||
$orderPart = $orderId === null ? '' : ' (order ' . $orderId . ')';
|
||||
throw new \RuntimeException(sprintf(
|
||||
'Preflight validation failed for line %d: %s%s',
|
||||
$lineIndex,
|
||||
$rule,
|
||||
$orderPart
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* Add queued draft lines using chunked requests.
|
||||
*
|
||||
@@ -246,11 +386,26 @@ class economic_invoice_draft
|
||||
*/
|
||||
public function addTextLine(string $text): void
|
||||
{
|
||||
// Defense in depth: sanitize ALL text lines at insertion time.
|
||||
// This catches anything that wasn't pre-sanitized at the call site.
|
||||
$sanitized = \classes\economic_export_sanitizer::sanitizeTextLine($text);
|
||||
if ($sanitized === '') {
|
||||
return; // Skip empty/whitespace-only lines
|
||||
}
|
||||
$this->draft_lines[] = [
|
||||
'description' => $text
|
||||
'description' => $sanitized
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the current draft lines (read-only view).
|
||||
* Used by integration tests; production code uses addLines() to send.
|
||||
*/
|
||||
public function getDraftLines(): array
|
||||
{
|
||||
return $this->draft_lines;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add an order to the draft invoice
|
||||
* @note The lines won't be saved until the addLines() method is called.
|
||||
@@ -485,6 +640,10 @@ class economic_invoice_draft
|
||||
// Sanitize product identifier and description (defense in depth — also done at addLines())
|
||||
$productNumber = \classes\economic_export_sanitizer::sanitizeProductNumber($productNumber);
|
||||
$description = \classes\economic_export_sanitizer::sanitizeProductDescription($description);
|
||||
// Skip if sanitization removed everything
|
||||
if ($productNumber === '' || $description === '') {
|
||||
return;
|
||||
}
|
||||
// We then add a 0 in front of the product number to make sure it's the right one, made by FlexPOS.
|
||||
// Add a line to the invoice
|
||||
$line = [
|
||||
|
||||
+323
@@ -0,0 +1,323 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* End-to-end integration test for the e-conomic draft invoice export flow.
|
||||
*
|
||||
* Verifies that:
|
||||
* - addTextLine() sanitizes all user input (slash → dash, control chars, length)
|
||||
* - addProductLine() sanitizes product numbers and descriptions
|
||||
* - preflightValidate() catches all 5 rule violations
|
||||
* - Mixed text + product lines (with/without discount) pass preflight
|
||||
* - Empty/whitespace-only lines are skipped (not added to draft)
|
||||
*
|
||||
* This test does NOT hit a live e-conomic API.
|
||||
* For live verification, see /workspace/scripts/verify-economic-drafts-live.php
|
||||
*
|
||||
* Run: php8.4 services/nginx/app/vendor/bin/phpunit \
|
||||
* -c services/nginx/app/phpunit.xml \
|
||||
* services/nginx/app/tests/Integration/Invoicing/EconomicDraftSanitizationIntegrationTest.php
|
||||
*/
|
||||
|
||||
namespace tests\Integration\Invoicing;
|
||||
|
||||
require_once __DIR__ . '/../../../classes/economic_export_sanitizer.php';
|
||||
require_once __DIR__ . '/../../../modules/economic/helpers/economic_invoice_draft.php';
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use helpers\economic_invoice_draft;
|
||||
|
||||
class EconomicDraftSanitizationIntegrationTest extends TestCase
|
||||
{
|
||||
private economic_invoice_draft $draft;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
$this->draft = new economic_invoice_draft(12345, 'DKK', true); // skip_fetch=true: no live API call
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// addTextLine — sanitization
|
||||
// ========================================================================
|
||||
|
||||
public function testAddTextLineSanitizesSlashToDash(): void
|
||||
{
|
||||
$this->draft->addTextLine('Reference: Order/123/ABC');
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame('Reference: Order-123-ABC', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testAddTextLineStripsControlChars(): void
|
||||
{
|
||||
$this->draft->addTextLine("Line 1\nLine 2\twith tab");
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertSame('Line 1 Line 2 with tab', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testAddTextLineTruncatesVeryLongString(): void
|
||||
{
|
||||
$long = str_repeat('A', 5000);
|
||||
$this->draft->addTextLine($long);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertLessThanOrEqual(250, mb_strlen($lines[0]['description']));
|
||||
$this->assertStringEndsWith('...', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testMultipleTextLinesAllSanitized(): void
|
||||
{
|
||||
$this->draft->addTextLine('Reference: A/B');
|
||||
$this->draft->addTextLine('PO: C/D');
|
||||
$this->draft->addTextLine('Reg 1: E/F');
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(3, $lines);
|
||||
$this->assertSame('Reference: A-B', $lines[0]['description']);
|
||||
$this->assertSame('PO: C-D', $lines[1]['description']);
|
||||
$this->assertSame('Reg 1: E-F', $lines[2]['description']);
|
||||
}
|
||||
|
||||
public function testEmptyAndWhitespaceOnlyLinesAreSkipped(): void
|
||||
{
|
||||
$this->draft->addTextLine('');
|
||||
$this->draft->addTextLine(' ');
|
||||
$this->draft->addTextLine("\t\n ");
|
||||
$this->draft->addTextLine('///'); // All slashes become dashes, then trim leaves '-', not empty
|
||||
$lines = $this->draft->getDraftLines();
|
||||
// '///' becomes '---' which is not empty after trim
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame('---', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testPurelyWhitespaceAfterSanitizationIsSkipped(): void
|
||||
{
|
||||
$this->draft->addTextLine("\x00\x01\x02"); // All control chars, no actual text
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(0, $lines);
|
||||
}
|
||||
|
||||
public function testMultibyteTextPreservedCorrectly(): void
|
||||
{
|
||||
$this->draft->addTextLine('Kunde: ÆØÅ / 中文 / 🚗');
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertSame('Kunde: ÆØÅ - 中文 - 🚗', $lines[0]['description']);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// addProductLine — sanitization
|
||||
// ========================================================================
|
||||
|
||||
public function testProductLineWithoutDiscount(): void
|
||||
{
|
||||
$this->draft->addProductLine(
|
||||
'PROD-001',
|
||||
'Bilvask Standard',
|
||||
1.0,
|
||||
150.0,
|
||||
1,
|
||||
1,
|
||||
0.0
|
||||
);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame('Bilvask Standard', $lines[0]['description']);
|
||||
$this->assertSame(0.0, $lines[0]['discountPercentage']);
|
||||
$this->assertSame('PROD-001', $lines[0]['product']['productNumber']);
|
||||
}
|
||||
|
||||
public function testProductLineWithDiscount(): void
|
||||
{
|
||||
$this->draft->addProductLine(
|
||||
'PROD-002',
|
||||
'Storvask Premium',
|
||||
1.0,
|
||||
250.0,
|
||||
1,
|
||||
1,
|
||||
20.0
|
||||
);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame(20.0, $lines[0]['discountPercentage']);
|
||||
}
|
||||
|
||||
public function testProductLineWithSlashInNumberSanitized(): void
|
||||
{
|
||||
$this->draft->addProductLine(
|
||||
'PROD/003',
|
||||
'Premium/Service',
|
||||
1.0,
|
||||
100.0,
|
||||
1,
|
||||
1
|
||||
);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
// Product numbers REMOVE the slash (sanitizeProductNumber), text lines REPLACE with dash
|
||||
$this->assertSame('PROD003', $lines[0]['product']['productNumber']);
|
||||
$this->assertSame('Premium-Service', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testProductLineWithEmptyDescriptionSkipped(): void
|
||||
{
|
||||
$this->draft->addProductLine('PROD-001', '', 1.0, 100.0, 1, 1);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(0, $lines);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// preflightValidate — via addLines (with transport stub would be ideal,
|
||||
// but for unit-style integration we exercise preflight directly)
|
||||
// ========================================================================
|
||||
|
||||
public function testPreflightCatchesEmptyDescription(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->expectExceptionMessage('description is empty');
|
||||
$this->draft->preflightValidate([
|
||||
['description' => '', 'quantity' => 1, 'unitNetPrice' => 100.0],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightCatchesTooLongDescription(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->expectExceptionMessage('exceeds 250 chars');
|
||||
$this->draft->preflightValidate([
|
||||
['description' => str_repeat('A', 251), 'quantity' => 1, 'unitNetPrice' => 100.0],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightCatchesInvalidProductNumber(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->expectExceptionMessage('productNumber does not match');
|
||||
$this->draft->preflightValidate([
|
||||
[
|
||||
'description' => 'Valid line',
|
||||
'product' => ['productNumber' => 'PROD/01'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 100.0,
|
||||
],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightCatchesZeroQuantity(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->expectExceptionMessage('quantity is not a positive number');
|
||||
$this->draft->preflightValidate([
|
||||
['description' => 'Valid line', 'quantity' => 0, 'unitNetPrice' => 100.0],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightCatchesNegativeUnitPrice(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->expectExceptionMessage('unitNetPrice is not a number');
|
||||
$this->draft->preflightValidate([
|
||||
['description' => 'Valid line', 'quantity' => 1, 'unitNetPrice' => -10.0],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightIncludesOrderIdInMessage(): void
|
||||
{
|
||||
try {
|
||||
$this->draft->preflightValidate(
|
||||
[['description' => '', 'quantity' => 1, 'unitNetPrice' => 100.0]],
|
||||
300
|
||||
);
|
||||
$this->fail('Expected RuntimeException');
|
||||
} catch (\RuntimeException $e) {
|
||||
$this->assertStringContainsString('order 300', $e->getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
public function testPreflightPassesValidLines(): void
|
||||
{
|
||||
// Should not throw
|
||||
$this->draft->preflightValidate([
|
||||
['description' => 'Line 1', 'quantity' => 2, 'unitNetPrice' => 100.0],
|
||||
[
|
||||
'description' => 'Line 2 with product',
|
||||
'product' => ['productNumber' => 'PROD-01'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 50.0,
|
||||
'discountPercentage' => 10,
|
||||
],
|
||||
]);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
public function testPreflightPassesExactly250Chars(): void
|
||||
{
|
||||
$exactlyMax = str_repeat('B', 250);
|
||||
// Should not throw
|
||||
$this->draft->preflightValidate([
|
||||
['description' => $exactlyMax, 'quantity' => 1, 'unitNetPrice' => 100.0],
|
||||
]);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
public function testPreflightFailsAt251Chars(): void
|
||||
{
|
||||
$this->expectException(\RuntimeException::class);
|
||||
$this->draft->preflightValidate([
|
||||
['description' => str_repeat('B', 251), 'quantity' => 1, 'unitNetPrice' => 100.0],
|
||||
]);
|
||||
}
|
||||
|
||||
public function testPreflightAcceptsProductNumberWithDotsAndDashes(): void
|
||||
{
|
||||
// Should not throw
|
||||
$this->draft->preflightValidate([
|
||||
[
|
||||
'description' => 'Valid',
|
||||
'product' => ['productNumber' => 'PROD-01.0_test'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 100.0,
|
||||
],
|
||||
]);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// End-to-end: mixed flow
|
||||
// ========================================================================
|
||||
|
||||
public function testMixedLinesAllTogetherAndPassPreflight(): void
|
||||
{
|
||||
$this->draft->addTextLine('Reference: Order/2024/Q1');
|
||||
$this->draft->addProductLine('PROD-001', 'Bilvask', 2.0, 100.0, 1, 1, 10.0);
|
||||
$this->draft->addProductLine('PROD-002', 'Storvask', 1.0, 200.0, 1, 1, 0.0);
|
||||
$this->draft->addTextLine('Note: paid/in/full');
|
||||
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(4, $lines);
|
||||
|
||||
$this->assertSame('Reference: Order-2024-Q1', $lines[0]['description']);
|
||||
$this->assertSame('Bilvask', $lines[1]['description']);
|
||||
$this->assertSame(10.0, $lines[1]['discountPercentage']);
|
||||
$this->assertSame('Storvask', $lines[2]['description']);
|
||||
$this->assertSame(0.0, $lines[2]['discountPercentage']);
|
||||
$this->assertSame('Note: paid-in-full', $lines[3]['description']);
|
||||
|
||||
// All sanitized lines pass preflight
|
||||
$this->draft->preflightValidate($lines, 12345);
|
||||
}
|
||||
|
||||
public function testDiscountPathProducesSingleProductLineWithDiscountPct(): void
|
||||
{
|
||||
$this->draft->addProductLine('DISC-01', 'Rabatservice', 1.0, 100.0, 1, 1, 25.0);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame(25.0, $lines[0]['discountPercentage']);
|
||||
$this->assertSame('Rabatservice', $lines[0]['description']);
|
||||
}
|
||||
|
||||
public function testNoDiscountPathProducesSingleProductLineWithZeroDiscount(): void
|
||||
{
|
||||
$this->draft->addProductLine('NODISC-01', 'Standardservice', 1.0, 100.0, 1, 1, 0.0);
|
||||
$lines = $this->draft->getDraftLines();
|
||||
$this->assertCount(1, $lines);
|
||||
$this->assertSame(0.0, $lines[0]['discountPercentage']);
|
||||
$this->assertSame('Standardservice', $lines[0]['description']);
|
||||
}
|
||||
}
|
||||
@@ -221,4 +221,121 @@ class EconomicExportSanitizerTest extends TestCase
|
||||
economic_export_sanitizer::sanitizeForEconApi('foo/bar')
|
||||
);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Recipient-block fields (TRU-193)
|
||||
//
|
||||
// The recipient block in the create-invoice payload is built from
|
||||
// e-conomic customer data (name, address, zip, city). We sanitize
|
||||
// defensively with field-appropriate length caps.
|
||||
// ========================================================================
|
||||
|
||||
public function testRecipientNameCapsAt100Chars(): void
|
||||
{
|
||||
$text = str_repeat('A', 200);
|
||||
$result = economic_export_sanitizer::sanitizeTextLine($text, 100);
|
||||
$this->assertSame(100, mb_strlen($result));
|
||||
$this->assertStringEndsWith('...', $result);
|
||||
}
|
||||
|
||||
public function testRecipientAddressCapsAt250Chars(): void
|
||||
{
|
||||
$text = str_repeat('B', 500);
|
||||
$result = economic_export_sanitizer::sanitizeTextLine($text, 250);
|
||||
$this->assertSame(250, mb_strlen($result));
|
||||
$this->assertStringEndsWith('...', $result);
|
||||
}
|
||||
|
||||
public function testRecipientAddressNewlinesAndSlashesReplaced(): void
|
||||
{
|
||||
// Address with embedded newlines and a slash — both common in EU street formats
|
||||
$input = "Main Street 1\nFloor 2/3\n1234 City";
|
||||
$result = economic_export_sanitizer::sanitizeTextLine($input, 250);
|
||||
$this->assertStringNotContainsString("\n", $result);
|
||||
$this->assertStringNotContainsString('/', $result);
|
||||
$this->assertSame('Main Street 1 Floor 2-3 1234 City', $result);
|
||||
}
|
||||
|
||||
public function testRecipientZipCapsAt20Chars(): void
|
||||
{
|
||||
$text = str_repeat('9', 50);
|
||||
$result = economic_export_sanitizer::sanitizeTextLine($text, 20);
|
||||
$this->assertSame(20, mb_strlen($result));
|
||||
$this->assertStringEndsWith('...', $result);
|
||||
}
|
||||
|
||||
public function testRecipientZipPreservesDanishFormat(): void
|
||||
{
|
||||
// Danish postal codes: "1234" — should pass through unchanged
|
||||
$this->assertSame('1234', economic_export_sanitizer::sanitizeTextLine('1234', 20));
|
||||
}
|
||||
|
||||
public function testRecipientZipHandlesUkFormatWithSlash(): void
|
||||
{
|
||||
// UK postcodes contain no slashes in practice but include spaces
|
||||
$this->assertSame('SW1A 1AA', economic_export_sanitizer::sanitizeTextLine('SW1A 1AA', 20));
|
||||
}
|
||||
|
||||
public function testRecipientCityCapsAt100Chars(): void
|
||||
{
|
||||
$text = str_repeat('C', 200);
|
||||
$result = economic_export_sanitizer::sanitizeTextLine($text, 100);
|
||||
$this->assertSame(100, mb_strlen($result));
|
||||
}
|
||||
|
||||
public function testRecipientCityHandlesDanishSpecialChars(): void
|
||||
{
|
||||
$this->assertSame('København Ø', economic_export_sanitizer::sanitizeTextLine('København Ø', 100));
|
||||
$this->assertSame('Aarhus C', economic_export_sanitizer::sanitizeTextLine('Aarhus C', 100));
|
||||
}
|
||||
|
||||
public function testRecipientNameWithAmpersand(): void
|
||||
{
|
||||
// & should pass through — the sanitizer does not strip XML/HTML entities
|
||||
$this->assertSame('Smith & Sons', economic_export_sanitizer::sanitizeTextLine('Smith & Sons', 100));
|
||||
}
|
||||
|
||||
public function testRecipientNameWithQuotes(): void
|
||||
{
|
||||
// Various quote styles
|
||||
$this->assertSame('"Bob" Inc.', economic_export_sanitizer::sanitizeTextLine('"Bob" Inc.', 100));
|
||||
$this->assertSame("Bob's Trucks", economic_export_sanitizer::sanitizeTextLine("Bob's Trucks", 100));
|
||||
}
|
||||
|
||||
public function testRecipientAddressCrlfNormalized(): void
|
||||
{
|
||||
$this->assertSame('line1 line2', economic_export_sanitizer::sanitizeTextLine("line1\r\nline2", 250));
|
||||
$this->assertSame('line1 line2', economic_export_sanitizer::sanitizeTextLine("line1\rline2", 250));
|
||||
}
|
||||
|
||||
public function testEmptyRecipientFieldsReturnEmpty(): void
|
||||
{
|
||||
$this->assertSame('', economic_export_sanitizer::sanitizeTextLine('', 100));
|
||||
$this->assertSame('', economic_export_sanitizer::sanitizeTextLine('', 250));
|
||||
$this->assertSame('', economic_export_sanitizer::sanitizeTextLine('', 20));
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// EAN sanitization (TRU-193)
|
||||
//
|
||||
// EANs in the recipient block should be digits-only. We use a
|
||||
// preg_replace('/[^0-9]/', '', $ean) in the endpoint, but we also
|
||||
// verify that the text-line sanitizer is safe to apply as a fallback.
|
||||
// ========================================================================
|
||||
|
||||
public function testTextLineSanitizerPreservesAllDigits(): void
|
||||
{
|
||||
$ean = '5798000000001';
|
||||
$this->assertSame($ean, economic_export_sanitizer::sanitizeTextLine($ean, 20));
|
||||
}
|
||||
|
||||
public function testTextLineSanitizerReplacesSpacesInEan(): void
|
||||
{
|
||||
// Real-world data sometimes has "5798 0000 0000 1" with spaces.
|
||||
// The text-line sanitizer keeps a single space (not strictly digit-only);
|
||||
// for true digit-only sanitization, the endpoint uses preg_replace('/[^0-9]/', '', $ean)
|
||||
// directly. The text-line sanitizer is only a defense-in-depth fallback.
|
||||
$this->assertSame('5798 0000 0000 1', economic_export_sanitizer::sanitizeTextLine('5798 0000 0000 1', 20));
|
||||
$this->assertSame('5798 0000 0000 1', economic_export_sanitizer::sanitizeTextLine('5798 0000 0000 1', 20));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,379 @@
|
||||
<?php
|
||||
|
||||
namespace tests\Unit\Economic;
|
||||
|
||||
use helpers\economic_invoice_draft;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use RuntimeException;
|
||||
|
||||
require_once __DIR__ . '/../../../modules/economic/helpers/economic_invoice_draft.php';
|
||||
|
||||
/**
|
||||
* Unit tests for the pre-flight validation in economic_invoice_draft.
|
||||
*
|
||||
* These tests build a draft instance via the skip_fetch path (so we never
|
||||
* hit the e-conomic API), call preflightValidate() directly, and assert
|
||||
* that each rule is enforced.
|
||||
*/
|
||||
class EconomicInvoiceDraftPreflightTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* Build a draft instance in skip_fetch mode. We never hit the network.
|
||||
*/
|
||||
private function makeDraft(bool $preflight = true): economic_invoice_draft
|
||||
{
|
||||
$draft = new economic_invoice_draft(12345, 'DKK', true);
|
||||
// Make the preflight_enabled flag mutable for the disabled test.
|
||||
$reflection = new \ReflectionClass($draft);
|
||||
$prop = $reflection->getProperty('preflight_enabled');
|
||||
$prop->setAccessible(true);
|
||||
$prop->setValue($draft, $preflight);
|
||||
return $draft;
|
||||
}
|
||||
|
||||
private function callPreflight(economic_invoice_draft $draft, array $lines, ?int $orderId = null): void
|
||||
{
|
||||
$draft->preflightValidate($lines, $orderId);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 1: description must be non-empty after trim()
|
||||
// ========================================================================
|
||||
|
||||
public function testEmptyDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], 42);
|
||||
}
|
||||
|
||||
public function testWhitespaceOnlyDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['description' => " \t \n "],
|
||||
], 99);
|
||||
}
|
||||
|
||||
public function testMissingDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['quantity' => 1, 'unitNetPrice' => 10.0],
|
||||
], 1);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 2: description must be <= 250 chars
|
||||
// ========================================================================
|
||||
|
||||
public function testTooLongDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/exceeds 250 chars/');
|
||||
$long = str_repeat('a', 251);
|
||||
$this->callPreflight($draft, [
|
||||
['description' => $long],
|
||||
], 7);
|
||||
}
|
||||
|
||||
public function testDescriptionAt250Passes(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
// 250 chars — should pass (not throw)
|
||||
$this->callPreflight($draft, [
|
||||
['description' => str_repeat('b', 250)],
|
||||
], 8);
|
||||
$this->assertTrue(true); // no exception means success
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 3: productNumber must match /^[A-Za-z0-9._-]{1,50}$/
|
||||
// ========================================================================
|
||||
|
||||
public function testInvalidProductNumberThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/productNumber does not match/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'PROD/01'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 11);
|
||||
}
|
||||
|
||||
public function testProductNumberTooLongThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/productNumber does not match/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => str_repeat('a', 51)],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 12);
|
||||
}
|
||||
|
||||
public function testValidProductNumberPasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'PROD-01.0_v2'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 13);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 4: quantity must be a positive number (> 0)
|
||||
// ========================================================================
|
||||
|
||||
public function testZeroQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 0,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 21);
|
||||
}
|
||||
|
||||
public function testNegativeQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => -1.5,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 22);
|
||||
}
|
||||
|
||||
public function testNonNumericQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 'NaN-ish',
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 23);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 5: unitNetPrice must be a number (>= 0)
|
||||
// ========================================================================
|
||||
|
||||
public function testNegativeUnitPriceThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/unitNetPrice is not a number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => -5.0,
|
||||
],
|
||||
], 31);
|
||||
}
|
||||
|
||||
public function testNonNumericUnitPriceThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/unitNetPrice is not a number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 'free',
|
||||
],
|
||||
], 32);
|
||||
}
|
||||
|
||||
public function testZeroUnitPricePasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
// 0 is allowed — it's "a number >= 0"
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 0,
|
||||
],
|
||||
], 33);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Happy path: a fully-valid line passes
|
||||
// ========================================================================
|
||||
|
||||
public function testValidLinePasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'A normal product line',
|
||||
'product' => ['productNumber' => 'WASH-01'],
|
||||
'quantity' => 2,
|
||||
'unitNetPrice' => 99.5,
|
||||
],
|
||||
[
|
||||
'description' => 'A text-only line',
|
||||
],
|
||||
[
|
||||
'description' => 'Discount',
|
||||
'product' => ['productNumber' => 'TotDiscount'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 0.0,
|
||||
],
|
||||
], 100);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Disabled preflight: bad lines must NOT throw
|
||||
// ========================================================================
|
||||
|
||||
public function testPreflightCanBeDisabled(): void
|
||||
{
|
||||
// The preflight_enabled flag gates addLines() (i.e. it controls whether
|
||||
// preflightValidate() runs before flushLinesInBatches). It does NOT
|
||||
// affect direct calls to preflightValidate(). So this test verifies:
|
||||
// 1. The default value is true.
|
||||
// 2. The flag is mutable to false.
|
||||
// 3. addLines() is wired to short-circuit preflight when the flag is false.
|
||||
$draft = $this->makeDraft(true);
|
||||
|
||||
// (1) Default: preflight is enabled
|
||||
$ref = new \ReflectionClass($draft);
|
||||
$prop = $ref->getProperty('preflight_enabled');
|
||||
$prop->setAccessible(true);
|
||||
$this->assertTrue($prop->getValue($draft), 'preflight_enabled should default to true');
|
||||
|
||||
// (2) Mutable
|
||||
$prop->setValue($draft, false);
|
||||
$this->assertFalse($prop->getValue($draft));
|
||||
|
||||
// (3) Disabled: addLines() must NOT call preflightValidate().
|
||||
// We assert this indirectly: queue a guaranteed-invalid line and then
|
||||
// catch the exception that flushLinesInBatches() would raise when it
|
||||
// tries to send to e-conomic. If preflight were enabled we'd get
|
||||
// RuntimeException("description is empty") first.
|
||||
$this->expectException(\Throwable::class);
|
||||
$reflection = new \ReflectionClass($draft);
|
||||
$linesProp = $reflection->getProperty('draft_lines');
|
||||
$linesProp->setAccessible(true);
|
||||
$linesProp->setValue($draft, [
|
||||
['description' => ''], // would fail rule 1 if preflight ran
|
||||
]);
|
||||
$draft->addLines();
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Multiple errors: report the first one
|
||||
// ========================================================================
|
||||
|
||||
public function testMultipleErrorsReportsFirst(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
// First line is fine
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
// Second line: empty description (rule 1)
|
||||
['description' => ''],
|
||||
// Third line: would also fail, but we should never get here
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'BAD/CHAR'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 300);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught, 'Expected a RuntimeException to be thrown');
|
||||
$this->assertStringContainsString('line 1', $caught->getMessage());
|
||||
$this->assertStringContainsString('description is empty', $caught->getMessage());
|
||||
$this->assertStringContainsString('order 300', $caught->getMessage());
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Exception message includes order id when provided
|
||||
// ========================================================================
|
||||
|
||||
public function testExceptionMessageIncludesOrderId(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], 4242);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught);
|
||||
$this->assertStringContainsString('order 4242', $caught->getMessage());
|
||||
}
|
||||
|
||||
public function testExceptionMessageOmitsOrderWhenNull(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], null);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught);
|
||||
$this->assertStringNotContainsString('order', $caught->getMessage());
|
||||
}
|
||||
}
|
||||
+101
@@ -0,0 +1,101 @@
|
||||
<?php
|
||||
|
||||
namespace tests\Unit\Economic;
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Unit tests for TRU-193 — the recipient-block sanitization in
|
||||
* economic_invoices_drafts_endpoint::add().
|
||||
*
|
||||
* Since the endpoint's `add()` method makes a live HTTP request to
|
||||
* e-conomic, we don't test it directly. Instead we test the building
|
||||
* blocks (sanitizer rules + the file-shape contract) that the endpoint
|
||||
* uses, so the behavior is regression-protected.
|
||||
*/
|
||||
class EconomicInvoiceDraftRecipientSanitizationTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* Verify the endpoint file still references the sanitizer for
|
||||
* the recipient-block fields (defense in depth, even though the
|
||||
* customer data comes from e-conomic).
|
||||
*/
|
||||
public function testEndpointSanitizesRecipientName(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
$this->assertStringContainsString(
|
||||
'sanitizeTextLine($customer->getName() ?? \'Ukendt\', 100)',
|
||||
$content,
|
||||
'recipient.name must be sanitized via sanitizeTextLine with a 100-char cap'
|
||||
);
|
||||
}
|
||||
|
||||
public function testEndpointSanitizesRecipientAddress(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
$this->assertStringContainsString(
|
||||
'sanitizeTextLine($customer->getAddress() ?? \'Ukendt\', 250)',
|
||||
$content,
|
||||
'recipient.address must be sanitized via sanitizeTextLine with a 250-char cap'
|
||||
);
|
||||
}
|
||||
|
||||
public function testEndpointSanitizesRecipientZip(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
$this->assertStringContainsString(
|
||||
'sanitizeTextLine($customer->getZipCode() ?? \'Ukendt\', 20)',
|
||||
$content,
|
||||
'recipient.zip must be sanitized via sanitizeTextLine with a 20-char cap'
|
||||
);
|
||||
}
|
||||
|
||||
public function testEndpointSanitizesRecipientCity(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
$this->assertStringContainsString(
|
||||
'sanitizeTextLine($customer->getCity() ?? \'Ukendt\', 100)',
|
||||
$content,
|
||||
'recipient.city must be sanitized via sanitizeTextLine with a 100-char cap'
|
||||
);
|
||||
}
|
||||
|
||||
public function testEndpointStripsNonDigitsFromEan(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
$this->assertStringContainsString(
|
||||
"preg_replace('/[^0-9]/', '', \$customer_ean)",
|
||||
$content,
|
||||
'recipient.ean must be stripped to digits only'
|
||||
);
|
||||
}
|
||||
|
||||
public function testEndpointOmitsEmptyEanInsteadOfSendingEmptyString(): void
|
||||
{
|
||||
$path = __DIR__ . '/../../../modules/economic/endpoints/invoices/economic_invoices_drafts_endpoint.php';
|
||||
$content = file_get_contents($path);
|
||||
$this->assertNotFalse($content);
|
||||
// After stripping non-digits, if the result is empty we should remove the key
|
||||
$this->assertStringContainsString(
|
||||
"unset(\$recipient['ean']);",
|
||||
$content,
|
||||
'recipient.ean must be removed from the payload when the sanitized EAN is empty'
|
||||
);
|
||||
// Verify the conditional structure: if empty, unset
|
||||
$this->assertMatchesRegularExpression(
|
||||
"/\\\$recipient\\['ean'\\]\\s*=\\s*preg_replace\\(\\s*['\\/\\^0-9\\/']/",
|
||||
$content,
|
||||
'recipient.ean must be assigned via preg_replace with a non-digit-stripping pattern'
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user