Skip to content

fix(checkout): still require a delivery address when the area restriction is disabled - #87

Merged
sampoyigi merged 1 commit into
tastyigniter:4.xfrom
iamnothardcoded:fix/require-delivery-address-when-area-check-disabled
Aug 4, 2026
Merged

fix(checkout): still require a delivery address when the area restriction is disabled#87
sampoyigi merged 1 commit into
tastyigniter:4.xfrom
iamnothardcoded:fix/require-delivery-address-when-area-check-disabled

Conversation

@iamnothardcoded

Copy link
Copy Markdown
Contributor

The problem

Since a252f72 (v4.2.0), the delivery address check in Checkout::validateCheckout() is gated on Location::requiresUserPosition():

if ($order->isDeliveryType() && Location::requiresUserPosition()) {

requiresUserPosition() is setting('location_order') == 1 — the "Reject Orders Outside Delivery Area" setting, which ships disabled by default (core/database/records/settings.json).

So on a default install, a delivery order can be placed with no address at all. The order is accepted with address_id = NULL, and the restaurant is left with an order it cannot deliver.

There is no second line of defence: the address fields are not part of resources/models/checkoutfields.php, so no validation rule covers them. They are populated solely by prepareDeliveryAddress() from the user position. When no position is set, $this->fields simply has no address keys — and that skipped after() callback was the only thing checking.

How to reproduce

  1. Leave "Reject Orders Outside Delivery Area" disabled (the default).
  2. Reach checkout in delivery mode without setting an address via the fulfilment modal.
  3. Place the order → it is accepted, orders.address_id is NULL.

Observed on three production installs after upgrading to v4.2.0; an install still on v4.1.6 correctly blocks the same attempt with the same setting disabled.

The fix

This keeps the intent of a252f72 fully intact — when the restriction is off, the address is still not geocoded and not area-checked, so customers are not blocked by geocoder failures, which is what that commit set out to fix.

It only adds back the weaker invariant: the address must exist. A delivery order without a street address is undeliverable regardless of whether the delivery area is enforced.

if ($order->isDeliveryType()) {
    if (Location::requiresUserPosition()) {
        // unchanged: geocode + delivery-area validation
    } elseif (blank(array_get($this->fields, 'address_1'))) {
        $validator->errors()->add('delivery_address', lang('igniter.local::default.alert_missing_street_address'));
    }
}

Reuses the existing igniter.local::default.alert_missing_street_address string, so no new translations are needed.

Tests

Two tests added:

  • onValidate requires a delivery address when the delivery area restriction is disabled — asserts the error is raised.
  • onValidate does not geocode the delivery address when the delivery area restriction is disabled — asserts Geocoder::geocode() is never called when an address is present, pinning the behaviour a252f72 introduced so it cannot regress.

tests/Livewire/CheckoutTest.php results (PHP 8.3, MariaDB 11):

result
baseline (this branch's base) 1 failed, 20 passed
with this change 1 failed, 22 passed

The single failure is pre-existing and identical on the unmodified base (onValidate adds a delivery address error when delivery address validation fails, a ViewException in my environment) — it is not affected by this change. vendor/bin/pint passes on both changed files.

…tion is disabled

Since a252f72 the delivery address check is skipped entirely when the
"Reject Orders Outside Delivery Area" setting (location_order) is disabled.
That setting ships disabled by default, so out of the box a delivery order
can be placed with no address at all: address_id ends up NULL and the order
is accepted, leaving the restaurant with an undeliverable order.

The address fields are not part of the checkout form config, so nothing else
validates them - the skipped after() callback was the only guard.

Keep the intent of a252f72 (no geocoding or delivery-area enforcement when
the restriction is off, so legitimate customers are not blocked by geocoder
failures), but still require the address to be present.
@iamnothardcoded
iamnothardcoded changed the base branch from master to 4.x August 2, 2026 23:36
iamnothardcoded added a commit to iamnothardcoded/tastyrhodos that referenced this pull request Aug 3, 2026
…rror

Two checkout fixes found during a UX pass on validation feedback.

1. Delivery orders were accepted with NO address at all (dev orders 666/667/
   668, elgrecomarl order 18 - all order_type=delivery, address_id=NULL,
   status 3). Root cause is upstream: ti-theme-orange v4.2.0 (commit a252f72d,
   released 2026-07-25) gated the checkout address check on
   Location::requiresUserPosition() == setting('location_order'), the "Reject
   Orders Outside Delivery Area" switch, which ships DISABLED by default. With
   it off the check never runs, and nothing else validates the address - the
   fields are not in checkoutfields.php and are only populated by
   prepareDeliveryAddress() from the user position.

   Filed upstream as tastyigniter/ti-theme-orange#87. Until that lands, jamasa
   guards it on the igniter.orange.validateCheckout event, which fires after
   upstream validation but BEFORE saveOrder, so nothing is persisted. Gated on
   Location::orderType() - exactly what applyRequiredAttributes() writes at save
   time - so the check cannot desync from what gets stored. Drop this guard when
   tastyigniter#87 merges.

2. A failed checkout re-rendered with the offending field off-screen, so on a
   phone nothing appeared to happen. famedo.js now scrolls the first visible
   error into view and focuses it, debounced until the commit burst from one
   submit goes quiet (a single tap fires blur + validate + confirm; acting on
   the earliest would scroll to the previous attempt's stale markers). Taps on
   the submit button do not disarm it: the button greys out for ~1s on a slow
   connection and an impatient re-tap is a retry, not the user moving on.

Also corrects the NominatimProvider docblock: core#65 merged, but that class
has since grown the geocoder-blind fail-open and the Photon suggestion system,
so the old "delete this class once upstream fixes it" note would now tear out
live features.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sampoyigi
sampoyigi merged commit dfd3402 into tastyigniter:4.x Aug 4, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants