🟠 Honor-system pay — no payment verification. confirmGuestPayment marks the order PROCESSING without confirming any Swish payment landed. Anyone with the token (or the customer) can mark an order paid without paying → BilHej mails a free letter. Acknowledged as Phase 0 and consistent with the authenticated confirmPayment, but harden with Swish Commerce callback verification before real money. Also: amountPaid / PAID status are never set on either path.
🟠 Guest token in the URL query string. The token is the customer's sole credential but here it's pushed into ?token=…, so it lands in browser history and the nginx access log (and risks Referer leakage). The magic-link /gast-order/:token is inherently URL-based, but this payment page needn't be — prefer sessionStorage / Pinia store / route state (or a #fragment) to keep the credential out of the query string.
🔵 Client email regex weaker than backend @Email. /\S+@\S+\.\S+/ lets a@b / x@.y through client-side, only to fail server-side with a confusing message. Mirror a stricter pattern or drop the client check and surface the backend @Email message.
🔵 Unused index. idx_orders_guest_email is created but findByGuestEmail was never added to OrderRepository (the PR body lists it, but only findByGuestToken is there), so there's no read path on guest_email yet. Either add the lookup now or defer the index until the email-link phase to avoid an unused schema object.
Verdict — solid, well-structured guest checkout. The security model (opaque 122-bit token, partial unique index, refuse-to-serve user-owned orders via the guest path) is thoughtful and the backend cleanly parallels OrderController without touching the JWT path. The main concerns are an unauthenticated create endpoint with no abuse protection, the guest token riding in the URL query string, and the honor-system /pay that should be hardened before real money. Advisory only — no blocking changes requested.
CI fix: added backend unit tests for guest checkout
Root cause: The PR added 3 new OrderService methods, GuestOrderController (3 endpoints), and 2 DTO records — all without unit…
🔴 Critical — honor-system payment on a public route. confirmGuestPayment transitions PENDING_PAYMENT → PROCESSING purely on the client's button click, with no Swish payment verification. Pre-existing on the authed confirmPayment, but this guest endpoint is JWT-less, so the only barrier is knowing the guest token. At minimum set amountPaid (and a paid_at) here so finance can reconcile against the Swish payout report; ideally gate the transition on a Swish Commerce payment request + callback.
🟢 No tests for the new guest methods. OrderServiceTest already covers createOrder/confirmPayment/cancelOrder/updatePendingOrder with the existing Mockito pattern. Please mirror that for createGuestOrder (plate uppercased, email lowercased, guestToken set via onCreate), getOrderByGuestToken (404 when userId is non-null — the defensive guard deserves a regression test of its own), and confirmGuestPayment (transition + notifyOrderProcessing invocation).
🟡 Missing DB-level orphan guard. The Order Javadoc asserts "Either userId or guestToken is set; never both, never neither" — but only onCreate() enforces this in Java. A stray INSERT (admin tooling, future script) can violate it. Suggest adding after the column additions:
Verdict: Well-structured guest-checkout scaffold — validation parity with the authed flow, defensive token lifecycle (only guest orders get a token, lookups refuse to serve user-owned orders), and a partial-unique-index migration that is backfill-safe. Frontend type-checks pass per the PR. Two blockers worth resolving before merge: the payment-confirmation path is honor-system (carried over from the authed path, but now on a public, JWT-less route), and the three new OrderService methods have no tests.
💡 Suggestion: Good resilience pattern — the silent catch degrades gracefully. Consider adding a test for this path: mockToDataURL.mockRejectedValue(...) → assert the Swish payment link and manual fallback still render.
💡 Suggestion: The comment says "a leading +" but /[\s+]/g strips all + characters anywhere in the string, not just a leading one. This is harmless for real phone numbers, but the comment slightly misrepresents the behaviour. Consider number.replace(/^\+/, '').replace(/\s/g, '') or adjusting the comment to say "any + characters".
Verdict: LGTM — solid fix for a real production bug. QR params now match the ISO/IEC 18004 spec, and the error-handling refactor is a good resilience improvement.