release: v0.6.61 - #303
Merged
Merged
Conversation
…uence it The public order-config resource projected each activity down to code, status, details, color, complete, pod_method and require_pod. Those describe an activity but say nothing about the flow's shape, so what reaches an API consumer is an unordered set of activities with no way to put them in order. The stored flow is a directed graph, and the fields that express it were the ones being dropped: `activities` names the codes an activity can transition to, `sequence` orders activities reachable from the same parent, and `logic` gates availability. OrderConfig::nextActivity walks exactly these server-side, and the console's internal resource returns the flow whole, so the gap is only visible from the public API. The consequence is not cosmetic. A client rendering progress from array position marks an order complete whenever `completed` happens to be declared before the order's current activity — the default transport config lists `completed` fourth and `dispatched` last, so a freshly dispatched order shows as finished and offers no next step at all. These describe the configured workflow rather than internal state, so there is nothing here a consumer of the config should not already see. Transitions are normalised to a list of codes, since flows have been authored both as bare codes and as objects carrying one, and the three fields are always present — null or empty rather than absent — so a client can read the contract instead of feeling for it.
`odometer` is fillable on the Vehicle model and unrestricted by the request rules, but `VehicleController::vehicleInputFromRequest()` builds its input with `$request->only([...])` and that allowlist had no odometer in it. A caller sending one therefore received a 200 and a response body that looked correct, while the reading went nowhere. Recording mileage is the most common write a driver app makes against a vehicle — it is what a fuel report is checked against — and a silent no-op is the worst of the three possible answers. Accept it or reject it; reporting success for a discarded field leaves the client with no way to tell. Adds `odometer` and `odometer_unit` to the projection, and validates them rather than merely accepting them: the model casts odometer to an integer, so an unchecked string would have been stored as 0, which reads as a vehicle that has never moved rather than as an error. Two tests, using the probe already in the suite for protected helpers: one that the odometer survives the projection, and one that the projection is still an allowlist — adding a field must not turn it into "whatever the caller sent", so company_uuid and uuid must still be dropped.
…tly by none The base filter resolves a query parameter to a method of the same name and silently ignores anything it cannot match. `IssueFilter` and `FuelReportFilter` define `driver()` and nothing else, so a client narrowing a list with `driver_uuid` — the column's own name, and the name the rest of the payload uses — had its filter dropped on the floor. What came back was scoped only by `company_uuid`: every driver's issues, every driver's fuel reports, with a 200 and nothing in the response to say the request had not been narrowed. For a driver app that is a disclosure rather than a nuisance — a driver asking for their own fuel reports receives the whole company's, and neither side can tell from the exchange that anything went wrong. Adds `driverUuid`, `driverAssigned` and `vehicleUuid` as aliases on both filters, delegating to the existing implementations so uuid, public id and search fallback all behave identically. Tests assert the alias constrains the driver relation and routes a uuid the same way `driver()` does. Both fail without the change, with "Call to undefined method" — which is precisely the failure a caller could not see. Deliberately not addressed here: the general behaviour of ignoring unrecognised filter parameters. Rejecting them would be the stronger fix and a breaking one, since any client currently passing an unknown key would start receiving 400s. Worth deciding separately.
…wns it The two odometer tests were appended to VehicleControllerHelperContractsTest, whose probe extends Internal\v1\VehicleController. The helper they exercise, vehicleInputFromRequest, lives on Api\v1\VehicleController, so the reflection lookup errored with "method does not exist" and took PHP CI down. Move them to ApiVehicleControllerContractsTest, which already has a probe exposing that helper, and pin the new odometer validation rules in RequestContractsTest alongside the rest of the vehicle request contract.
…ters The coverage gate held at 100%; the new vehicleUuid() alias on FuelReportFilter and IssueFilter was the one statement in each file no test reached, dropping them to 44/45 and 50/51 and failing the gate. Exercise the alias through both routing branches — uuid and public id — so it is pinned the same way driverUuid() already is.
…fields The compact-resource test pinned the pre-change flow entry, so adding sequence, activities and logic to the projection made its whole-array comparison fail and took PHP CI down. Update the expectation to the shape the resource now emits. Also cover projectTransitions' non-array guard, which no fixture reached — it was the one statement standing between the new code and the 100% gate.
Inlining the layout switch into the actionButtons getter put `this.layout =` inside a computed property, which ember/no-side-effects rejects — four errors, and lint:js took the Ember CI build down. Put the assignment back behind the `changeLayout` action the getter had replaced, now persisting the choice to appCache, and let both menu items call it. Also strip the trailing whitespace and restore the final newlines that prettier flagged in card.js and this file.
A local symlink into a sibling checkout slipped into the previous commit and broke `pnpm install` in CI with ENOTDIR. .gitignore's `/node_modules/` has a trailing slash, so it matches the directory but not a symlink of that name.
A local symlink into a sibling checkout slipped into the previous commit and broke `pnpm install` in CI with ENOTDIR. .gitignore's `/node_modules/` has a trailing slash, so it matches the directory but not a symlink of that name.
The trailing slash in `/node_modules/` restricts the pattern to directories, so a symlink of that name is untracked-but-visible and easy to commit by accident. A committed one breaks `pnpm install` in CI with ENOTDIR.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #303 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 9815 9899 +84
============================================
Files 523 526 +3
Lines 37888 38163 +275
============================================
+ Hits 37888 38163 +275
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:
|
…graph feat(api): publish the order config flow's graph so consumers can sequence it
fix(api): scope issue and fuel-report lists by driver_uuid, not silently by none
fix(api): stop PUT /v1/vehicles/{id} silently discarding the odometer
…update
PUT/PATCH /v1/drivers/{id} passed `password` straight through to the driver's
user record. Anyone holding a driver's token — or an unlocked handset — could
set a new password without proving they knew the old one, and the account was
theirs. Nothing in the request rules or the controller asked for the current
password, and nothing logged that it had changed.
Changing a password is an authorisation decision, not an attribute update, so
it gets its own operations:
POST /v1/drivers/{id}/change-password current_password + password (confirmed)
POST /v1/drivers/forgot-password identity -> code by email or SMS
POST /v1/drivers/reset-password identity + code + password
`update()` no longer accepts `password` at all. Creating a driver still may:
setting a password on an account that does not exist yet proves nothing about
anyone.
Three things the implementation is deliberate about. A change revokes every
other token and hands the caller a fresh one in the same response, so the
password change ends other sessions without signing the driver out of the phone
in their hand. `forgot-password` answers identically whether or not the identity
exists, because a reset endpoint that 404s on an unknown number is a way to
enumerate a company's drivers. And a wrong code, an expired code and an unknown
identity all return the same message, so reset cannot be used as an oracle
either.
Follows the pattern already established for customers, and reuses the
VerificationCode mechanism drivers already use for OTP sign-in.
The regression test asserts a password sent to update() never reaches the user.
It could not be run locally: every test touching a controller in this package
fatals here with "Trait AuthorizesRequests not found", on a clean checkout too —
a missing illuminate/foundation in the local server_vendor, not this change.
A manifest is a driver's route: an order-agnostic sequence of stops that may
span several orders, or none the driver has ever seen as an order. The models
are complete — Manifest and ManifestStop carry sequence, per-leg distance and
duration, estimated and actual arrival, and markArrived/markCompleted/
markSkipped with their side effects — but every endpoint lived behind the
console's internal namespace. A driver could be assigned a route with no way to
read it, which is why the Navigator's Route tab has been a placeholder.
GET /v1/drivers/{id}/manifests the driver's routes, newest first
GET /v1/manifests/{id} the route, stops in sequence
PATCH /v1/manifest-stops/{id} arrive, complete or skip a stop
POST /v1/manifests/{id}/optimize re-sequence what is still to do
Deliberately narrow: a driver may read their own manifests, update the stops on
them, and reorder the ones they have not done. Creating, cancelling and deleting
a manifest is dispatch work and stays internal.
Status changes go through the model's transitions rather than being written as a
column, because arriving and completing carry side effects the column does not.
On optimize, and what it is not. This is the driver's optimise, not the
orchestrator's: the orchestrator allocates orders across a fleet and produces
manifests, while this reorders the stops on one manifest already assigned. It is
a nearest-neighbour heuristic over real road distances from OSRM — walk to the
closest remaining stop, then the closest from there. That is usually a large
improvement on an arbitrary order and is not guaranteed optimal, and the code
says so, because calling it anything stronger would be a claim the
implementation does not support. Completed and skipped stops keep their place: a
route already driven is not re-planned. Fewer than three pending stops returns
unchanged, since there is no ordering to find.
The stop resource carries its place inline rather than as an id — a route of
twenty stops should be one request, not twenty-one — and the manifest resource
omits stops unless they were loaded, so the list endpoint stays a list.
Three resource tests, run locally.
CI enforces 100% line coverage and the new controller was 0/77, because I had written no controller tests — every such test fatals on my machine, where illuminate/foundation is not installed. That was the wrong conclusion to stop at. The driver contracts test already stubs a missing Laravel class behind a class_exists guard; the same trick works for the three foundation traits and the response() helper, and does nothing at all in CI where the real ones exist. So these run locally now, and so does every other controller test in the package. Three changes to the controller itself, all for the better: The status check now runs before the transition rather than after it. An unrecognised status previously fell through a match with a null default and was then refused — the right answer by accident, and only because the default did nothing. The database lookups and the distance calculation moved behind protected seams. That is what lets the nearest-neighbour walk be exercised against known distances rather than a live OSRM. Fifteen tests: the not-found paths, each status transition, meta with and without, the filters applied only when asked, the default cap, the too-short route that returns unchanged, a stop whose place never resolved, and the walk itself ordering three stops nearest-first while a completed one keeps its place. The walk test failed at first and the fixture was wrong, not the code: the controller reads `place.location`, as a Point sits on the real model, and my fake exposed getLat()/getLng() at the top level. Both ends resolved to null, no distance was ever measured, and the stops kept their declared order. Worth recording because a fixture that silently produces null is the kind that makes a green test meaningless.
…pendency CI enforces 100% line coverage; the three new methods were 61 uncovered lines. Validation now follows the house style rather than $request->validate(). The customer password endpoints next door do explicit checks and return apiError, and $request->validate() is a foundation macro — matching the neighbours means the same shape of response and code that can be exercised without booting an application. Four small seams make the rest testable without changing any production behaviour: the identity lookup, the reset-code lookup, sending the code, and the password comparison. Each is a one-line method delegating to what it did inline, and the probe overrides them. Fifteen tests covering every branch: both missing inputs, a password too short, a mismatched confirmation, a missing driver, a driver with no user account, the wrong current password changing nothing, and the successful change ending other sessions while returning a fresh token. For reset: the same-answer-for-everything property is asserted directly — an unknown identity and a bad code return identical bodies, so neither can be used as an oracle — along with the code being spent and every session ending, because a reset is a recovery from losing control and nothing should keep working. The tests stub the foundation traits and response() behind existence guards, as the driver contracts test already does for one Laravel class. In CI, where the real ones are installed, the guards do nothing. Locally they are what makes a controller test runnable at all — I had previously concluded these could not be run here, which was wrong.
Covering the distance helper made its cost obvious. A nearest-neighbour walk compares every remaining stop at every step, so a twenty-stop route asks roughly four hundred distance questions — as OSRM lookups that is four hundred network round trips for one tap, on a handset with a driver waiting. Straight-line distance instead. Ordering by straight line and ordering by road rarely disagree about which of several stops is nearest, and where they do the result is still a valid route, just not the shortest one. The method is a heuristic either way, so the cheap version is the honest one, and the docblock no longer claims road distances. Also covers the geometry itself against known coordinates, and reaches the four database lookups the probe otherwise replaces — a lookup that swallowed a connection failure and returned null would make a broken database indistinguishable from a missing record, and both would answer 404.
The seams that made the endpoints testable were themselves uncovered, because the probe replaces every one of them. Each is a one-line delegation — to Eloquent, to the verification-code generator, to the hasher, to the application — and what is worth asserting is that they delegate at all: a password comparison that quietly returned true, or a code sender that quietly did nothing, would each turn a security control into decoration. Both delivery branches are exercised, an identity shaped like an email address and one shaped like a phone number, since they take different paths.
The coverage gate rounded to 100% while the file sat at 545/546, and codecov's patch check — which does not round — caught the one line: a `return` placed after a call that can throw, in the email branch of the reset-code sender. Nothing can reach it, so if/else replaces the early return. Also runs php-cs-fixer over the two files this branch touches. Its alignment pass reaches further, but reformatting files this change never went near does not belong in a security fix.
… default CARTO's basemaps.cartocdn.com raster tiles now require an API key (https://carto.com/basemaps/apikey/) and render 'API key required' watermarks on every Fleetbase instance using the Leaflet map. - Default all Leaflet maps to the keyless OpenStreetMap tile server - Add Leaflet tile provider URL settings (light + dark mode) to Fleet-Ops map settings, persisted per-company via the existing fleet-ops/settings/map endpoint - Resolve tile URLs through the map-settings service / a new leaflet-tile-url helper so a custom provider (including keyed CARTO URLs) applies across all console Leaflet maps - Sanitize custom tile URLs server-side (http/https only)
CI enforces 100% line coverage on server/src; the new sanitizeTileUrl branches (non-string input and valid custom URL passthrough) were the two uncovered statements failing the coverage gate.
…config-130982 feat(map): configurable Leaflet tile provider + keyless OpenStreetMap default
`password` is guarded on User, so `User::create()` dropped it without a word — while CreateDriverRequest has always accepted and validated one. A driver created through the public API could never sign in with the password their operator chose for them, and the first change-password call would be refused because the stored hash was never theirs. Set it after creation, where the model's mutator hashes it. The helper test harness only had the hashing contract, not an implementation, so the mutator had nothing to call; bind PHP's own hashing behind the contract so the password path is exercised.
changePassword resolved the account through Driver::getUser(), which goes through the `user` relation — and that relation selects a named subset of columns with `password` not among them. The comparison therefore ran against an empty string and refused every caller, whatever they typed. Verified against a live instance: the same call made in-process returns 200 where the endpoint returned 422. Load the record directly when a password is at stake.
…uires-current fix(security): a driver's password can no longer be set by a general update
feat(api): driver-facing manifests — read, run, and re-sequence a route
feat(drivers): add card view layout for Drivers Management
…lace-editing Fix editing unsaved geocoded places in order form
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
2 participants
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.
Release branch for v0.6.61. Versions bumped in
composer.json,package.jsonandextension.jsonviaflb version-bump --patch, andRELEASE.mdwritten for the tag body.Included
PUT /v1/vehicles/{id}silently discarding the odometerdriver_uuid, not silently by noneEach of the above is retargeted onto this branch and merges here before this PR merges to
main.