From 359b645c627fec901e99d040754a69177a9dc3ed Mon Sep 17 00:00:00 2001 From: Konstantinos Arvanitakis Date: Tue, 29 Sep 2026 00:43:48 +0300 Subject: [PATCH] Feat: Hiding unused shipping types and removing unused fulfillment types --- src/Providers/ShippingServiceProvider.php | 30 ++++---- .../StorePickup/StorePickupRateDriver.php | 6 +- .../Contracts/DeclaresFulfillmentType.php | 18 ++--- .../ShippingMethodListExtension.php | 2 +- .../ShippingMethodResourceExtension.php | 69 ++----------------- src/Shipping/Support/FulfillmentType.php | 47 +++---------- src/Shipping/Support/ShippingManager.php | 7 +- 7 files changed, 46 insertions(+), 133 deletions(-) diff --git a/src/Providers/ShippingServiceProvider.php b/src/Providers/ShippingServiceProvider.php index 91656f7..64cd7e2 100644 --- a/src/Providers/ShippingServiceProvider.php +++ b/src/Providers/ShippingServiceProvider.php @@ -60,16 +60,6 @@ class ShippingServiceProvider extends ServiceProvider // first load, the Livewire registration covers every AJAX // round-trip (form submits, table interactions) afterwards. $this->app->bind(VendorManageShippingRates::class, ManageShippingRates::class); - - // Binds AFTER the vendor's own ShippingServiceProvider — the - // last-registered bind() for a given abstract wins in Laravel's - // container, and vendor providers register before this one lists - // theirs as a dependency implicitly via composer.json's package - // discovery order — see Modules\Core\Shipping\Support\ - // ShippingManager's own docblock for why this override exists at - // all (dropping the vendor's generic drivers from - // getSupportedDrivers()). - $this->app->bind(ShippingMethodManagerInterface::class, fn ($app) => $app->make(ShippingManager::class)); } public function boot(): void @@ -94,10 +84,9 @@ class ShippingServiceProvider extends ServiceProvider // resolveCarrier() for the same lookup pattern already used to // resolve a carrier driver from it). // - // Resolves via Modules\Core\Shipping\Support\FulfillmentType (driver- - // declared for acs/box-now, merchant-configured data['fulfillment_type'] - // fallback for table-rate-shipping's generic drivers) rather than - // through a ShippingMethod::macro('isStorePickup', ...) — + // Resolves via Modules\Core\Shipping\Support\FulfillmentType + // (hardcoded per driver — see that class's own docblock) rather + // than through a ShippingMethod::macro('isStorePickup', ...) — // Lunar\Base\Traits\HasModelExtending::__callStatic() (used by // Lunar\Shipping\Models\ShippingMethod via Lunar\Base\BaseModel) // intercepts EVERY unmatched static call, including macro() @@ -132,8 +121,19 @@ class ShippingServiceProvider extends ServiceProvider // Deferred: the Shipping facade resolves a binding registered in // lunarphp/table-rate-shipping's own ShippingServiceProvider::boot(), - // and provider boot order between packages isn't guaranteed. + // and provider boot order between packages isn't guaranteed — in + // fact composer's package discovery registers boboko/core BEFORE + // lunarphp/table-rate-shipping (alphabetical), so a bind() in this + // provider's own register()/boot() runs first and gets silently + // overwritten by the vendor's own later bind() of the same + // abstract. booted() is the first point every provider's + // register()/boot() has definitely already run, so this is also + // where the ShippingMethodManagerInterface override belongs (must + // run before the Shipping::extend() calls below, which resolve — + // and the facade then CACHES — whatever's bound at that moment). $this->app->booted(function () { + $this->app->bind(ShippingMethodManagerInterface::class, fn ($app) => $app->make(ShippingManager::class)); + Shipping::extend('acs', fn ($app) => $app->make(AcsRateDriver::class)); Shipping::extend('box-now', fn ($app) => $app->make(BoxNowRateDriver::class)); Shipping::extend('store-pickup', fn ($app) => $app->make(StorePickupRateDriver::class)); diff --git a/src/Shipping/Carriers/StorePickup/StorePickupRateDriver.php b/src/Shipping/Carriers/StorePickup/StorePickupRateDriver.php index 2655bd5..19dc5f4 100644 --- a/src/Shipping/Carriers/StorePickup/StorePickupRateDriver.php +++ b/src/Shipping/Carriers/StorePickup/StorePickupRateDriver.php @@ -23,9 +23,9 @@ use Modules\Core\Shipping\Contracts\DeclaresFulfillmentType; * ShippingMethodName's own docblock), so the storefront showed the raw * JSON blob as the option's name instead of the translated string. * 2. Unambiguously store pickup, like ACS/Box Now are unambiguously - * carrier — implementing DeclaresFulfillmentType means a merchant never - * has to separately pick "Collect in store" from the generic - * data['fulfillment_type'] field (see that contract's own docblock). + * carrier — implementing DeclaresFulfillmentType means this is a + * hardcoded fact about the driver, never a merchant configuration + * choice (see that contract's own docblock). * * No live pricing — there's no API for in-person pickup, just the method's * own charge_by + price-break configuration (usually free), the same diff --git a/src/Shipping/Contracts/DeclaresFulfillmentType.php b/src/Shipping/Contracts/DeclaresFulfillmentType.php index 7eced62..9ed0c85 100644 --- a/src/Shipping/Contracts/DeclaresFulfillmentType.php +++ b/src/Shipping/Contracts/DeclaresFulfillmentType.php @@ -3,18 +3,14 @@ namespace Modules\Core\Shipping\Contracts; /** - * Optional contract a shipping rate driver implements to declare whether - * it fulfils via carrier delivery or in-store pickup — e.g. + * Every shipping rate driver this module registers implements this to + * declare whether it fulfils via carrier delivery or in-store pickup — * Modules\Core\Shipping\Carriers\Acs\AcsRateDriver and BoxNowRateDriver - * are unambiguously carrier-only, so this is a hardcoded fact about the - * driver, not something a merchant should have to configure per row. - * - * table-rate-shipping's own generic drivers (flat-rate, ship-by, - * free-shipping) don't implement this — they're genuinely ambiguous (a - * merchant could configure one for either carrier delivery or store - * pickup), so Modules\Core\Shipping\Support\FulfillmentType::resolve() - * falls back to ShippingMethod.data['fulfillment_type'] (still merchant- - * overridable) only for drivers that don't implement this contract. + * are unambiguously carrier-only, Modules\Core\Shipping\Carriers\ + * StorePickup\StorePickupRateDriver unambiguously store-pickup, so this + * is a hardcoded fact about each driver, never something a merchant + * configures per row. See Modules\Core\Shipping\Support\FulfillmentType:: + * resolve(), the single source of truth this feeds. */ interface DeclaresFulfillmentType { diff --git a/src/Shipping/Extensions/ShippingMethodListExtension.php b/src/Shipping/Extensions/ShippingMethodListExtension.php index 61ad91c..6359709 100644 --- a/src/Shipping/Extensions/ShippingMethodListExtension.php +++ b/src/Shipping/Extensions/ShippingMethodListExtension.php @@ -48,6 +48,6 @@ class ShippingMethodListExtension extends BaseExtension ->label('Type') ->options(fn () => collect(Shipping::getSupportedDrivers()) ->mapWithKeys(fn ($driver, $key) => [$key => $driver->name()])) - ->default('flat-rate'); + ->default('acs'); } } diff --git a/src/Shipping/Extensions/ShippingMethodResourceExtension.php b/src/Shipping/Extensions/ShippingMethodResourceExtension.php index 1da2c8a..f9675a3 100644 --- a/src/Shipping/Extensions/ShippingMethodResourceExtension.php +++ b/src/Shipping/Extensions/ShippingMethodResourceExtension.php @@ -13,7 +13,6 @@ use Filament\Tables\Table; use Lunar\Admin\Support\Extending\ResourceExtension; use Lunar\Admin\Support\Forms\Components\TranslatedText; use Lunar\Shipping\Facades\Shipping; -use Modules\Core\Shipping\Contracts\DeclaresFulfillmentType; use Modules\Core\Shipping\Contracts\SupportsLivePricing; use Modules\Core\Shipping\Support\ShippingMethodName; @@ -22,11 +21,9 @@ class ShippingMethodResourceExtension extends ResourceExtension public function extendForm(Schema $schema): Schema { return $schema->components( - $this->replaceFulfillmentTypeField( - $this->replaceChargeByField( - $this->replaceNameField( - $this->replaceDriverField($schema->getComponents()) - ) + $this->replaceChargeByField( + $this->replaceNameField( + $this->replaceDriverField($schema->getComponents()) ) ) ); @@ -98,64 +95,6 @@ class ShippingMethodResourceExtension extends ResourceExtension return $field; } - /** - * Inserts the `fulfillment_type` Select right after `charge_by`, in - * the SAME Group (vendor's own `Group::make([getChargeByFormComponent()]) - * ->columns(2)`) — formerly appended at the very end of the whole - * form, disconnected from `driver`/`charge_by`, the decisions it - * actually relates to. Only rendered at all for a driver that DOESN'T - * already declare its own fulfillment type (see Modules\Core\Shipping\ - * Contracts\DeclaresFulfillmentType, Modules\Core\Shipping\Support\ - * FulfillmentType) — acs/box-now are unambiguously carrier-only, so - * asking a merchant to also pick "Carrier delivery" for every ACS/Box - * Now method was redundant, error-prone config with no real decision - * behind it. Still offered for table-rate-shipping's generic drivers - * (flat-rate, ship-by, free-shipping), which are genuinely ambiguous. - */ - private function replaceFulfillmentTypeField(array $components): array - { - $result = []; - - foreach ($components as $component) { - $result[] = $component; - - if (method_exists($component, 'getName') && $component->getName() === 'charge_by') { - $result[] = $this->fulfillmentTypeSelect(); - } elseif (in_array(HasChildComponents::class, class_uses_recursive($component), true)) { - $component->schema($this->replaceFulfillmentTypeField($component->getChildComponents())); - } - } - - return $result; - } - - private function fulfillmentTypeSelect(): Select - { - return Select::make('data.fulfillment_type') - ->label('Fulfillment type') - ->options([ - 'carrier' => 'Carrier delivery', - 'store_pickup' => 'Collect in store', - ]) - ->default('carrier') - ->required() - ->visible(fn (Get $get) => $this->driverIsFulfillmentAmbiguous($get('../driver'))) - ->helperText('Whether an order using this method is handed to a carrier, or collected by the customer in person.'); - } - - private function driverIsFulfillmentAmbiguous(?string $driver): bool - { - if (! $driver) { - return true; - } - - try { - return ! Shipping::driver($driver) instanceof DeclaresFulfillmentType; - } catch (InvalidArgumentException) { - return true; - } - } - /** * Extend the vendor's cart_total/weight charge_by Select with a third * "live" option — only offered when the currently selected driver @@ -297,7 +236,7 @@ class ShippingMethodResourceExtension extends ResourceExtension ->label('Type') ->options(fn () => collect(Shipping::getSupportedDrivers()) ->mapWithKeys(fn ($driver, $key) => [$key => $driver->name()])) - ->default('flat-rate') + ->default('acs') ->live(); } } diff --git a/src/Shipping/Support/FulfillmentType.php b/src/Shipping/Support/FulfillmentType.php index 84b6373..2d25dd4 100644 --- a/src/Shipping/Support/FulfillmentType.php +++ b/src/Shipping/Support/FulfillmentType.php @@ -8,20 +8,16 @@ use Modules\Core\Shipping\Contracts\DeclaresFulfillmentType; /** * The single source of truth for "is this ShippingMethod a carrier - * delivery or an in-store pickup" — replaces a merchant-facing - * data['fulfillment_type'] Select that used to exist for every method - * regardless of driver. Modules\Core\Shipping\Carriers\Acs\AcsRateDriver - * and BoxNowRateDriver are unambiguously carrier-only (see - * Modules\Core\Shipping\Contracts\DeclaresFulfillmentType's own - * docblock), so asking a merchant to also pick "Carrier delivery" for - * every ACS/Box Now method was redundant, error-prone config with no - * real decision behind it. - * - * table-rate-shipping's own generic drivers (flat-rate, ship-by, - * free-shipping) don't implement DeclaresFulfillmentType — a merchant - * could genuinely configure one for either purpose (e.g. "Flat Rate — - * Athens Store Pickup") — so those still fall back to the merchant-set - * data['fulfillment_type'], defaulting to 'carrier' when unset. + * delivery or an in-store pickup" — every driver this module registers + * (ACS, Box Now, Modules\Core\Shipping\Carriers\StorePickup\ + * StorePickupRateDriver) implements DeclaresFulfillmentType, so this is + * now a hardcoded fact about the driver, never a merchant choice. There + * used to be a merchant-facing data['fulfillment_type'] Select as a + * fallback for table-rate-shipping's own generic drivers (flat-rate, + * ship-by, free-shipping, collection), which were genuinely ambiguous + * (a merchant could configure one for either purpose) — those drivers + * are no longer offered at all (see Modules\Core\Shipping\Support\ + * ShippingManager), so the fallback and the field it read from are gone. */ class FulfillmentType { @@ -29,32 +25,11 @@ class FulfillmentType { $driver = collect(Shipping::getSupportedDrivers())->get($method->driver); - if ($driver instanceof DeclaresFulfillmentType) { - return $driver->fulfillmentType(); - } - - return $method->data['fulfillment_type'] ?? 'carrier'; + return $driver instanceof DeclaresFulfillmentType ? $driver->fulfillmentType() : 'carrier'; } public static function isStorePickup(ShippingMethod $method): bool { return static::resolve($method) === 'store_pickup'; } - - /** - * Whether the merchant-facing "Fulfillment type" Select should be - * shown at all for a given driver — hidden entirely for a driver that - * already declares its own fulfillment type, since there is no real - * decision left for the merchant to make. - */ - public static function isConfigurableFor(?string $driverKey): bool - { - if ($driverKey === null) { - return true; - } - - $driver = collect(Shipping::getSupportedDrivers())->get($driverKey); - - return ! $driver instanceof DeclaresFulfillmentType; - } } diff --git a/src/Shipping/Support/ShippingManager.php b/src/Shipping/Support/ShippingManager.php index 538b663..f63f4d7 100644 --- a/src/Shipping/Support/ShippingManager.php +++ b/src/Shipping/Support/ShippingManager.php @@ -20,8 +20,11 @@ use Lunar\Shipping\Managers\ShippingManager as VendorShippingManager; * only needs to override the one method that lists what's offered. * * Bound over the vendor's own ShippingMethodManagerInterface binding in - * ShippingServiceProvider — see that class for why (registration order: - * this module's provider binds after the vendor's own). + * ShippingServiceProvider, from inside its own $this->app->booted(...) + * callback — a plain register()-time bind() here runs BEFORE the + * vendor's own (composer discovers boboko/core before lunarphp/ + * table-rate-shipping alphabetically), so the vendor's later bind() + * would silently win instead. See that provider's own comment. */ class ShippingManager extends VendorShippingManager {