Verdict: Solid security improvement, correct implementation. Minor suggestions below.
💡 Redundant = 49 initializer + int money type. The 49 default lives in three places (the :49, the field init, and application.yml), and the = 49 masks @Value failures in the unit test (Mockito's @InjectMocks never resolves @Value, so the test rides on the initializer). Consider private BigDecimal letterPrice; and order.setAmountPaid(letterPrice) directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already scale=2). See review body for the full test-coverage note.
♻️ The setStatus -> setAmountPaid -> save -> notify -> return block is duplicated in confirmPayment (line 102). Extract private Order markPaidAndProcessing(Order order) to collapse it (the duplication pre-existed but is deepened here).
⚠️ amountPaid here is the configured letterPrice, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
🧪 This assertion passes via the = 49 field initializer, not via @Value: @InjectMocks doesn't resolve Spring @Value, so this test cannot catch a misnamed app.payment.letter-price PropertySource or a removed @Value. The guest-path twin at line 350 has the same gap. Strengthen with ReflectionTestUtils.setField(orderService, "letterPrice", 99) + assert 99, or a @SpringBootTest slice that asserts the resolved config differs from a sentinel.
Verdict: Approve-worthy fix — amountPaid is now persisted on both payment-confirmation paths with state-guarded idempotency and matching tests. A few advisory nits; none block merge.
💡 Suggestion (non-blocking): worth a @DataJpaTest that asserts this constraint rejects invalid rows — persist an Order with both userId and guestToken set, expecting a DataIntegrityViolationException referencing chk_user_or_guest. The @PrePersist in Order.onCreate() already prevents the neither-set case at the application layer, so the both-set rejection is the untested, highest-value path.
Verdict: Looks good — clean, correct, well-documented migration. Ship it.
💡 Suggestion: The test asserts margin, width, and color but not errorCorrectionLevel: 'M'. Consider adding expect(options.errorCorrectionLevel).toBe('M') to GuestPaymentRedirect.spec.ts to guard against silent regression on this parameter.
💡 Suggestion (DRY): The QR options object (width, margin, errorCorrectionLevel, color) is now duplicated verbatim between this file and PaymentRedirect.vue. This duplication is how the original bug happened. Consider extracting a shared QR_OPTIONS constant (e.g., in @/api/payment.ts) so both payment pages use the same source of truth.