diff --git a/server/src/Http/Requests/UpdateOrderRequest.php b/server/src/Http/Requests/UpdateOrderRequest.php index c43458007..fe6532360 100644 --- a/server/src/Http/Requests/UpdateOrderRequest.php +++ b/server/src/Http/Requests/UpdateOrderRequest.php @@ -4,4 +4,27 @@ class UpdateOrderRequest extends CreateOrderRequest { + /** + * Get the validation rules that apply to the request. + * + * An update edits an order that already has a payload, so a `payload` object may carry only + * the parts that change: entities alone, a new dropoff alone, and so on. The route endpoints + * that creation requires are therefore optional here. An empty waypoints list is allowed: + * the controller treats it as "remove the intermediate stops". + * + * @return array + */ + public function rules(): array + { + $validations = parent::rules(); + + if ($this->has('payload') && $this->isArray('payload')) { + $validations['payload'] = 'array'; + $validations['payload.pickup'] = 'nullable'; + $validations['payload.dropoff'] = 'nullable'; + $validations['payload.waypoints'] = 'array'; + } + + return $validations; + } } diff --git a/server/src/Models/Place.php b/server/src/Models/Place.php index ce68be110..d60d3d66d 100644 --- a/server/src/Models/Place.php +++ b/server/src/Models/Place.php @@ -405,6 +405,63 @@ public static function insertFromGeocodingLookup(string $address) return static::insertGetUuid(static::getValuesFromGeocodingLookup($address)); } + /** + * Whether a place array carries address fields, as opposed to being only coordinates. + */ + public static function hasStructuredAddress(mixed $place): bool + { + if (!is_array($place)) { + return false; + } + + foreach (['street1', 'address', 'city', 'postal_code'] as $field) { + if (static::normalizePlaceValue(data_get($place, $field)) !== null) { + return true; + } + } + + return false; + } + + /** + * Turn `latitude`/`longitude` (or `lat`/`lng`) on a structured place array into its + * `location`, unless a location was given explicitly. The loose keys are removed so they are + * not mass assigned. + * + * @param array $place + * + * @return array + */ + public static function withProvidedLocation(array $place): array + { + $coordinates = array_intersect_key($place, array_flip(['latitude', 'longitude', 'lat', 'lng', 'lon'])); + + if (empty($place['location']) && count($coordinates) >= 2) { + $point = Utils::getPointFromCoordinatesStrict($coordinates); + if ($point && !($point->getLat() == 0.0 && $point->getLng() == 0.0)) { + $place['location'] = new SpatialPoint($point->getLat(), $point->getLng()); + } + } + + return array_diff_key($place, array_flip(['latitude', 'longitude', 'lat', 'lng', 'lon'])); + } + + /** + * Whether a structured place already has both a street and a usable location. + * + * @param array $place + */ + public static function hasProvidedLocationAndStreet(array $place): bool + { + if (static::normalizePlaceValue(data_get($place, 'street1')) === null || empty($place['location'])) { + return false; + } + + $point = Utils::getPointFromMixed($place['location']); + + return $point instanceof SpatialPoint && !($point->getLat() == 0.0 && $point->getLng() == 0.0); + } + /** * Compose a fuller geocoding query from structured place attributes. */ @@ -708,8 +765,9 @@ public static function createFromMixed($place, $attributes = [], $saveInstance = // Return a new Place object created from a geocoding lookup return static::createFromGeocodingLookup($place, $saveInstance); } - // If $place is an array of coordinates - elseif (Utils::isCoordinatesStrict($place)) { + // If $place is only coordinates. An array that also carries address fields is a + // structured place: its address is kept and the coordinates become its location. + elseif (Utils::isCoordinatesStrict($place) && !static::hasStructuredAddress($place)) { return static::createFromCoordinates($place, $attributes, $saveInstance); } // If $place is an array @@ -730,12 +788,17 @@ public static function createFromMixed($place, $attributes = [], $saveInstance = return $existingPlace; } + // Coordinates sent alongside the address are the caller's location for it. + $place = static::withProvidedLocation($place); + $query = static::composeGeocodingQuery($place) ?? static::normalizePlaceValue(data_get($place, 'address')) ?? static::normalizePlaceValue(data_get($place, 'street1')); + // A street address that arrives with its own coordinates needs no lookup: geocoding + // would only add latency and could move the pin to a different nearby address. $geocoded = []; - if ($query) { + if ($query && !static::hasProvidedLocationAndStreet($place)) { $geocoded = static::getValuesFromGeocodingLookup($query); } diff --git a/server/tests/RequestContractsTest.php b/server/tests/RequestContractsTest.php index 045909958..a77d8d20e 100644 --- a/server/tests/RequestContractsTest.php +++ b/server/tests/RequestContractsTest.php @@ -154,6 +154,7 @@ public function __toString(): string use Fleetbase\FleetOps\Http\Requests\UpdateDeviceRequest; use Fleetbase\FleetOps\Http\Requests\UpdateFuelReportRequest; use Fleetbase\FleetOps\Http\Requests\UpdateIssueRequest; + use Fleetbase\FleetOps\Http\Requests\UpdateOrderRequest; use Fleetbase\FleetOps\Http\Requests\UpdateWorkOrderRequest; use Fleetbase\FleetOps\Http\Requests\VerifyCreateCustomerRequest; use Fleetbase\FleetOps\Rules\ComputableAlgo; @@ -197,6 +198,19 @@ function ruleStrings(array $rules): array return $strings; } + class FleetOpsPublicUpdateOrderRequestProbe extends UpdateOrderRequest + { + public function isArray(string $key): bool + { + return is_array($this->input($key)); + } + + public function isString(string $key): bool + { + return is_string($this->input($key)); + } + } + class FleetOpsPublicCreateOrderRequestProbe extends CreateOrderRequest { public function isArray(string $key): bool @@ -608,6 +622,40 @@ protected function canUpdateDriver(): bool ]); }); + test('public update order request accepts partial payload edits', function () { + app('config')->set('fleetops.pod_methods', ['scan', 'signature']); + + // Entities alone: an edit to an existing order need not resend its route + $entitiesOnly = FleetOpsPublicUpdateOrderRequestProbe::create('/fleetops-test', 'PUT', [ + 'payload' => ['entities' => [['name' => 'Loaf']]], + ])->rules(); + + expect($entitiesOnly['payload'])->toBe('array') + ->and($entitiesOnly['payload.entities'])->toBe('array') + ->and($entitiesOnly['payload.pickup'])->toBe('nullable') + ->and($entitiesOnly['payload.dropoff'])->toBe('nullable') + ->and($entitiesOnly['payload.waypoints'])->toBe('array') + ->and($entitiesOnly['payload.return'])->toBe('nullable'); + + // A new dropoff alone, and an empty waypoints list that clears intermediate stops + $dropoffOnly = FleetOpsPublicUpdateOrderRequestProbe::create('/fleetops-test', 'PUT', [ + 'payload' => ['dropoff' => ['street1' => '500 Pine St'], 'waypoints' => []], + ])->rules(); + + expect($dropoffOnly['payload.pickup'])->toBe('nullable') + ->and($dropoffOnly['payload.waypoints'])->toBe('array'); + + // A payload id and top-level fields keep the create contract + $payloadId = FleetOpsPublicUpdateOrderRequestProbe::create('/fleetops-test', 'PUT', [ + 'payload' => 'payload_abc1234', + ])->rules(); + $noPayload = FleetOpsPublicUpdateOrderRequestProbe::create('/fleetops-test', 'PUT', ['notes' => 'x'])->rules(); + + expect($payloadId['payload'])->toBe('required|exists:payloads,public_id') + ->and($noPayload)->not->toHaveKeys(['payload', 'pickup', 'dropoff', 'waypoints']) + ->and($noPayload['customer'][1])->toBeInstanceOf(CustomerIdOrDetails::class); + }); + test('internal create order request validates uuid payload contracts and messages', function () { app('config')->set('fleetops.pod_methods', ['scan', 'signature']); diff --git a/server/tests/Unit/Models/PlaceCreationAndImportTest.php b/server/tests/Unit/Models/PlaceCreationAndImportTest.php index 551dd7413..7d2df89a0 100644 --- a/server/tests/Unit/Models/PlaceCreationAndImportTest.php +++ b/server/tests/Unit/Models/PlaceCreationAndImportTest.php @@ -389,3 +389,109 @@ public function reverseQuery($query) expect(Place::insertFromCoordinates([1.42, 103.92], ['name' => 'Array Inserted Stop']))->toBeFalse() ->and($connection->table('places')->where('name', 'Array Inserted Stop')->count())->toBe(0); }); + +/** + * Geocoder fake that records whether it was asked anything and answers reverse lookups with a + * different, nearby address, which is what used to replace the street a caller sent. + */ +function fleetopsPlaceRecordingGeocoder(): object +{ + $geocoder = new class { + public array $calls = []; + + public function geocodeQuery($query) + { + $this->calls[] = 'geocode'; + + return $this; + } + + public function reverseQuery($query) + { + $this->calls[] = 'reverse'; + + return $this; + } + + public function all() + { + return []; + } + + public function __call($method, $arguments) + { + return $this; + } + }; + app()->instance('fleetops.geocoder', $geocoder); + + return $geocoder; +} + +test('a structured address sent with coordinates keeps its street and location without a lookup', function () { + $connection = fleetopsPlaceCreationBoot(); + $geocoder = fleetopsPlaceRecordingGeocoder(); + + $place = Place::createFromMixed([ + 'street1' => '500 Pine St', + 'city' => 'Seattle', + 'province' => 'WA', + 'postal_code' => '98101', + 'country' => 'US', + 'latitude' => 47.6114, + 'longitude' => -122.3373, + ]); + + $location = Fleetbase\FleetOps\Support\Utils::getPointFromMixed($place->location); + + expect($place)->toBeInstanceOf(Place::class) + ->and($place->street1)->toBe('500 Pine St') + ->and($place->city)->toBe('Seattle') + ->and($location->getLat())->toBe(47.6114) + ->and($location->getLng())->toBe(-122.3373) + ->and($geocoder->calls)->toBe([]) + ->and($connection->table('places')->where('street1', '500 Pine St')->count())->toBe(1); +}); + +test('bare coordinates still resolve through a reverse lookup', function () { + fleetopsPlaceCreationBoot(); + $geocoder = fleetopsPlaceRecordingGeocoder(); + + $place = Place::createFromMixed(['latitude' => 47.6114, 'longitude' => -122.3373], [], true); + + expect($place)->toBeInstanceOf(Place::class) + ->and($geocoder->calls)->toBe(['reverse']); +}); + +test('an address without coordinates is still geocoded', function () { + fleetopsPlaceCreationBoot(); + $geocoder = fleetopsPlaceRecordingGeocoder(); + + $place = Place::createFromMixed(['street1' => '1100 4th Ave', 'city' => 'Seattle', 'country' => 'US']); + + expect($place->street1)->toBe('1100 4th Ave') + ->and($geocoder->calls)->toBe(['geocode']); +}); + +test('structured address helpers detect addresses and provided locations', function () { + expect(Place::hasStructuredAddress(['latitude' => 1.3, 'longitude' => 103.8]))->toBeFalse() + ->and(Place::hasStructuredAddress(['latitude' => 1.3, 'longitude' => 103.8, 'street1' => ' ']))->toBeFalse() + ->and(Place::hasStructuredAddress(['postal_code' => '98101', 'lat' => 1.3, 'lng' => 103.8]))->toBeTrue() + ->and(Place::hasStructuredAddress('1100 4th Ave'))->toBeFalse(); + + $converted = Place::withProvidedLocation(['street1' => 'A', 'lat' => 1.3, 'lng' => 103.8]); + expect($converted)->not->toHaveKeys(['lat', 'lng']) + ->and($converted['location'])->toBeInstanceOf(Point::class) + ->and($converted['location']->getLat())->toBe(1.3); + + // An explicit location wins, a 0,0 pair is a geocoder miss and is dropped, a lone key is ignored + $explicit = Place::withProvidedLocation(['street1' => 'A', 'location' => 'keep', 'latitude' => 9, 'longitude' => 9]); + expect($explicit['location'])->toBe('keep') + ->and(Place::withProvidedLocation(['street1' => 'A', 'latitude' => 0, 'longitude' => 0]))->not->toHaveKey('location') + ->and(Place::withProvidedLocation(['street1' => 'A', 'latitude' => 1.3]))->toBe(['street1' => 'A']); + + expect(Place::hasProvidedLocationAndStreet(['street1' => 'A', 'location' => new Point(1.3, 103.8)]))->toBeTrue() + ->and(Place::hasProvidedLocationAndStreet(['street1' => 'A', 'location' => new Point(0, 0)]))->toBeFalse() + ->and(Place::hasProvidedLocationAndStreet(['street1' => 'A']))->toBeFalse() + ->and(Place::hasProvidedLocationAndStreet(['location' => new Point(1.3, 103.8)]))->toBeFalse(); +});