release: v0.6.61 - #303
Open
roncodes wants to merge 12 commits into
Open
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.
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 9829 +14
===========================================
Files 523 523
Lines 37888 37918 +30
===========================================
+ Hits 37888 37918 +30
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
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.
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.