From 12aaa43f1032f6e5a66e166fd1dba369655fe68d Mon Sep 17 00:00:00 2001 From: Konstantinos Arvanitakis Date: Fri, 18 Sep 2026 00:54:43 +0300 Subject: [PATCH] Fix: Updates to OrderFullfilmentServices and box now clients, order views and checkout services --- .../Exceptions/NoShippingAddressException.php | 18 ++++ src/Checkout/Services/CheckoutService.php | 85 ++++++++++++++++++- .../MarkOrderPlacedOnDeferredPayment.php | 66 ++++++++++++++ .../Services/OrderFulfillmentService.php | 36 ++++++++ .../OrderPaymentResolutionService.php | 18 ++++ .../Drivers/CashOnDeliveryPaymentDriver.php | 18 +++- src/Payment/Events/PaymentDeferred.php | 46 ++++++++++ .../Resources/PaymentMethodResource.php | 46 +++++++++- .../Pages/ListPaymentMethods.php | 10 +++ src/Providers/OrderServiceProvider.php | 4 + src/Shipping/Carriers/BoxNow/BoxNowClient.php | 14 +++ .../BoxNow/BoxNowFulfillmentService.php | 33 ++++++- .../Extensions/OrderShipmentsExtension.php | 26 ++++++ .../Extensions/OrderViewExtension.php | 35 +++++--- src/Shipping/Jobs/PollShipmentTrackingJob.php | 13 ++- 15 files changed, 450 insertions(+), 18 deletions(-) create mode 100644 src/Checkout/Exceptions/NoShippingAddressException.php create mode 100644 src/Order/Listeners/MarkOrderPlacedOnDeferredPayment.php create mode 100644 src/Payment/Events/PaymentDeferred.php diff --git a/src/Checkout/Exceptions/NoShippingAddressException.php b/src/Checkout/Exceptions/NoShippingAddressException.php new file mode 100644 index 0000000..4a90b0b --- /dev/null +++ b/src/Checkout/Exceptions/NoShippingAddressException.php @@ -0,0 +1,18 @@ +cart->currentOrCreate()->setShippingAddress($address); + $cartBefore = $this->cart->currentOrCreate(); + $boxNowLocker = $cartBefore->shippingAddress?->meta['box_now_locker'] ?? null; + + $cart = $cartBefore->setShippingAddress($address); + + if ($boxNowLocker !== null) { + $newAddress = $cart->shippingAddress; + $newAddress->meta = [...($newAddress->meta?->toArray() ?? []), 'box_now_locker' => $boxNowLocker]; + $newAddress->save(); + } Event::dispatch(new ShippingAddressSet($cart, $address)); @@ -141,11 +168,67 @@ class CheckoutService $cart = $cartBefore->setShippingOption($option); + // Switching away from Box Now leaves a stale box_now_locker on the + // address's meta (see setShippingAddress()'s own docblock for why + // it survives address-row recreation) — irrelevant while a + // different method is selected, but wrong if the shopper later + // switches BACK to Box Now and it resurfaces as if still chosen, + // possibly for a locker that no longer exists/fits. Cleared here, + // the one place that knows the method just changed. + if ($identifier !== 'box-now') { + $address = $cart->shippingAddress; + + if ($address && isset($address->meta['box_now_locker'])) { + $meta = $address->meta->toArray(); + unset($meta['box_now_locker']); + $address->meta = $meta; + $address->save(); + } + } + Event::dispatch(new ShippingOptionSelected($cart, $option)); return $cart; } + /** + * Records the shopper's chosen Box Now locker on the cart's shipping + * address (Cart\Addresses::shippingAddress()->meta['box_now_locker']), + * not on the cart itself — Lunar\Pipelines\Order\Creation\ + * CreateOrderAddresses copies every cart address's full attributes + * (meta included) onto the new order address when the order is placed, + * so this is what Modules\Core\Shipping\Carriers\BoxNow\ + * BoxNowFulfillmentService and Modules\Core\Shipping\Extensions\ + * OrderViewExtension already expect to find at + * $order->shippingAddress->meta['box_now_locker']['locationId']. + * + * No validation against Box Now's own /destinations list here — this + * mirrors setShippingAddress()'s leniency (see its own docblock/the + * class-level note on required-field enforcement happening at the + * payment gate, not mid-checkout). An invalid/stale locationId still + * surfaces later, at BoxNowFulfillmentService::createShipment() time. + * + * @throws NoShippingAddressException if the cart has no shipping + * address yet + */ + public function selectBoxNowLocker(array $locker): Cart + { + $cart = $this->cart->currentOrCreate(); + $address = $cart->shippingAddress; + + if (! $address) { + throw new NoShippingAddressException(); + } + + $address->meta = [ + ...($address->meta?->toArray() ?? []), + 'box_now_locker' => $locker, + ]; + $address->save(); + + return $cart; + } + /** * Every payment method currently offered to the storefront, ordered by * Modules\Core\Payment\Models\PaymentMethod::position — a row is diff --git a/src/Order/Listeners/MarkOrderPlacedOnDeferredPayment.php b/src/Order/Listeners/MarkOrderPlacedOnDeferredPayment.php new file mode 100644 index 0000000..90ef307 --- /dev/null +++ b/src/Order/Listeners/MarkOrderPlacedOnDeferredPayment.php @@ -0,0 +1,66 @@ + DecrementStockOnOrderPlaced), + * so queueing this one would just move the same stock-oversell risk one + * hop earlier. + * + * A thin reactor, same shape as ApplyResolvedPaymentStatus — the actual + * decisions ("this order counts as placed the moment a deferred-payment + * driver resolves, independent of Order::paid" and "such an order also + * has nothing to sit at awaiting_payment for") live in PaymentDeferred's + * and OrderPaymentResolutionService::resolveDeferredPayment()'s own + * docblocks, re-confirmed with the user; this only extracts the order id + * and applies both, guarded against a duplicate/replayed event the same + * way OrderPaymentResolutionService::resolveCaptureOrAuthorization() is. + * + * Without the status advance below, a COD order was left sitting at + * 'awaiting_payment' forever — placed_at/OrderPlaced alone fixed order + * visibility and stock decrement, but nothing ever moved `status` off its + * initial value, since resolveCaptureOrAuthorization() only does that for + * an actual capture. Caught and fixed after the fact. + */ +class MarkOrderPlacedOnDeferredPayment +{ + public function __construct( + private readonly OrderPaymentResolutionService $resolution, + ) {} + + public function handle(PaymentDeferred $event): void + { + $orderId = $event->context['order_id'] ?? null; + + if ($orderId === null) { + return; + } + + $order = Order::findOrFail($orderId); + + $this->resolution->resolveDeferredPayment($order, self::class); + + if (! blank($order->placed_at)) { + return; + } + + $order->update(['placed_at' => now()]); + + Event::dispatch(new OrderPlaced($order)); + } +} diff --git a/src/Order/Services/OrderFulfillmentService.php b/src/Order/Services/OrderFulfillmentService.php index 1874883..70eec8d 100644 --- a/src/Order/Services/OrderFulfillmentService.php +++ b/src/Order/Services/OrderFulfillmentService.php @@ -8,6 +8,8 @@ use Modules\Core\Order\DTOs\OrderFulfillmentResult; use Modules\Core\Order\Events\OrderPickedUp; use Modules\Core\Order\Events\OrderReadyForDispatch; use Modules\Core\Order\Events\OrderReadyForPickup; +use Modules\Core\Payment\DTOs\PaymentResult; +use Modules\Core\Payment\Enums\PaymentResultStatus; use Modules\Core\Shipping\Contracts\CarrierFulfillmentInterface; use Modules\Core\Shipping\DTOs\ShipmentRequest; use Throwable; @@ -31,6 +33,7 @@ class OrderFulfillmentService public function __construct( private readonly OrderStatusWriter $writer, private readonly OrderStatusFlow $flow, + private readonly TransactionRecorder $transactions, ) {} public function markReady(Order $order): OrderFulfillmentResult @@ -121,6 +124,39 @@ class OrderFulfillmentService return OrderFulfillmentResult::failure('This order cannot be marked paid right now.'); } + // canMarkPaid() only ever returns true for an order whose payment + // method resolves to the cash-on-delivery DRIVER (see + // OrderStatusFlow::isCod(), which checks PaymentMethod::driver, + // never the merchant-chosen `type` slug directly — a store could + // name that method "cod", "pay-on-delivery", anything). Such an + // order never runs through Payment's pay()/authorize() flow at + // checkout, so nothing else records a Transaction for it. Money + // changes hands right here, at this click, so this is the one + // place that write can happen; there is no earlier Payment event + // to hang it off of the way Modules\Core\Order\Listeners\ + // RecordPaymentTransaction does for a 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 + // the merchant named it, e.g. 'cash-on-delivery' or 'cod') — + // Transaction.driver's established meaning everywhere else in this + // codebase (see RecordPaymentTransaction/TransactionRecorder's own + // docblocks) is that type key, never the underlying driver CLASS. + // No fallback guess here: CheckoutService::initiatePayment() always + // writes Order.meta['payment_method'] before charging, and + // canMarkPaid() already guarantees this order got that far. + $this->transactions->record( + $order, + type: 'capture', + driver: (string) $order->meta['payment_method'], + result: new PaymentResult( + status: PaymentResultStatus::Succeeded, + reference: 'cod-manual-'.$order->id, + amount: $order->total, + ), + ); + $this->writer->markPaid($order, self::class.'::markPaid'); return OrderFulfillmentResult::success('Order marked as paid.'); diff --git a/src/Order/Services/OrderPaymentResolutionService.php b/src/Order/Services/OrderPaymentResolutionService.php index 5efdb27..d990382 100644 --- a/src/Order/Services/OrderPaymentResolutionService.php +++ b/src/Order/Services/OrderPaymentResolutionService.php @@ -52,6 +52,24 @@ class OrderPaymentResolutionService } } + /** + * A deferred-capture payment (currently only cash-on-delivery — see + * Payment\Events\PaymentDeferred's own docblock): no money has moved, + * so unlike resolveCaptureOrAuthorization() this never calls + * $writer->markPaid() — Order::paid stays false until staff explicitly + * mark it received. But per OrderStatusFlow's own docblock, payment + * method never affects the status SEQUENCE at all — a COD order has + * nothing to "await" at checkout (no payment attempt happens), so + * 'awaiting_payment' is simply the wrong first status for it. Reuses + * the exact same advancePastAwaitingPayment() a capture uses, since + * the status-sequence logic itself doesn't differ by payment method, + * only whether `paid` also flips alongside it. + */ + public function resolveDeferredPayment(Order $order, string $causeClass): void + { + $this->advancePastAwaitingPayment($order, $causeClass); + } + /** * Requires the refund Transaction row to already exist (Modules\Core\ * Order\Listeners\RecordPaymentTransaction must run first — see diff --git a/src/Payment/Drivers/CashOnDeliveryPaymentDriver.php b/src/Payment/Drivers/CashOnDeliveryPaymentDriver.php index 866167b..ea568bf 100644 --- a/src/Payment/Drivers/CashOnDeliveryPaymentDriver.php +++ b/src/Payment/Drivers/CashOnDeliveryPaymentDriver.php @@ -8,6 +8,7 @@ use Modules\Core\Payment\Contracts\Configurable; use Modules\Core\Payment\Contracts\SupportsPay; use Modules\Core\Payment\DTOs\PaymentResult; use Modules\Core\Payment\Enums\PaymentResultStatus; +use Modules\Core\Payment\Events\PaymentDeferred; /** * Cash-on-delivery/cash-on-pickup — the shopper pays staff in person, at @@ -31,6 +32,17 @@ use Modules\Core\Payment\Enums\PaymentResultStatus; * marking it received (Modules\Core\Order\Services\ * OrderFulfillmentService::markPaid()), offered by the single "Update * Status" action at any time, independent of status. + * + * Despite returning Pending, this order IS fully placed the moment pay() + * returns — unlike a Stripe 3-D Secure Pending, nothing will ever resolve + * this into a later PaymentCaptured/PaymentAuthorized (COD has no gateway + * callback at all). Without PaymentDeferred, no listener ever set + * Order::placed_at for a COD order: invisible in customer order history, + * no stock decrement (Modules\Core\Order\Listeners\ + * DecrementStockOnOrderPlaced only reacts to Checkout\Events\OrderPlaced), + * and the storefront's own post-checkout confirmation could never find it + * — a real bug, not a hypothetical, caught and fixed after the fact. See + * PaymentDeferred's own docblock for the full reasoning. */ class CashOnDeliveryPaymentDriver implements Configurable, SupportsPay { @@ -41,10 +53,14 @@ class CashOnDeliveryPaymentDriver implements Configurable, SupportsPay public function pay(string $type, Price $amount, array $data = [], array $context = []): PaymentResult { - return new PaymentResult( + $result = new PaymentResult( status: PaymentResultStatus::Pending, reference: 'cod-'.Str::uuid(), amount: $amount, ); + + PaymentDeferred::dispatch($type, $result, $context); + + return $result; } } diff --git a/src/Payment/Events/PaymentDeferred.php b/src/Payment/Events/PaymentDeferred.php new file mode 100644 index 0000000..b9df3f2 --- /dev/null +++ b/src/Payment/Events/PaymentDeferred.php @@ -0,0 +1,46 @@ + $context + */ + public function __construct( + public readonly string $type, + public readonly PaymentResult $result, + public readonly array $context = [], + ) {} +} diff --git a/src/Payment/Filament/Resources/PaymentMethodResource.php b/src/Payment/Filament/Resources/PaymentMethodResource.php index 2cfbc2c..2ea9eed 100644 --- a/src/Payment/Filament/Resources/PaymentMethodResource.php +++ b/src/Payment/Filament/Resources/PaymentMethodResource.php @@ -3,10 +3,13 @@ namespace Modules\Core\Payment\Filament\Resources; use Filament\Actions\Action; +use Filament\Forms\Components\Hidden; use Filament\Forms\Components\Select; use Filament\Forms\Components\TextInput; use Filament\Resources\Resource; use Filament\Schemas\Components\Component; +use Filament\Schemas\Components\Utilities\Get; +use InvalidArgumentException; use Filament\Tables\Columns\IconColumn; use Filament\Tables\Columns\TextColumn; use Filament\Tables\Columns\ToggleColumn; @@ -14,6 +17,7 @@ use Filament\Tables\Table; use Illuminate\Support\Facades\Event; use Lunar\Admin\Support\Forms\Components\TranslatedText; use Modules\Core\Payment\Contracts\Configurable; +use Modules\Core\Payment\Contracts\SupportsAuthorization; use Modules\Core\Payment\Events\PaymentMethodsReordered; use Modules\Core\Payment\Filament\Resources\PaymentMethodResource\Pages\ListPaymentMethods; use Modules\Core\Payment\Models\PaymentMethod; @@ -144,7 +148,31 @@ class PaymentMethodResource extends Resource ]) ->default('pay') ->live() - ->required(), + // Only meaningful for a driver that actually implements + // SupportsAuthorization — CheckoutService::initiatePayment() + // calls $driver->authorize() when capture_mode is + // "authorize", which fatals on a driver missing that method + // entirely (e.g. CashOnDeliveryPaymentDriver, which only + // ever implements SupportsPay: the shopper pays staff in + // person, at an unknown future moment — there is no + // "hold now, settle later" operation to offer for that at + // all). Hidden rather than merely disabled, since a + // hidden field is also excluded from validation/dehydration + // — required() below would otherwise still block saving. + ->visible(fn (Get $get) => static::driverSupportsAuthorization($get('driver'))) + ->required(fn (Get $get) => static::driverSupportsAuthorization($get('driver'))), + // Every OTHER fillForm() value not backed by a real component + // here is silently dropped — an Action::schema() modal only + // dehydrates fields present in its own schema, unlike a + // resource's form(); ListPaymentMethods::getHeaderActions()'s + // CreateAction::fillForm() used to set 'position' this same + // way and it never reached PaymentMethodService::create(), + // so every new method saved with the column's raw DB default + // (0) regardless of what fillForm() computed. Hidden here + // purely so it actually dehydrates; the table's own + // reorderable('position') drag-and-drop remains the real + // staff-facing way to change it afterward. + Hidden::make('position'), ]; } @@ -153,9 +181,23 @@ class PaymentMethodResource extends Resource return Select::make('driver') ->label('Driver') ->options(fn () => app(PaymentDriverRegistry::class)->labels()) + ->live() ->required(); } + private static function driverSupportsAuthorization(?string $driver): bool + { + if (! $driver) { + return true; + } + + try { + return app(PaymentDriverRegistry::class)->resolve($driver) instanceof SupportsAuthorization; + } catch (InvalidArgumentException) { + return true; + } + } + public static function getPages(): array { return [ @@ -180,7 +222,7 @@ class PaymentMethodResource extends Resource ->icon('heroicon-o-pencil-square') ->schema(static::getFormComponents()) ->fillForm(fn (PaymentMethod $record) => $record->only([ - 'name', 'type', 'driver', 'capture_mode', + 'name', 'type', 'driver', 'capture_mode', 'position', ])) ->action(fn (PaymentMethod $record, array $data) => app(PaymentMethodService::class)->update($record, $data)); } diff --git a/src/Payment/Filament/Resources/PaymentMethodResource/Pages/ListPaymentMethods.php b/src/Payment/Filament/Resources/PaymentMethodResource/Pages/ListPaymentMethods.php index 9d170b1..663d654 100644 --- a/src/Payment/Filament/Resources/PaymentMethodResource/Pages/ListPaymentMethods.php +++ b/src/Payment/Filament/Resources/PaymentMethodResource/Pages/ListPaymentMethods.php @@ -5,6 +5,7 @@ namespace Modules\Core\Payment\Filament\Resources\PaymentMethodResource\Pages; use Filament\Actions\CreateAction; use Filament\Actions; use Filament\Resources\Pages\ListRecords; +use Lunar\Models\Language; use Modules\Core\Payment\Filament\Resources\PaymentMethodResource; use Modules\Core\Payment\Models\PaymentMethod; use Modules\Core\Payment\Services\PaymentMethodService; @@ -22,6 +23,15 @@ class ListPaymentMethods extends ListRecords 'position' => (PaymentMethod::max('position') ?? 0) + 1, 'enabled' => false, 'data' => [], + // TranslatedText's own default() (getLanguageDefaults()) + // never reaches this mounted action's initial state — + // unlike a resource's own form(), an Action::schema() + // modal starts from exactly what fillForm() returns, so + // `name` was landing as null rather than the expected + // per-locale array, and every locale's sub-input + // silently failed to bind to it (required() on the + // default locale then correctly rejected the null). + 'name' => Language::pluck('code')->mapWithKeys(fn (string $code) => [$code => ''])->all(), ]) // Every PaymentMethod write goes through PaymentMethodService // — see PaymentMethodResource's own docblock — so this diff --git a/src/Providers/OrderServiceProvider.php b/src/Providers/OrderServiceProvider.php index b5b57e5..4e1cdc3 100644 --- a/src/Providers/OrderServiceProvider.php +++ b/src/Providers/OrderServiceProvider.php @@ -21,6 +21,7 @@ use Modules\Core\Order\Listeners\CompleteOrderOnPickedUp; use Modules\Core\Order\Listeners\DecrementStockOnOrderPlaced; use Modules\Core\Order\Listeners\DeriveOrderDeliveredFromShipment; use Modules\Core\Order\Listeners\MarkDeliveryFailedOnCarrierCheckpoint; +use Modules\Core\Order\Listeners\MarkOrderPlacedOnDeferredPayment; use Modules\Core\Order\Listeners\RecordPaymentTransaction; use Modules\Core\Order\Listeners\RecordStatusTransition; use Modules\Core\Order\Models\OrderStatusTransition; @@ -37,6 +38,7 @@ use Modules\Core\Order\Observers\TransactionObserver; use Modules\Core\Order\Support\OrderStatus; use Modules\Core\Payment\Events\PaymentAuthorized; use Modules\Core\Payment\Events\PaymentCaptured; +use Modules\Core\Payment\Events\PaymentDeferred; use Modules\Core\Payment\Events\PaymentRefunded; use Modules\Core\Payment\Events\PaymentVoided; use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier; @@ -74,6 +76,8 @@ class OrderServiceProvider extends ServiceProvider Event::listen(PaymentRefunded::class, RecordPaymentTransaction::class); Event::listen(PaymentRefunded::class, ApplyResolvedPaymentStatus::class); + Event::listen(PaymentDeferred::class, MarkOrderPlacedOnDeferredPayment::class); + Event::listen(OrderPlaced::class, DecrementStockOnOrderPlaced::class); Event::listen(OrderStatusChanged::class, [RecordStatusTransition::class, 'handleStatusChanged']); diff --git a/src/Shipping/Carriers/BoxNow/BoxNowClient.php b/src/Shipping/Carriers/BoxNow/BoxNowClient.php index dd8bbea..2507249 100644 --- a/src/Shipping/Carriers/BoxNow/BoxNowClient.php +++ b/src/Shipping/Carriers/BoxNow/BoxNowClient.php @@ -61,6 +61,20 @@ class BoxNowClient return $response->json() ?? []; } + /** + * List available APM (locker) destinations — the data behind Box Now's + * own Destination Map widget, which only works against their + * Production environment (not Stage/sandbox). Used to build a plain + * locker picker on the storefront checkout instead, working against + * whichever environment is configured. + * + * @return array> + */ + public function destinations(array $query = []): array + { + return $this->locationRequest('/destinations', $query)['data'] ?? []; + } + /** * Fetch raw bytes (e.g. a PDF label) rather than JSON. */ diff --git a/src/Shipping/Carriers/BoxNow/BoxNowFulfillmentService.php b/src/Shipping/Carriers/BoxNow/BoxNowFulfillmentService.php index 971df90..7dbae9a 100644 --- a/src/Shipping/Carriers/BoxNow/BoxNowFulfillmentService.php +++ b/src/Shipping/Carriers/BoxNow/BoxNowFulfillmentService.php @@ -67,7 +67,7 @@ class BoxNowFulfillmentService implements CarrierFulfillmentInterface, SupportsT 'locationId' => config('boxnow.origin_location_id'), ], 'destination' => [ - 'contactNumber' => $address->contact_phone, + 'contactNumber' => $this->internationalPhone($address->contact_phone), 'contactEmail' => $address->contact_email, 'contactName' => trim("{$address->first_name} {$address->last_name}"), 'locationId' => $destinationLocationId, @@ -105,6 +105,37 @@ class BoxNowFulfillmentService implements CarrierFulfillmentInterface, SupportsT return $shipments->first(); } + /** + * Box Now rejects any contactNumber not in full international format + * (error P405 — confirmed in practice: a plain Greek mobile like + * "6955994563" 400s with {"code":"P405"}). Checkout collects phone + * numbers in local format, with no international-format enforcement of + * its own — this store is Greece-only (see CheckoutController:: + * STORE_COUNTRY_ISO3), so a bare local number is assumed Greek and + * prefixed accordingly, same convention ACS's own sender config already + * uses (config('boxnow.sender.phone') is documented as "+30..." there + * too). A number already carrying a country code (leading "+" or "00") + * is passed through unchanged. + */ + private function internationalPhone(?string $phone): ?string + { + if ($phone === null) { + return null; + } + + $digitsOnly = preg_replace('/[^\d+]/', '', $phone); + + if (str_starts_with($digitsOnly, '+')) { + return $digitsOnly; + } + + if (str_starts_with($digitsOnly, '00')) { + return '+'.substr($digitsOnly, 2); + } + + return '+30'.ltrim($digitsOnly, '0'); + } + public function printLabel(Shipment $shipment): string { $bytes = $this->client->requestRaw("/parcels/{$shipment->tracking_reference}/label.pdf"); diff --git a/src/Shipping/Extensions/OrderShipmentsExtension.php b/src/Shipping/Extensions/OrderShipmentsExtension.php index 0298812..a321642 100644 --- a/src/Shipping/Extensions/OrderShipmentsExtension.php +++ b/src/Shipping/Extensions/OrderShipmentsExtension.php @@ -5,6 +5,8 @@ namespace Modules\Core\Shipping\Extensions; use Filament\Actions\Action; use Filament\Infolists\Components\RepeatableEntry; use Filament\Infolists\Components\TextEntry; +use Illuminate\Support\Collection; +use Modules\Core\Shipping\Models\ShipmentInfo; use Filament\Notifications\Notification; use Filament\Schemas\Components\Section; use Illuminate\Support\Facades\URL; @@ -112,10 +114,34 @@ class OrderShipmentsExtension extends ViewPageExtension ->action(fn (Shipment $record) => $this->cancel($record)) ->visible(fn (Shipment $record) => ! $record->cancelled_at), ]), + RepeatableEntry::make('shipmentInfo') + ->label('Tracking history') + ->state(fn (Shipment $record) => $this->orderedCheckpoints($record)) + ->hidden(fn (Shipment $record) => $record->shipmentInfo->isEmpty()) + ->schema([ + TextEntry::make('status') + ->label(fn (ShipmentInfo $record) => $record->occurred_at->format('Y-m-d H:i')) + ->inlineLabel() + ->state(fn (ShipmentInfo $record) => (string) str($record->status->value)->replace('_', ' ')->title()) + ->helperText(fn (ShipmentInfo $record) => $record->location), + ]), ]), ]); } + /** + * Oldest first — a delivery journey (Collected → In Transit → + * Delivered) reads naturally top-to-bottom in that order, unlike + * statusLabel()/statusColor() above which only ever need the single + * latest checkpoint and so use latestShipmentInfo() directly instead. + * + * @return Collection + */ + private function orderedCheckpoints(Shipment $record): Collection + { + return $record->shipmentInfo->sortBy('occurred_at')->values(); + } + private function carrierLabel(Shipment $record): string { return match ($record->carrier) { diff --git a/src/Shipping/Extensions/OrderViewExtension.php b/src/Shipping/Extensions/OrderViewExtension.php index f792860..0685b26 100644 --- a/src/Shipping/Extensions/OrderViewExtension.php +++ b/src/Shipping/Extensions/OrderViewExtension.php @@ -56,10 +56,11 @@ use Modules\Core\Shipping\Support\WeightCalculator; * each with its own S/M/L size) instead — see * Modules\Core\Shipping\Carriers\BoxNow\BoxNowFulfillmentService for how * multiple boxes become multiple Shipment rows from one delivery request. - * Box Now's locker is locked to read-only once the shopper's own checkout - * selection ($order->shippingAddress->meta['box_now_locker']) is present - * — staff can only fill it in manually for the (current, checkout-UI-less) - * case where nothing set it yet. + * Box Now's locker field defaults from the shopper's own checkout + * selection ($order->shippingAddress->meta['box_now_locker']) when present, + * but stays editable — staff can override to a different locker (e.g. the + * customer's choice turns out to be unavailable) or fill it in manually for + * an order placed before the checkout locker picker existed. * * "Mark Paid" is a third, separate header action — Order::paid is * independent of `status` (see OrderStatusFlow's own docblock), so it @@ -124,16 +125,9 @@ class OrderViewExtension extends ViewPageExtension TextInput::make('destination_location_id') ->label('Box Now locker ID') ->default($lockerId) - // Locked once the shopper's own checkout selection is - // known — staff should not be able to redirect a - // parcel to a different locker than the one the - // customer picked. Only editable for the (current, - // checkout-UI-less) case where nothing set it yet. - ->disabled(filled($lockerId)) - ->dehydrated() ->required() ->helperText($lockerId - ? 'Set by the customer at checkout.' + ? 'Set by the customer at checkout — override if the parcel needs to go to a different locker.' : 'No locker was selected at checkout — enter it manually.'), Repeater::make('boxes') ->label('Boxes') @@ -152,12 +146,29 @@ class OrderViewExtension extends ViewPageExtension ]; }) ->action(function (Order $record, array $data, Action $action) { + // Derived from the order itself, never from staff input — + // whether a shipment collects cash on delivery is a fact + // about the order (which PaymentMethod it was placed + // against), not a choice to make again at dispatch time. + // Previously this was never set at all (defaulted to + // ShipmentRequest::$paymentMode's own null), which silently + // made AcsFulfillmentService::createShipment()'s + // `$request->paymentMode === 'cod'` branch (sends + // Cod_Ammount/Cod_Payment_Way to ACS) permanently + // unreachable, and BoxNowFulfillmentService::createShipment() + // always ship 'prepaid' with amountToBeCollected '0.00' — + // a real COD order would arrive with the carrier believing + // full payment was already settled, and never collect it. + $isCod = app(OrderStatusFlow::class)->isCod($record); + $result = $this->service()->createShipmentAndDispatch( $record, new ShipmentRequest( weight: filled($data['weight'] ?? null) ? (float) $data['weight'] : null, packageCount: (int) ($data['package_count'] ?? 1), destinationLocationId: $data['destination_location_id'] ?? null, + paymentMode: $isCod ? 'cod' : 'prepaid', + amountToCollect: $isCod ? $record->total->decimal : null, boxes: collect($data['boxes'] ?? [])->pluck('size')->all(), ), ); diff --git a/src/Shipping/Jobs/PollShipmentTrackingJob.php b/src/Shipping/Jobs/PollShipmentTrackingJob.php index 7714e22..7090aac 100644 --- a/src/Shipping/Jobs/PollShipmentTrackingJob.php +++ b/src/Shipping/Jobs/PollShipmentTrackingJob.php @@ -14,12 +14,19 @@ use Modules\Core\Shipping\Enums\TrackingStatus; use Modules\Core\Shipping\Events\ShipmentStatusUpdatedByCarrier; use Modules\Core\Shipping\Models\Shipment; use Modules\Core\Shipping\Models\ShipmentInfo; +use Throwable; /** * Carrier-agnostic: polls every Shipment not yet in a terminal state, * skipping carriers whose fulfillment service doesn't implement * SupportsTracking. New checkpoints are recorded in shipment_info and * dispatch ShipmentStatusUpdatedByCarrier — one event per new checkpoint. + * + * Each shipment's trackShipment() call is individually try/caught in + * pollCarrierShipments() — one shipment's tracking lookup failing (a + * carrier 500, a malformed parcel response) must not stop the rest of that + * carrier's shipments in the same batch from being polled. The failure is + * reported and the loop continues. */ class PollShipmentTrackingJob implements ShouldQueue { @@ -68,7 +75,11 @@ class PollShipmentTrackingJob implements ShouldQueue } foreach ($shipments as $shipment) { - $this->recordNewCheckpoints($shipment, $service->trackShipment($shipment)); + try { + $this->recordNewCheckpoints($shipment, $service->trackShipment($shipment)); + } catch (Throwable $e) { + report($e); + } } }