Expand public Fleet, Vehicle, and Driver API contracts - #311
Open
roncodes wants to merge 8 commits into
Open
Conversation
The public v1 API exposed a small subset of what these records can hold, and the
gaps were silent rather than loud: a caller sending a field the controller did
not copy received a 200 and a response body that looked correct while the value
was discarded.
Fleets
- Create and update accept name, color, task, status, and the service_area,
zone, vendor and parent_fleet relationships as public ids. Only name and
service_area were reachable before, so a fleet hierarchy could not be built
through the API at all.
- parent_fleet: null clears a parent. A fleet may not be its own parent, nor sit
beneath one of its own descendants; both answer 422.
- Four public membership endpoints, all taking public ids and sharing one
response shape:
POST|DELETE /v1/fleets/{fleet}/vehicles/{vehicle}
POST|DELETE /v1/fleets/{fleet}/drivers/{driver}
Assignment is idempotent and restores a soft-deleted membership rather than
duplicating it; removal is a safe no-op and touches only the pivot.
Vehicles
- The input projection covered 21 of the model's 99 fields; it now covers all 90
safe ones, with type-appropriate validation for each.
- vendor, category, warranty and photo resolve from public ids.
- The create-time `online` default no longer applies to updates, where it
silently took a vehicle offline on any partial write.
Drivers
- Replaces an except() blocklist with an explicit allowlist. Anything nobody had
thought to exclude — auth_token, user_uuid, company_uuid — reached
Driver::create() intact, while location, heading, altitude, speed and meta
were dropped on every write.
- email and phone are optional. An operational record may have neither; nothing
is invented to fill the gap, and no invitation is sent when there is nowhere
to send one. Such a driver cannot sign in to Navigator until credentials are
supplied.
- Driver::$fillable held 'meta,' — a trailing comma inside the string — so meta
was never mass assignable.
- Driver photo upload wrote photo_uuid to users, which has no such column, so
every photo uploaded through the public API was dropped.
Tenant isolation
- Relationship inputs are validated with company-scoped exists rules and
resolved again through a company-scoped lookup. A cross-company public id is
answered exactly as a missing one, so a response cannot be used to probe
another organization's data.
- Relationship filters resolved public ids against uuid columns and so could
never match. FleetFilter::query searched a `user` relation Fleet does not
have, DriverFilter::phone a `phone` relation that does not exist, and
FleetFilter::zone a zone_uuid column zones does not have.
- Public responses report relationships as public ids; no *_uuid column appears
in a public payload. Internal console responses keep their existing shape.
Validation: php scripts/pest-file-runner.php — 434 files, exit 0.
composer test:lint reports 4 files, all pre-existing on origin/main and none
touched here. composer test:types fails on a pre-existing 13,739-error baseline;
the four new source files report zero.
6 tasks
The coverage gate caught 17 statements the new code added but no test entered. Every one is now reached by a test that asserts the behaviour, not by a call made only to move the number. - CreateFleetRequest::attributes() — asserted in RequestContractsTest, which already pins the rest of the fleet request contract. - PublicRelationNotFoundException::getRelation()/getIdentifier() — covered in ExceptionContractsTest alongside the other FleetOps exceptions, including the null-identifier case. - ResolvesPublicRelationUuids' blank-identifier early return — a filter given an empty value must resolve to nothing without reaching the database. - DriverFilter's console uuid branch — Http::isInternalRequest() reads the resolved route's uri rather than the request path, so the branch needs a request with an internal route resolver to be reachable at all. The test now builds one, which is also what proves the branch is internal-only. - FleetController: the update path's cross-company relationship rejection (the create path was already covered), removeVehicle's and removeDriver's not-found answers, and the real bodies of findVehicle, findDriver, withPublicRelations and queryFleets — the last four exercised against SQLite in FleetPublicContractTest, which asserts that the lookups are company-scoped and that the query pipeline eager loads the relations the public resource reports as public ids. Local baseline: 100.00% on all three metrics — 34670/34670 statements, 4428/4428 methods, 530/530 classes. The statement total matches the figure CI reported exactly, so the 17 closed here are precisely the ones it flagged. php scripts/pest-file-runner.php: 434 files, exit 0.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v0.6.62 #311 +/- ##
====================================================
Coverage 100.00% 100.00%
- Complexity 9899 10003 +104
====================================================
Files 526 531 +5
Lines 38163 38585 +422
====================================================
+ Hits 38163 38585 +422
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…eness
Corrects the response design from the first revision of this branch and closes
the remaining gaps.
Relationships are two keys, never one key with two types
- The previous revision returned a public id *under* the object key — so
`vendor` was an object when expanded and a string otherwise. The SDK stores
what the API returns verbatim (`Resource::$attributes = $attributes`, no
normalisation), so a property that changes type between calls breaks every
consumer that dereferences it. Navigator interpolates `driver.user` straight
into a socket channel name; an object there subscribes it to
`user.[object Object]` and quietly stops delivering messages.
- Each relationship now has an always-present `<name>_id` public identifier and,
separately, the nested `<name>` object with exactly the shape the base branch
returned: absent unless loaded, an object when loaded, never a string.
Fleet gains service_area_id, zone_id, vendor_id, parent_fleet_id, photo_id;
Vehicle gains driver_id, vendor_id, category_id, warranty_id, photo_id.
No key was removed from any resource — the diff against the base is additive
on all three.
- Driver keeps `user` and `company` as the public-id strings they have always
been, keeps `company_name`, and keeps `vehicle`, `vendor` and `current_job` as
objects on create, update and retrieve, which still load them without a
`with`.
Expansion
- `?with=vendor`, `?with[]=vendor`, `?with=vendor,driver` and the `expand` alias
all normalise to one list. Names are mapped through an explicit per-resource
allowlist, so nothing user-controlled reaches `load()` — Core hands `with`
straight to Eloquent, where an unknown name is a 500 for what is only a typo.
Unsupported names are ignored.
- `?with=subfleets` finally resolves. The public name and the relation differ
only in case, and because PHP method calls are case-insensitive the old
`load('subfleets')` stored a second copy of the relation under a mis-cased
key that `whenLoaded('subFleets')` could not see. Nested
`subfleets.drivers` / `subfleets.vehicles` work explicitly, and the implicit
nesting the released contract had is preserved.
Membership uniqueness is now the database's job
- A migration adds composite unique indexes on fleet_vehicles(fleet_uuid,
vehicle_uuid) and fleet_drivers(fleet_uuid, driver_uuid), collapsing existing
duplicates first: an active row wins over a tombstone, the lowest id wins
among equals, and a pair that is entirely soft-deleted keeps one restorable
row. Only redundant pivot rows are removed.
- Soft-deleted rows stay inside the key, deliberately — a removed membership is
restored on re-assignment rather than replaced, so its key must stay taken.
- The assignment path catches only the duplicate-key violation, adopts the
winner's row and answers successfully. Any other violation is rethrown rather
than reported as a successful assignment.
internal_id and public_id
- Exact on the public API, partial in the console, chosen by the resolved route.
An importer asking whether VEH-10 exists must not be told yes because VEH-100
does; the console's search box must keep finding it. VIN and plate number are
untouched.
Driver write/read parity
- `timezone` is copied to the linked user on update. It was accepted,
documented, answered 200 — and dropped, because the update copied only name,
email and phone and the driver record has no timezone column.
- Email and phone uniqueness is now enforced on update against every other live
user, ignoring the driver's own by uuid, so an unchanged value still succeeds.
Also fixed: `assignedOrdersCount()` and `currentOrderReference()` ran a query on
every public Vehicle and Driver response and discarded the result, because
`when()` evaluates a plain value argument eagerly.
Validation: php scripts/pest-file-runner.php — 437 files, exit 0. Coverage
100.00% on all three metrics (34784/34784 statements, 4441/4441 methods,
531/531 classes). SDK and Navigator contract compatibility verified against the
untouched checkouts; neither was modified.
Validates this branch against fleetbase/postman@0fcdff3, the commit that documents and asserts these endpoints. The collection on postman main predates every endpoint added here, so a run against it proves only that nothing already released regressed. Reverted by the next commit — release-bound code must not stay pinned to an unmerged branch.
The cross-repository contract run answered 500 for
`GET /v1/vehicles/{id}?with[]=vendor&with[]=not_a_relation`:
"Call to undefined relationship [not_a_relation]".
Both retrieve actions took the request as an optional parameter — Vehicle as
`?Request $request = null`, Driver not at all — and Laravel's controller
dispatcher skips resolving a type-hinted dependency that carries a default. It
arrived null, the expansions were never mapped or allowlisted, and the raw name
reached Eloquent. Retrieve is the endpoint most likely to be handed a stale
relation name, and it was the only one where the allowlist did not run.
Both now read the container's request. Covered by a test that drives the real
find() with an unsupported name and asserts the response is intact.
Still reverted before merge.
…n commit" This reverts commit a249e7d.
This reverts commit d303483.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The public v1 API exposed a small subset of what the Fleet, Vehicle and Driver
records can hold, and the gaps were silent rather than loud: a caller sending a
field the controller did not copy received a
200and a response body thatlooked correct while the value was discarded.
This PR expands those contracts, adds fleet hierarchies and fleet membership,
and makes a driver recordable without credentials — while leaving every
released response shape intact.
The compatibility rule this PR is built around
The SDK is a pass-through:
Resource::$attributes = $attributes, andgetAttribute()is a path read over that object. Nothing normalises a propertythat changes between an object, a string, and absent. So an existing
relationship key keeps its name and its type, always, and anything new arrives
beside it.
Navigator makes that concrete. It reads
driver.userin eight places and usesit as a scalar:
An object there subscribes it to
user.[object Object]and quietly stopsdelivering messages.
driver.userstays a public-id string, and is notexpandable at all.
Compatibility matrix, taken from the merge base (
a9131dae)usercompany,company_namevehicle,vendor,current_jobwhenLoadedobject — and create/update/retrieve load themwithneededvehicle_id,vendor_id,job_id,timezone,bearing,current_status, orchestrator fieldsdriverwhenLoadedobject, never loaded by the public controller → absentwith=driver, else absent)vendor,category,warranty,photowith, plus*_iddriver_id,vendor_id,category_id,warranty_id,photo_idservice_area,zone,vendor,parent_fleetwhenLoadedobject → absent unless?with=service_area_id,zone_id,vendor_id,parent_fleet_id,photo_id,color,photo_url,photoA key-by-key diff of all three resources against the base reports
removed = none. Every change is additive.This corrects the first revision of this branch, which returned a public id
under the object key — making
vendoran object when expanded and a stringotherwise. That is the exact failure the rule above exists to prevent.
Relationship design
{ "vendor_id": "vendor_123", "vendor": { "id": "vendor_123", "name": "Acme" } }<name>_idis always present, is a public ID ornull, and does not changewhen the object appears beside it.
<name>is the nested object, with the base branch's shape: present when therelation was loaded, absent otherwise, never a string.
fleets,drivers,vehicles,subfleets,devices) keep their existing behaviour; no*_idsarrays were added.Expansion
?with=vendor,?with[]=vendor,?with[]=vendor&with[]=driver,?with=vendor,driverand theexpandalias of each all normalise to one list.Names are mapped through an explicit per-resource allowlist before anything
reaches Eloquent. Core reads
with/expanditself and hands the value straightto
$result->load(...)with no allowlist, so today any string reachesload()and an unknown one is a
500for what is only a typo. Unsupported names areignored rather than rejected, so a generated client sending a relation this
version does not have still gets its response.
?with=subfleetsnow resolves. The public name and the relation differ only incase, and because PHP method calls are case-insensitive
load('subfleets')succeeded and stored a second copy of the relation under the mis-cased key —
which
whenLoaded('subFleets')could not see.subfleets.driversandsubfleets.vehicleswork explicitly; the implicit nesting the released contracthad (
with[]=subfleets&with[]=drivers) is preserved. Nothing under a subfleetre-opens the tree, so an expansion cannot recurse.
Allowlists: Fleet —
service_area,zone,vendor,parent_fleet,photo,subfleets,drivers,vehicles. Vehicle —driver,vendor,category,warranty,photo,devices. Driver —vehicle,vendor,current_job,fleets.userandcompanyare deliberately not expandable.Fleet membership uniqueness is now the database's job
firstOrNewis idempotent only against itself: two requests can both findnothing and both insert. A retrying importer is exactly the caller that produces
that.
fleet_vehicles(fleet_uuid, vehicle_uuid)andfleet_drivers(fleet_uuid, driver_uuid).tombstone, the lowest id wins among equals, and a pair whose rows are all
soft-deleted keeps one restorable row (removing them all would turn a later
re-assignment into a new row and lose the membership's history). Only
redundant pivot rows are deleted; no fleet, vehicle or driver is touched. A
second run is a no-op. Rows with a NULL side are left alone — orphaned data,
and NULLs never collide in a unique index anyway.
provider-transaction keys where a tombstone frees the key through a generated
column. Here a removed membership must keep its key so re-assignment restores
that row instead of inserting a second one.
UniqueConstraintViolationException,adopts the winner's row (restoring it if it was a tombstone) and returns the
normal successful response. A violation on any other key is rethrown — a real
failure must not be reported as a successful assignment.
internal_idandpublic_idExact on the public API, partial in the console, selected by the resolved route:
?internal_id=VEH-10v1/vehiclesVEH-10onlyint/v1/fleet-ops/vehiclesVEH-10,VEH-100,VEH-101An importer asking whether
VEH-10exists must not be told yes becauseVEH-100does — that produces a duplicate on every rerun. The console's searchbox must keep finding it. An unknown request context defaults to exact, the safe
half of the pair. VIN and plate number are untouched and still match on a
prefix.
public_idfollows the same split.Driver write/read parity
timezonenow persists. It is accepted, documented and answered200—and was dropped, because the update copied only name, email and phone, and the
driver record has no timezone column. It belongs to the linked user.
the driver's own by uuid — so resending an unchanged address or number still
succeeds while another user's returns
422. Update previously skipped thecheck entirely, so two drivers could end up sharing an identity that password
reset matches on.
passwordis still not accepted on update; the dedicated endpoints areunchanged. Email and phone remain optional, and a driver with neither is still
creatable.
Vehicle write/read parity
All 90 safe writable fields are accepted, persisted and returned. Proved by a
database-backed round trip through the real controller: create → retrieve →
update → query, comparing every field sent against every field returned, with
explicit allowance for the documented transformations:
status: "active"availablelatitude+longitudelocation; no duplicate pair at the top levelpurchased_at,lease_expires_at,loan_first_payment<name>_idPartial updates leave omitted fields alone.
fuel_card_numberwas alreadyreturned and is now in the Postman object definition.
photo_idis returned;photo_urlremains the convenience URL andavatar_urlis documentedseparately as a display asset rather than a file reference.
vin_data,telematicsandslugstay read-only.Also fixed
assignedOrdersCount()andcurrentOrderReference()ran a query on everypublic Vehicle and Driver response and discarded the result, because
when()evaluates a plain value argument eagerly. Both are closures now, so the public
path no longer pays for data it does not return.
SDK and Navigator compatibility
Neither repository was modified. Both were verified by replaying base-branch and
new response shapes through the built SDK from the untouched checkout:
SDK COMPATIBILITY: PASS— 35 checks. Every base property still resolves tothe same type and value; new
*_idproperties are readable; an expandedobject and its identifier agree.
NAVIGATOR CONTRACT COMPATIBILITY: PASS— 15 checks over every driverattribute path found in Navigator's source, including the socket channel
interpolation and the chat participant string comparison.
Navigator's own jest suite covers permissions and UI, not the API shape, so it
is not evidence either way. Native/device validation has not been run.
Tests
Added:
VehiclePublicContractTest(full-field round trip, status/coordinatetransformations, additive relationships, internal counters),
PublicIdentifierFilterTest(exact vs partial, tenant isolation, VIN/plateregression, the request's company-scoped user lookup),
FleetMembershipMigrationTest(cleanup policy, idempotence, orphaned rows).Extended: fleet/vehicle/driver contract tests,
FleetResourceTestandFleetHierarchyResourceTest(now behavioural rather than source-textassertions),
FleetMembershipTest(index refusal, both race arms),ControllerFilterContractsTest,RequestContractsTest.Validation
php-cs-fixerclean on every changed file. PHPStan atlevel: maxreports zeroerrors for all new files; the project-wide count is a pre-existing baseline.
PHP CI and Ember.js CI both green.
Cross-repository contract run
The contract job was temporarily pinned to the exact commit on
fleetbase/postman#59 (
postman-ref), so this PR was validated against thecollection that documents it rather than against a
maincollection predatingevery endpoint added here. The pin has been reverted; the branch is not
pinned to an unmerged ref.
Run 33953882575
— fleetops
a249e7df× postman91e72fe:(Against
postman@mainthe same job runs 218 requests / 221 assertions — itcannot exercise anything this PR adds.)
It found two real defects that 437 green test files did not:
GET /v1/vehicles/{id}?with[]=…answered 500. Both retrieve actions tookthe request as an optional parameter, and Laravel's controller dispatcher
skips resolving a type-hinted dependency that carries a default — so it
arrived
null, the allowlist never ran, and an unknown relation name reachedEloquent. Fixed in
9f014f9, with a test that drives the realfind().pm.request.body.raw, which re-resolves{{$randomEmail}}on every read.Fixed on the Postman branch.
Related
Contract documentation and tests: fleetbase/postman#59
Shipping in: #312 (
release: v0.6.62)Confirmation
No customer-specific code, fixtures or data. No secrets. No Core API, SDK or
Navigator changes. Nothing merged.