Fix: Bank transfer orders should stay in awaiting payment
This commit is contained in:
@@ -34,6 +34,7 @@ class OrderFulfillmentService
|
|||||||
private readonly OrderStatusWriter $writer,
|
private readonly OrderStatusWriter $writer,
|
||||||
private readonly OrderStatusFlow $flow,
|
private readonly OrderStatusFlow $flow,
|
||||||
private readonly TransactionRecorder $transactions,
|
private readonly TransactionRecorder $transactions,
|
||||||
|
private readonly OrderPaymentResolutionService $resolution,
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
public function markReady(Order $order): OrderFulfillmentResult
|
public function markReady(Order $order): OrderFulfillmentResult
|
||||||
@@ -114,9 +115,13 @@ class OrderFulfillmentService
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Independent of `status` entirely — offered by the single "Update
|
* For a COD order, independent of `status` entirely — offered by the
|
||||||
* Status" action regardless of current status (see
|
* single "Update Status" action regardless of current status (see
|
||||||
* OrderStatusFlow::canMarkPaid()).
|
* OrderStatusFlow::canMarkPaid()). For a bank transfer order, status
|
||||||
|
* genuinely does advance here too (see below) — unlike COD, a bank
|
||||||
|
* transfer order has been sitting at 'awaiting_payment' since checkout
|
||||||
|
* (BankTransferPaymentDriver::pay() deliberately never advances it),
|
||||||
|
* and this click is the only thing that ever will.
|
||||||
*/
|
*/
|
||||||
public function markPaid(Order $order): OrderFulfillmentResult
|
public function markPaid(Order $order): OrderFulfillmentResult
|
||||||
{
|
{
|
||||||
@@ -125,18 +130,20 @@ class OrderFulfillmentService
|
|||||||
}
|
}
|
||||||
|
|
||||||
// canMarkPaid() only ever returns true for an order whose payment
|
// canMarkPaid() only ever returns true for an order whose payment
|
||||||
// method resolves to the cash-on-delivery DRIVER (see
|
// method resolves to the cash-on-delivery or bank-transfer DRIVER
|
||||||
// OrderStatusFlow::isCod(), which checks PaymentMethod::driver,
|
// (see OrderStatusFlow::isCod()/isBankTransfer(), which check
|
||||||
// never the merchant-chosen `type` slug directly — a store could
|
// PaymentMethod::driver, never the merchant-chosen `type` slug
|
||||||
// name that method "cod", "pay-on-delivery", anything). Such an
|
// directly — a store could name that method "cod", "pay-on-delivery",
|
||||||
// order never runs through Payment's pay()/authorize() flow at
|
// "wire", anything). Neither ever runs a Transaction-recording event
|
||||||
// checkout, so nothing else records a Transaction for it. Money
|
// through to completion at checkout (COD dispatches nothing capture-
|
||||||
// changes hands right here, at this click, so this is the one
|
// shaped at all; bank transfer's pay() returns Pending with no event
|
||||||
// place that write can happen; there is no earlier Payment event
|
// dispatched — see that driver's own docblock). Money changes hands
|
||||||
// to hang it off of the way Modules\Core\Order\Listeners\
|
// right here, at this click, so this is the one place that write can
|
||||||
// RecordPaymentTransaction does for a gateway driver. See
|
// happen; there is no earlier Payment event to hang it off of the way
|
||||||
// TransactionRecorder's own docblock — it already anticipated
|
// Modules\Core\Order\Listeners\RecordPaymentTransaction does for a
|
||||||
// exactly this "manually-triggered ... from Filament" call site.
|
// gateway driver. See TransactionRecorder's own docblock — it already
|
||||||
|
// anticipated exactly this "manually-triggered ... from Filament"
|
||||||
|
// call site.
|
||||||
//
|
//
|
||||||
// $driver below is the payment method's own `type` slug (whatever
|
// $driver below is the payment method's own `type` slug (whatever
|
||||||
// the merchant named it, e.g. 'cash-on-delivery' or 'cod') —
|
// the merchant named it, e.g. 'cash-on-delivery' or 'cod') —
|
||||||
@@ -146,19 +153,25 @@ class OrderFulfillmentService
|
|||||||
// No fallback guess here: CheckoutService::initiatePayment() always
|
// No fallback guess here: CheckoutService::initiatePayment() always
|
||||||
// writes Order.meta['payment_method'] before charging, and
|
// writes Order.meta['payment_method'] before charging, and
|
||||||
// canMarkPaid() already guarantees this order got that far.
|
// canMarkPaid() already guarantees this order got that far.
|
||||||
|
$type = (string) $order->meta['payment_method'];
|
||||||
|
|
||||||
$this->transactions->record(
|
$this->transactions->record(
|
||||||
$order,
|
$order,
|
||||||
type: 'capture',
|
type: 'capture',
|
||||||
driver: (string) $order->meta['payment_method'],
|
driver: $type,
|
||||||
result: new PaymentResult(
|
result: new PaymentResult(
|
||||||
status: PaymentResultStatus::Succeeded,
|
status: PaymentResultStatus::Succeeded,
|
||||||
reference: 'cod-manual-'.$order->id,
|
reference: "manual-{$type}-{$order->id}",
|
||||||
amount: $order->total,
|
amount: $order->total,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
|
||||||
$this->writer->markPaid($order, self::class.'::markPaid');
|
$this->writer->markPaid($order, self::class.'::markPaid');
|
||||||
|
|
||||||
|
if ($this->flow->isBankTransfer($order)) {
|
||||||
|
$this->resolution->advancePastAwaitingPayment($order, self::class.'::markPaid');
|
||||||
|
}
|
||||||
|
|
||||||
return OrderFulfillmentResult::success('Order marked as paid.');
|
return OrderFulfillmentResult::success('Order marked as paid.');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -92,7 +92,14 @@ class OrderPaymentResolutionService
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function advancePastAwaitingPayment(Order $order, string $causeClass): void
|
/**
|
||||||
|
* Also called directly by OrderFulfillmentService::markPaid() for a
|
||||||
|
* bank transfer order — unlike a COD markPaid() (which never touches
|
||||||
|
* status, since nothing was ever awaited), a bank transfer order
|
||||||
|
* genuinely sat at 'awaiting_payment' until this moment, and nothing
|
||||||
|
* else will ever advance it if this doesn't.
|
||||||
|
*/
|
||||||
|
public function advancePastAwaitingPayment(Order $order, string $causeClass): void
|
||||||
{
|
{
|
||||||
if ($order->status !== 'awaiting_payment') {
|
if ($order->status !== 'awaiting_payment') {
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -54,6 +54,24 @@ class OrderStatusFlow
|
|||||||
return PaymentMethod::where('type', $type)->value('driver') === 'cash-on-delivery';
|
return PaymentMethod::where('type', $type)->value('driver') === 'cash-on-delivery';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Same meta-first/Transaction-fallback resolution as isCod(). Unlike COD
|
||||||
|
* — where nothing is ever awaited, since payment happens on delivery —
|
||||||
|
* a bank transfer order genuinely sits at 'awaiting_payment' until staff
|
||||||
|
* confirm the wire arrived (see BankTransferPaymentDriver's own
|
||||||
|
* docblock and OrderFulfillmentService::markPaid()).
|
||||||
|
*/
|
||||||
|
public function isBankTransfer(Order $order): bool
|
||||||
|
{
|
||||||
|
$type = $order->meta['payment_method'] ?? $order->transactions()->latest('id')->value('driver');
|
||||||
|
|
||||||
|
if ($type === null) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return PaymentMethod::where('type', $type)->value('driver') === 'bank-transfer';
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return array<string, string> value => label — every status in the
|
* @return array<string, string> value => label — every status in the
|
||||||
* order's own branch (carrier or pickup), plus the refund options,
|
* order's own branch (carrier or pickup), plus the refund options,
|
||||||
@@ -124,13 +142,16 @@ class OrderStatusFlow
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Whether the "mark paid" option should be offered right now —
|
* Whether the "mark paid" option should be offered right now —
|
||||||
* entirely independent of $order->status. True whenever this is a
|
* entirely independent of $order->status for a COD order (true whenever
|
||||||
* cash-on-delivery order and payment hasn't been recorded yet,
|
* payment hasn't been recorded yet, regardless of fulfillment progress,
|
||||||
* regardless of fulfillment progress (before OR after completed).
|
* before OR after completed). A bank transfer order is also eligible,
|
||||||
|
* for the same "no earlier Payment event recorded this" reason (see
|
||||||
|
* OrderFulfillmentService::markPaid()), but unlike COD its own status
|
||||||
|
* genuinely does need advancing once marked paid — see that method.
|
||||||
*/
|
*/
|
||||||
public function canMarkPaid(Order $order): bool
|
public function canMarkPaid(Order $order): bool
|
||||||
{
|
{
|
||||||
return ! $order->paid && $this->isCod($order);
|
return ! $order->paid && ($this->isCod($order) || $this->isBankTransfer($order));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -9,30 +9,47 @@ use Modules\Core\Payment\Contracts\SupportsPay;
|
|||||||
use Modules\Core\Payment\Contracts\SupportsRefunds;
|
use Modules\Core\Payment\Contracts\SupportsRefunds;
|
||||||
use Modules\Core\Payment\DTOs\PaymentResult;
|
use Modules\Core\Payment\DTOs\PaymentResult;
|
||||||
use Modules\Core\Payment\Enums\PaymentResultStatus;
|
use Modules\Core\Payment\Enums\PaymentResultStatus;
|
||||||
use Modules\Core\Payment\Events\PaymentCaptured;
|
|
||||||
use Modules\Core\Payment\Events\PaymentRefunded;
|
use Modules\Core\Payment\Events\PaymentRefunded;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Manual/attested, same trust model as OfflinePaymentDriver — there is no
|
* refund() is manual/attested, same trust model as OfflinePaymentDriver —
|
||||||
* bank API to call, so both pay() and refund() decide success immediately
|
* there is no bank API to call, so it decides success immediately on a
|
||||||
* on a staff member's say-so (they've already sent/received the wire
|
* staff member's say-so (they've already sent the wire outside the
|
||||||
* outside the system). Distinct from OfflinePaymentDriver in intent: this
|
* system). Distinct from OfflinePaymentDriver in intent: this exists so a
|
||||||
* exists so a payment taken through a DIFFERENT method (e.g.
|
* payment taken through a DIFFERENT method (e.g. cash-on-delivery) can
|
||||||
* cash-on-delivery) can still be REFUNDED via bank transfer — an admin
|
* still be REFUNDED via bank transfer — an admin chooses this driver
|
||||||
* chooses this driver explicitly in the refund action, independent of
|
* explicitly in the refund action, independent of which driver the
|
||||||
* which driver the original payment went through (see
|
* original payment went through (see
|
||||||
* Payment\Support\TransactionDriverAdapter::refundVia() and
|
* Payment\Support\TransactionDriverAdapter::refundVia() and
|
||||||
* Order\Filament\Extensions\OrderActionsExtension). pay() exists so
|
* Order\Filament\Extensions\OrderActionsExtension).
|
||||||
* the same driver also covers receiving a payment by bank transfer, but
|
*
|
||||||
* the admin UI for that (bank reference, notes, proof-of-transfer upload)
|
* pay() is the opposite trust direction from refund(): a bank transfer
|
||||||
* is deliberately not built yet — see the follow-up work tracked from this
|
* payment requires the money to arrive BEFORE the order can be
|
||||||
* session; pay() itself is complete and usable via the registry today.
|
* considered paid (unlike cash-on-delivery, where payment happens on
|
||||||
|
* delivery — see CashOnDeliveryPaymentDriver's own docblock for that
|
||||||
|
* driver's mirror-image reasoning). So pay() returns Pending, dispatching
|
||||||
|
* no event at all — no PaymentCaptured (nothing has been paid yet), and
|
||||||
|
* deliberately NOT PaymentDeferred either (unlike COD, whose
|
||||||
|
* MarkOrderPlacedOnDeferredPayment listener immediately advances the
|
||||||
|
* order past 'awaiting_payment' since a COD order has nothing to await at
|
||||||
|
* checkout). A bank transfer order genuinely DOES have something to
|
||||||
|
* await: it stays at 'awaiting_payment' with Order::paid false until
|
||||||
|
* staff confirm the wire arrived via OrderFulfillmentService::markPaid(),
|
||||||
|
* which — unlike its COD path — also advances the order's status, since
|
||||||
|
* nothing else ever will (see that method's own docblock).
|
||||||
|
* CheckoutController::placeOrder() already treats a Pending result with
|
||||||
|
* no continuation as a fully placed order (see its own docblock), so the
|
||||||
|
* order is still created and visible to the shopper immediately; only its
|
||||||
|
* payment/status is what's left outstanding.
|
||||||
*
|
*
|
||||||
* $reference is generated here for the same reason as OfflinePaymentDriver's
|
* $reference is generated here for the same reason as OfflinePaymentDriver's
|
||||||
* pay(): there is no gateway to hand one back. 'notes' in $context (not
|
* pay(): there is no gateway to hand one back. refund()'s 'notes' (in
|
||||||
* $data — refund() has no $data parameter) is folded into
|
* $context — it has no $data parameter) is folded into PaymentResult::$meta,
|
||||||
* PaymentResult::$meta, which Order\Services\TransactionRecorder::record()
|
* which Order\Services\TransactionRecorder::record() already writes straight
|
||||||
* already writes straight into Transaction.meta with no extra plumbing.
|
* into Transaction.meta with no extra plumbing; pay() has no equivalent
|
||||||
|
* write, since nothing ever records a Transaction from its own result (see
|
||||||
|
* above) — any notes a shopper enters at checkout would need surfacing some
|
||||||
|
* other way, e.g. when staff mark the order paid.
|
||||||
*/
|
*/
|
||||||
class BankTransferPaymentDriver implements Configurable, SupportsPay, SupportsRefunds
|
class BankTransferPaymentDriver implements Configurable, SupportsPay, SupportsRefunds
|
||||||
{
|
{
|
||||||
@@ -46,16 +63,11 @@ class BankTransferPaymentDriver implements Configurable, SupportsPay, SupportsRe
|
|||||||
|
|
||||||
public function pay(string $type, Price $amount, array $data = [], array $context = []): PaymentResult
|
public function pay(string $type, Price $amount, array $data = [], array $context = []): PaymentResult
|
||||||
{
|
{
|
||||||
$result = new PaymentResult(
|
return new PaymentResult(
|
||||||
status: PaymentResultStatus::Succeeded,
|
status: PaymentResultStatus::Pending,
|
||||||
reference: 'bank-transfer-'.Str::uuid(),
|
reference: 'bank-transfer-'.Str::uuid(),
|
||||||
amount: $amount,
|
amount: $amount,
|
||||||
meta: array_filter(['notes' => $data['notes'] ?? null]),
|
|
||||||
);
|
);
|
||||||
|
|
||||||
PaymentCaptured::dispatch($type, $result, $context);
|
|
||||||
|
|
||||||
return $result;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function refund(string $reference, Price $amount, array $context = []): PaymentResult
|
public function refund(string $reference, Price $amount, array $context = []): PaymentResult
|
||||||
|
|||||||
Reference in New Issue
Block a user