fix: Prevent double charges and duplicate events in the checkout flow

[sveneberth]

Problem

1. Double charge: checkout_order guards only via can_order, which checks is_ordered — but that flag is set after the payment provider checkout() call:

   pp_res = self.get_payment_provider_by_name(...).checkout(order_skel)  # external charge
   order_skel = self.set_ordered(order_skel, pp_res)                     # flag set here

   If the process crashes in between, or a double click / replayed request races the first call, is_ordered is still falsy and a repeated checkout_order calls the payment provider again — a second charge.

2. Duplicate events: set_ordered / set_paid / set_rts unconditionally wrote the state and fired their events. When the return handler and the (60s-delayed) payment webhook process the same payment, both saw is_paid=False (read-then-write, no precondition) and both fired ORDER_PAID — side effects like discount accounting (mark_discount_used on ORDER_ORDERED) or mails ran twice.

3. Unreachable raise statements after return JsonResponse(...) in checkout_start and checkout_order; the latter referenced an undefined variable (error_) and would have been a NameError if ever reached.

Solution

• New OrderSkel.checkout_order_started (DateBone): checkout_order claims the order via a toolkit.set_status precondition before calling the payment provider. A concurrent or repeated call is rejected with HTTP 409. The claim is released when the provider call raises (immediate retry possible) and expires after 15 minutes (CHECKOUT_ORDER_CLAIM_TIMEOUT) to recover from crashes after the provider call.
• New _set_state_once helper: set_ordered / set_paid / set_rts perform their transition with a precondition and fire their events only on the actual transition. A repeated call changes and triggers nothing.
• Removed the dead raise lines.
• Docstrings added at all touched methods describing the idempotency guarantees.

Note

toolkit.set_status currently never opens a transaction due to a bug (if db.IsInTransaction: — missing call parentheses, see viur-framework/viur-toolkit#65). The precondition pattern in this PR already narrows the race window considerably without it and becomes fully transactional once that fix is released.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens the checkout flow against double charges and duplicate side effects by introducing an explicit “checkout claim” and making order state transitions idempotent.

Changes:

• Adds checkout_order_started to claim an order before calling the payment provider, rejecting concurrent/replayed checkout_order attempts.
• Introduces _set_state_once so set_ordered / set_paid / set_rts only transition and fire their events once.
• Removes unreachable raise statements after JSON error responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/viur/shop/skeletons/order.py	Adds checkout_order_started timestamp used to claim an order during final checkout.
src/viur/shop/modules/order.py	Implements claim logic + idempotent state transitions; removes dead raises in checkout endpoints.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sveneberth]

Fixed in ff8ac20: removed the extra EVENT_SERVICE.call(Event.ORDER_CHANGED, ...); set_ordered() already fires it on the actual transition.

[sveneberth]

Fixed in ff8ac20: UID assignment is now guarded with if not order_skel["order_uid"]: so a retry after an overtaken claim no longer reassigns it.

[sveneberth]

Fixed in ff8ac20: the release now uses a precondition comparing checkout_order_started against the timestamp this request itself claimed, so it only clears its own claim and no longer clobbers a newer one from a concurrent retry.