Files
api/services/nginx/app/routes/userInvoicesRoute.php
T
Jeppe BOpenClaw Backend AgentJeppe Bjeppemaxclaw[bot] <bot@jeppemaxclaw.local>Bugfix Subagent
60222a7d91 fix(api): clarify user-invoice PUT validation so customers can invoice (TRU-128) (#382)
## Summary

Fixes **TRU-128** ("Jeg kan ikke fakturere") — a customer in
#afdelingsansvarlige could not invoice because the customer-facing
invoice PUT endpoint returned a misleading 400 error.

## Root cause

`PUT /collected-invoices` in
`services/nginx/app/routes/userInvoicesRoute.php` had two related bugs:

1. **Misleading error message** — the 'both fields missing' guard
errored with
   `'Missing required parameters: po_number, closed_at'`, which reads as
   if BOTH fields are required. The actual condition (`&&`) only fires
   when neither is set, so only one is required. Customers who tried
   different combinations kept getting the same error and concluded the
   system was broken.

2. **Inconsistent `closed_at` clearing** — the 'forbidden closed_at for
   non-superusers' guard fired for ANY present `closed_at` key,
   including `null` and `""`. That blocked customers from CLEARING a
   previously-set `closed_at`, even though the handler further down
   already nulls the field when it receives an empty value.

## Fix

- Reword the missing-fields error to state the actual contract:
  *"At least one of po_number or closed_at must be provided"*.
- Narrow the forbidden guard to *non-empty* `closed_at`, so customers
  can still pass `null` / `""` to clear a previously-set value.
  The clear-on-null/empty logic further down in the handler is unchanged
  — the guard now matches it.

## Test

`tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php`
- Locks in the new error message.
- Locks in the new `$closed_at_is_non_empty` guard shape with the
  `if (self::isParametersSet(['closed_at'])) { ... }` pre-check.
- Locks in the regression: the previous 'any present closed_at -> 403'
  pattern is explicitly asserted to be absent.

## Files changed

- `services/nginx/app/routes/userInvoicesRoute.php`
-
`services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php`

## Refs

- TRU-128
- Slack: #afdelingsansvarlige (kunde-rapport)

---------

Co-authored-by: OpenClaw Backend Agent <agent@openclaw.ai>
Co-authored-by: Jeppe B <jeppe@copenhagentruckwash.io>
Co-authored-by: jeppemaxclaw[bot] <bot@jeppemaxclaw.local>
Co-authored-by: Bugfix Subagent <bugfix-subagent@openclaw.local>
2026-08-16 18:20:03 +02:00

128 lines
5.8 KiB
PHP

<?php
namespace routes;
use classes\authentication;
use objects\collected_order_invoices_o;
use objects\logs_o;
use traits\route_t;
class userInvoicesRoute
{
use route_t;
public function run(): void
{
$this->get('/user/invoices', function () {
// Require the user to be logged in
global $response;
$this->requirePermission('user_invoices');
$user = (new authentication())->get_user();
if (!$user) {
(new logs_o())->add('user_invoices', 'global', 0, 0, 'USER_INVOICES', 'User not logged in');
$response->error('Invalid session', 400);
}
// Return the invoices owned by the user
$collected_order_invoices = new collected_order_invoices_o();
$result = $collected_order_invoices->listObjectsWithPaginationIfSet(
function ($invoice_collection_array) {
$tmp_invoice = (new collected_order_invoices_o())->select((int)$invoice_collection_array['id']);
return [
...$tmp_invoice->asArray(),
];
},
$collected_order_invoices->forceRestrictFilters(
[
// This makes sure that the user can only see orders from the departments they explicitly have access to
'customer_number' => $user->customer_number->value()
]
)
);
$response->success($result);
},
[
'user_invoices' => 'Get the invoices of the user',
]
);
$this->put('/collected-invoices', function () {
// Require the user to be logged in
global $response;
$this->requirePermission('user_invoices');
$user = (new authentication())->get_user();
if (!$user) {
(new logs_o())->add('user_invoices', 'global', 0, 0, 'USER_INVOICES', 'User not logged in');
$response->error('Invalid session', 400);
}
self::requireParameters(['id']);
self::requireType((int)self::getParameter('id'), self::type_int());
$id = (int)self::getParameter('id');
// Make sure the id is valid
self::requireMinValue($id, 1);
self::requireSameLength($id, self::getParameter('id'));
$is_superuser = $this->hasPermission('superuser');
if (!self::isParametersSet(['po_number']) && !self::isParametersSet(['closed_at'])) {
// At least one of po_number or closed_at must be provided. The
// previous message said "Missing required parameters:
// po_number, closed_at" which read as if BOTH were required
// and confused customers trying to invoice (TRU-128).
$response->error('At least one of po_number or closed_at must be provided', 400);
}
// Only superusers may set a non-empty closed_at. Customers are
// still allowed to pass an empty/null closed_at to CLEAR a
// previously set value (the field is then set to null below).
$closed_at_is_non_empty = false;
if (self::isParametersSet(['closed_at'])) {
$raw_closed_at = self::getParameter('closed_at');
$closed_at_is_non_empty = ($raw_closed_at !== null && $raw_closed_at !== '');
}
if ($closed_at_is_non_empty && !$is_superuser) {
$response->error('Forbidden: only superusers can update closed_at', 403);
}
// Make sure optional fields are valid
if (self::isParametersSet(['po_number'])) {
self::requireType((string)self::getParameter('po_number'), self::type_string());
self::requireMinLength('po_number', 0);
self::requireMaxLength('po_number', 255);
}
$closed_at = null;
if (self::isParametersSet(['closed_at'])) {
$closed_at = self::getParameter('closed_at');
if ($closed_at !== null && $closed_at !== '') {
self::requireType((string)$closed_at, self::type_string());
self::requireDateFormat((string)$closed_at, self::FORMAT_DATE());
}
}
// Get the invoice
$collected_order_invoices = new collected_order_invoices_o();
$invoice = $collected_order_invoices->select((int)$id);
$invoice->requireSelected();
// Make sure the invoice belongs to the user
if ((int)$invoice->customer_number->value() !== (int)$user->customer_number->value() && !$is_superuser) {
(new logs_o())->add(
'user_invoices',
'global',
0,
0,
'USER_INVOICES',
'User not allowed to access this invoice (invoice_customer=' . (int)$invoice->customer_number->value() . ', user_customer=' . (int)$user->customer_number->value() . ')'
);
$response->error('Forbidden: invoice does not belong to authenticated user', 403);
}
// Update the invoice
if (self::isParametersSet(['po_number'])) {
$invoice->po_number->set((string)self::getParameter('po_number'));
}
if (self::isParametersSet(['closed_at'])) {
$invoice->closed_at->set($closed_at === null || $closed_at === '' ? null : date('Y-m-d 23:59:59', strtotime((string)$closed_at . ' 00:00:01')));
}
// Return success
$response->success($invoice->asArray());
},
[
'user_invoices' => 'Get the invoices of the user',
]
);
}
}