Feature: Updating Listreners, Separating Logic from listeners, Queuing Policies

This commit is contained in:
2026-09-16 23:24:02 +03:00
parent a411e6bbc1
commit a55697ce82
25 changed files with 445 additions and 163 deletions
@@ -2,12 +2,19 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Contracts\Queue\ShouldQueue;
use Modules\Core\Order\Events\OrderDispatched;
use Modules\Core\Order\Services\OrderStatusFlow;
use Modules\Core\Order\Services\OrderStatusWriter;
use Modules\Core\Shipping\Enums\TrackingStatus;
use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier;
/**
* Queued — see Modules\Core\Order\Listeners\DeriveOrderDeliveredFromShipment's
* own docblock: ShipmentStatusUpdatedByCarrier comes from a scheduled
* polling job, not a webhook, so nothing needs this to complete before a
* request returns.
*
* The automatic half of "Dispatched" — the manual fallback is the staff
* "Update Status" action (Modules\Core\Shipping\Extensions\
* OrderViewExtension). Listens to ShipmentStatusUpdatedByCarrier directly,
@@ -19,14 +26,17 @@ use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier;
* carrier that skips straight there without a distinct collection
* checkpoint.
*
* Guarded to only fire from 'ready_for_dispatch' — a late/duplicate
* checkpoint, or an order the manual action already advanced, is a
* silent no-op.
* Guarded by OrderStatusFlow::isValidTransition() rather than a hardcoded
* "only fire from 'ready_for_dispatch'" comparison — the single source of
* truth for the status graph lives there, not duplicated here. A
* late/duplicate checkpoint, or an order the manual action already
* advanced, is a silent no-op either way.
*/
class AdvanceFulfillmentOnCarrierCheckpoint
class AdvanceFulfillmentOnCarrierCheckpoint implements ShouldQueue
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
) {}
public function handle(ShipmentStatusUpdatedByCarrier $event): void
@@ -38,7 +48,7 @@ class AdvanceFulfillmentOnCarrierCheckpoint
$order = $event->shipmentInfo->shipment->order;
if (! $order || $order->status !== 'ready_for_dispatch') {
if (! $order || ! $this->flow->isValidTransition($order, 'dispatched')) {
return;
}
@@ -2,10 +2,18 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Contracts\Queue\ShouldQueue;
use Modules\Core\Order\Events\OrderDelivered;
use Modules\Core\Order\Services\OrderStatusFlow;
use Modules\Core\Order\Services\OrderStatusWriter;
/**
* Queued — OrderDelivered is only ever dispatched from Modules\Core\Order\
* Listeners\DeriveOrderDeliveredFromShipment, itself queued (see that
* class's own docblock: the triggering ShipmentStatusUpdatedByCarrier
* comes from a scheduled polling job, not a request with a page waiting
* on the result).
*
* Writes `status` to 'delivered' once a carrier confirms delivery, rather
* than jumping straight to 'completed'. Carrier orders get a return
* window between delivery and completion (see Modules\Core\Order\
@@ -20,21 +28,23 @@ use Modules\Core\Order\Services\OrderStatusWriter;
* OrderDelivered — deriving "was this delivered" and acting on it by
* writing `status` are deliberately two different listeners.
*
* Guarded to only fire from 'dispatched' — a duplicate/late Delivered
* Guarded by OrderStatusFlow::isValidTransition() rather than a hardcoded
* "only fire from 'dispatched'" comparison. A duplicate/late Delivered
* checkpoint, or an order a manual action already moved past, is a
* silent no-op.
* silent no-op either way.
*/
class AdvanceFulfillmentOnDelivered
class AdvanceFulfillmentOnDelivered implements ShouldQueue
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
) {}
public function handle(OrderDelivered $event): void
{
$order = $event->order;
if ($order->status !== 'dispatched') {
if (! $this->flow->isValidTransition($order, 'delivered')) {
return;
}
@@ -2,13 +2,8 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Support\Facades\Event;
use Lunar\Models\Order;
use Modules\Core\Checkout\Events\OrderPlaced;
use Modules\Core\Order\Enums\PaymentStatus;
use Modules\Core\Order\Services\OrderStatusFlow;
use Modules\Core\Order\Services\OrderStatusWriter;
use Modules\Core\Order\Support\OrderStatus;
use Modules\Core\Order\Services\OrderPaymentResolutionService;
use Modules\Core\Payment\Events\PaymentAuthorized;
use Modules\Core\Payment\Events\PaymentCaptured;
use Modules\Core\Payment\Events\PaymentRefunded;
@@ -17,38 +12,24 @@ use Modules\Core\Payment\Events\PaymentRefunded;
* Registered against PaymentCaptured, PaymentAuthorized, AND
* PaymentRefunded (see OrderServiceProvider).
*
* PaymentCaptured writes both Order::paid/paid_at (via
* OrderStatusWriter::markPaid()) AND advances `status` out of
* 'awaiting_payment' to the next step in the order's flow (see
* OrderStatusFlow::nextOptions()) — re-confirmed with the user: a
* captured payment, manual or via Stripe's webhook, should never leave an
* order sitting at 'awaiting_payment'. Only fires when status is still
* exactly 'awaiting_payment', so a duplicate/delayed capture event never
* regresses an order staff already advanced further. PaymentAuthorized
* only marks paid — an authorization is not yet captured funds, so
* status stays put until the actual capture.
*
* A refund still moves `status` (returned -> refunded/partially_refunded)
* — refunds are a normal step in Modules\Core\Order\Services\
* OrderStatusFlow's own sequence, unlike captures. Derives
* Refunded/PartialRefund from Modules\Core\Order\Support\OrderStatus::
* payment() — the existing, unchanged derived-enum logic, reused rather
* than reimplemented.
*
* Reads $event->context['order_id'] to find which Order this outcome
* belongs to — Payment has no concept of an Order.
*
* Dispatches Checkout\Events\OrderPlaced itself, once placed_at is set.
* Never fires from the PaymentRefunded path — a refund can only ever
* happen after an order was already placed.
* A thin reactor — resolves which Order this outcome belongs to (Payment
* has no concept of an Order, so this reads $event->context['order_id'])
* and hands off to Modules\Core\Order\Services\
* OrderPaymentResolutionService for the actual decisions: whether to mark
* the order paid, whether/how far to advance `status`, and what a refund
* does to it. See that service's own docblock, and its methods' own
* docblocks, for the full business reasoning (re-confirmed with the
* user): a captured payment, manual or via Stripe's webhook, should
* never leave an order sitting at 'awaiting_payment'; an authorization
* only marks paid, since it isn't yet captured funds; a refund is a
* normal step in the order's own status sequence, unlike a capture.
*
* Deliberately does NOT react to PaymentVoided.
*/
class ApplyResolvedPaymentStatus
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
private readonly OrderPaymentResolutionService $resolution,
) {}
public function handle(PaymentCaptured|PaymentAuthorized|PaymentRefunded $event): void
@@ -62,58 +43,11 @@ class ApplyResolvedPaymentStatus
$order = Order::findOrFail($orderId);
if ($event instanceof PaymentRefunded) {
$this->applyRefund($order, $event);
$this->resolution->resolveRefund($order, $event::class);
return;
}
$wasPlaced = ! blank($order->placed_at);
$this->writer->markPaid($order, $event::class);
if ($event instanceof PaymentCaptured) {
$this->advancePastAwaitingPayment($order, $event);
}
if (! $wasPlaced) {
$order->update(['placed_at' => $order->placed_at ?? now()]);
Event::dispatch(new OrderPlaced($order));
}
}
private function advancePastAwaitingPayment(Order $order, PaymentCaptured $event): void
{
if ($order->status !== 'awaiting_payment') {
return;
}
$next = $this->flow->nextOptions($order);
$target = array_key_first($next);
if ($target !== null) {
$this->writer->write($order, $target, $event::class);
}
}
/**
* Requires the refund Transaction row to already exist (Modules\Core\
* Order\Listeners\RecordPaymentTransaction must run first — see
* OrderServiceProvider's listener registration order for
* PaymentRefunded), so the relation is refreshed here rather than
* trusted from a possibly-stale $order instance.
*/
private function applyRefund(Order $order, PaymentRefunded $event): void
{
$order->load('transactions');
$target = match (OrderStatus::payment($order)) {
PaymentStatus::Refunded => 'refunded',
PaymentStatus::PartialRefund => 'partially_refunded',
default => null,
};
if ($target !== null && $order->status !== $target) {
$this->writer->write($order, $target, $event::class);
}
$this->resolution->resolveCaptureOrAuthorization($order, $event::class, isCapture: $event instanceof PaymentCaptured);
}
}
@@ -4,9 +4,19 @@ namespace Modules\Core\Order\Listeners;
use Modules\Core\Order\Events\OrderCompleted;
use Modules\Core\Order\Events\OrderPickedUp;
use Modules\Core\Order\Services\OrderStatusFlow;
use Modules\Core\Order\Services\OrderStatusWriter;
/**
* Deliberately NOT queued — OrderPickedUp is dispatched from a staff
* Filament action (see OrderFulfillmentService::markPickedUp()), and the
* page staff are looking at needs to show `status` as 'completed'
* immediately after they click, not still 'picked_up' until a queue
* worker catches up. Unlike ShipmentStatusUpdatedByCarrier's listeners
* (queued — dispatched from a scheduled polling job with no page waiting
* on the result), this one has a real same-request/same-page-load
* dependency.
*
* The store-pickup mirror of AdvanceFulfillmentOnDelivered — reacts to
* OrderPickedUp (dispatched by Modules\Core\Order\Services\
* OrderFulfillmentService::markPickedUp() the moment staff confirm the
@@ -15,20 +25,22 @@ use Modules\Core\Order\Services\OrderStatusWriter;
* business design — unlike the carrier branch, there is no 'delivered'
* intermediate value on this path.
*
* Guarded to only fire from 'picked_up' — a duplicate dispatch (e.g. a
* stale page re-submitting the action) is a silent no-op.
* Guarded by OrderStatusFlow::isValidTransition() rather than a hardcoded
* "only fire from 'picked_up'" comparison. A duplicate dispatch (e.g. a
* stale page re-submitting the action) is a silent no-op either way.
*/
class CompleteOrderOnPickedUp
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
) {}
public function handle(OrderPickedUp $event): void
{
$order = $event->order;
if ($order->status !== 'picked_up') {
if (! $this->flow->isValidTransition($order, 'completed')) {
return;
}
@@ -2,63 +2,38 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Support\Facades\DB;
use Lunar\Models\Product;
use Lunar\Models\ProductVariant;
use Modules\Core\Catalog\Services\StockService;
use Modules\Core\Checkout\Events\OrderPlaced;
/**
* The only place ProductVariant::stock is written as a result of an order —
* fires once per order regardless of capture_mode/driver, same reasoning as
* Modules\Core\Order\Notifications\OrderPlacedNotification: OrderPlaced is
* dispatched exactly once, from the one place an order's placed_at
* actually gets set (Modules\Core\Order\Listeners\ApplyResolvedPaymentStatus),
* so this can't double-decrement across a capture/authorize/refund sequence
* the way listening to PaymentCaptured directly could.
* Deliberately NOT queued — unlike this codebase's other queued side
* effects (cache flushes, audit logs, search reindexes), a stalled queue
* here isn't just cosmetic staleness: it widens the window in which
* another order can be accepted against stock this order already
* committed (Lunar has no stock-reservation step at checkout time to
* begin with — see StockService's own "Never lets stock go negative"
* note — so some oversell race already exists, but a queue stall of
* minutes/hours extends that window far past the sub-millisecond one a
* synchronous write leaves open). StockService's atomic `GREATEST(stock -
* qty, 0)` SQL still protects against a LOST update between two orders
* decrementing the same variant concurrently; running it synchronously
* keeps the exposure window as small as possible on top of that.
*
* Only decrements for `purchasable === 'in_stock'` variants — 'always' and
* 'backorder' variants are deliberately allowed to sell past (or without
* regard to) their stock count already (see ProductVariant::
* canBeFulfilledAtQuantity()), so decrementing their stock would just make
* that column an inaccurate, decreasingly-negative number with no purchasing
* consequence. Only `OrderLine::type === 'physical'` lines are considered —
* a digital line has no stock to decrement (ProductVariant::getType()).
*
* A single UPDATE per variant (`DB::table(...)->decrement()`), not a
* read-then-write on the Eloquent model — avoids a lost-update race between
* two orders decrementing the same variant concurrently, and skips
* Modules\Core\Catalog\Services\ProductIndexer::stock's staleness gap for
* the DB value itself even though the search index still only refreshes on
* the next reindex event/nightly job (see that class's own docblock).
*
* Never lets stock go negative (`GREATEST(stock - qty, 0)` via a raw
* expression) — an order can still be placed against a variant whose stock
* was already fully consumed by another concurrent order (Lunar has no
* stock-reservation step at cart/checkout time), so this is a best-effort
* count, not a hard inventory guarantee.
* The actual decrement logic lives in Modules\Core\Catalog\Services\
* StockService — stock (the column, its invariants) is a Catalog concern,
* not an Order one; this listener is just the "an order was placed"
* trigger. Fires once per order regardless of capture_mode/driver, same
* reasoning as Modules\Core\Order\Notifications\OrderPlacedNotification:
* OrderPlaced is dispatched exactly once, from the one place an order's
* placed_at actually gets set (Modules\Core\Order\Listeners\
* ApplyResolvedPaymentStatus), so this can't double-decrement across a
* capture/authorize/refund sequence the way listening to PaymentCaptured
* directly could.
*/
class DecrementStockOnOrderPlaced
{
public function handle(OrderPlaced $event): void
{
$lines = $event->order->lines()
->where('type', 'physical')
->where('purchasable_type', ProductVariant::morphName())
->get(['purchasable_id', 'quantity']);
foreach ($lines as $line) {
DB::table((new ProductVariant())->getTable())
->where('id', $line->purchasable_id)
->where('purchasable', 'in_stock')
->update([
'stock' => DB::raw('GREATEST(stock - '.(int) $line->quantity.', 0)'),
]);
}
$productIds = ProductVariant::whereIn('id', $lines->pluck('purchasable_id'))
->pluck('product_id')
->unique();
Product::whereIn('id', $productIds)->get()->each->searchable();
app(StockService::class)->decrementForOrder($event->order);
}
}
@@ -2,17 +2,23 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Contracts\Queue\ShouldQueue;
use Modules\Core\Order\Events\OrderDelivered;
use Modules\Core\Shipping\Enums\TrackingStatus;
use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier;
/**
* Queued — ShipmentStatusUpdatedByCarrier is dispatched from
* Modules\Core\Shipping\Jobs\PollShipmentTrackingJob, a scheduled job with
* no HTTP request waiting on a response, so there is no same-request
* timing pressure for any of this event's listeners (unlike a webhook).
*
* Translates a carrier tracking checkpoint into OrderDelivered — the event
* OrderDeliveredNotification (via NotificationRegistry) actually listens
* to. Kept separate from the notification itself so the "is this checkpoint
* a delivery" filtering doesn't leak into notification code.
*/
class DeriveOrderDeliveredFromShipment
class DeriveOrderDeliveredFromShipment implements ShouldQueue
{
public function handle(ShipmentStatusUpdatedByCarrier $event): void
{
@@ -2,20 +2,28 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Contracts\Queue\ShouldQueue;
use Modules\Core\Order\Services\OrderStatusFlow;
use Modules\Core\Order\Services\OrderStatusWriter;
use Modules\Core\Shipping\Enums\TrackingStatus;
use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier;
/**
* Queued — see Modules\Core\Order\Listeners\DeriveOrderDeliveredFromShipment's
* own docblock: ShipmentStatusUpdatedByCarrier comes from a scheduled
* polling job, not a webhook.
*
* Wires TrackingStatus::Failed to the 'delivery_failed' status for the
* first time — previously an unused enum case. Guarded to only fire from
* 'dispatched': a stale/duplicate checkpoint, or an order a manual action
* already moved past, is a silent no-op.
* first time — previously an unused enum case. Guarded by
* OrderStatusFlow::isValidTransition() rather than a hardcoded "only fire
* from 'dispatched'" comparison. A stale/duplicate checkpoint, or an
* order a manual action already moved past, is a silent no-op either way.
*/
class MarkDeliveryFailedOnCarrierCheckpoint
class MarkDeliveryFailedOnCarrierCheckpoint implements ShouldQueue
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
) {}
public function handle(ShipmentStatusUpdatedByCarrier $event): void
@@ -26,7 +34,7 @@ class MarkDeliveryFailedOnCarrierCheckpoint
$order = $event->shipmentInfo->shipment->order;
if (! $order || $order->status !== 'dispatched') {
if (! $order || ! $this->flow->isValidTransition($order, 'delivery_failed')) {
return;
}
@@ -10,6 +10,17 @@ use Modules\Core\Payment\Events\PaymentRefunded;
use Modules\Core\Payment\Events\PaymentVoided;
/**
* Deliberately NOT queued, despite looking like a pure audit-trail write
* with no same-request reader — Modules\Core\Providers\
* OrderServiceProvider registers this to run BEFORE
* Modules\Core\Order\Listeners\ApplyResolvedPaymentStatus for
* PaymentRefunded specifically, because that listener's refund-status
* resolution reads the Transaction row this listener just wrote. Queueing
* this would run it asynchronously while ApplyResolvedPaymentStatus (sync)
* proceeds immediately, almost certainly executing before the queued job
* and silently breaking that read. See OrderServiceProvider's own
* registration-order comment.
*
* Writes the Transaction row for a successful payment outcome — the
* "record what happened" half of reacting to Payment's events, separate
* from Modules\Core\Order\Listeners\ApplyResolvedPaymentStatus's "update
@@ -2,11 +2,16 @@
namespace Modules\Core\Order\Listeners;
use Illuminate\Contracts\Queue\ShouldQueue;
use Modules\Core\Order\Events\OrderPaidChanged;
use Modules\Core\Order\Events\OrderStatusChanged;
use Modules\Core\Order\Services\OrderStatusTransitionRecorder;
/**
* Queued — a pure history-log write with no same-request reader anywhere
* in the codebase (no Filament page renders order_status_transitions
* immediately after a status change; it's browsed later, if at all).
*
* The one place order_status_transitions rows actually get written —
* listens to OrderStatusChanged (every write of the single `status`
* column, via Modules\Core\Order\Services\OrderStatusWriter::write()) and
@@ -15,7 +20,7 @@ use Modules\Core\Order\Services\OrderStatusTransitionRecorder;
* one consistent audit trail entry ('paid', with a null from_status)
* rather than a second, separate table.
*/
class RecordStatusTransition
class RecordStatusTransition implements ShouldQueue
{
public function __construct(
private readonly OrderStatusTransitionRecorder $recorder,
@@ -0,0 +1,90 @@
<?php
namespace Modules\Core\Order\Services;
use Illuminate\Support\Facades\Event;
use Lunar\Models\Order;
use Modules\Core\Checkout\Events\OrderPlaced;
use Modules\Core\Order\Enums\PaymentStatus;
use Modules\Core\Order\Support\OrderStatus;
/**
* The actual business decisions behind reacting to a payment outcome —
* previously these lived entirely inside Modules\Core\Order\Listeners\
* ApplyResolvedPaymentStatus, a listener with no Service behind it, even
* though "should this order be marked paid," "should its status advance,
* and to what," and "what does a refund do to status" are all genuine
* decisions about Order state, not side effects of Payment's own events.
* That listener is now a thin reactor: extract the order id from
* $event->context, load the Order, call this service, done.
*
* See ApplyResolvedPaymentStatus's own docblock for the full business
* reasoning (re-confirmed with the user) behind each rule enforced here —
* this class only re-documents what's specific to the decision logic
* itself, not the "why" already recorded there.
*/
class OrderPaymentResolutionService
{
public function __construct(
private readonly OrderStatusWriter $writer,
private readonly OrderStatusFlow $flow,
) {}
/**
* A captured or authorized payment: marks the order paid (capture
* only — an authorization is not yet captured funds), advances status
* out of 'awaiting_payment' (capture only), and marks the order
* placed if this is the first payment outcome it's seen.
*/
public function resolveCaptureOrAuthorization(Order $order, string $causeClass, bool $isCapture): void
{
$wasPlaced = ! blank($order->placed_at);
$this->writer->markPaid($order, $causeClass);
if ($isCapture) {
$this->advancePastAwaitingPayment($order, $causeClass);
}
if (! $wasPlaced) {
$order->update(['placed_at' => $order->placed_at ?? now()]);
Event::dispatch(new OrderPlaced($order));
}
}
/**
* Requires the refund Transaction row to already exist (Modules\Core\
* Order\Listeners\RecordPaymentTransaction must run first — see
* OrderServiceProvider's listener registration order for
* PaymentRefunded), so the relation is refreshed here rather than
* trusted from a possibly-stale $order instance.
*/
public function resolveRefund(Order $order, string $causeClass): void
{
$order->load('transactions');
$target = match (OrderStatus::payment($order)) {
PaymentStatus::Refunded => 'refunded',
PaymentStatus::PartialRefund => 'partially_refunded',
default => null,
};
if ($target !== null && $order->status !== $target) {
$this->writer->write($order, $target, $causeClass);
}
}
private function advancePastAwaitingPayment(Order $order, string $causeClass): void
{
if ($order->status !== 'awaiting_payment') {
return;
}
$next = $this->flow->nextOptions($order);
$target = array_key_first($next);
if ($target !== null) {
$this->writer->write($order, $target, $causeClass);
}
}
}
+16
View File
@@ -133,6 +133,22 @@ class OrderStatusFlow
return ! $order->paid && $this->isCod($order);
}
/**
* Whether moving $order to $to is a valid transition from its CURRENT
* status — the single source of truth for "is this a legal next step,"
* so a caller reacting to an external event (a carrier tracking
* checkpoint, a staff action) doesn't need to hardcode its own "only
* fire from status X" guard duplicating what nextOptions() already
* knows. See e.g. Modules\Core\Order\Listeners\
* AdvanceFulfillmentOnCarrierCheckpoint, which used to compare
* $order->status to a literal 'ready_for_dispatch' inline instead of
* asking this class.
*/
public function isValidTransition(Order $order, string $to): bool
{
return array_key_exists($to, $this->nextOptions($order));
}
private function label(string $status): string
{
return (string) str($status)->replace('_', ' ')->title();