e6e41fe5c6e8b2cf0a1b38ab54394cfaeb434889
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
e6e41fe5c6 |
Wrap the eight invitation-key downloads
`GOALS.md` listed these as "eight routes that take no auth token, so they do
not fit the authenticated request path and need a different entry point."
That was wrong on every count, and wrong in the way goal 6 exists to catch:
the premise came from the route names rather than the route file.
All eight sit inside `routes/api.php`'s `api/v1` group, under its
`token_auth` middleware, and the controllers' own OpenAPI blocks declare
`X-API-TOKEN`. So they needed no new entry point — just a macro, alongside
`entity_action_methods!` and for the same four entities. What the item had
actually described is the client-portal family in `routes/client.php`, which
is genuinely token-less, is not under `/api/v1`, and stays unwrapped for
that reason; it is now the gap entry in `COVERAGE.md`.
`download_{invoice,quote,credit,purchase_order}_by_invitation_key` returns
the PDF and `download_e_*_by_invitation_key` the XML e-document. These take
a contact's invitation key rather than a record id, so the server renders in
that contact's locale — a client with contacts in two languages gets a
different document per key, which the id-based `download_*` cannot express.
Two things here are easy to get wrong, so both are pinned by tests. The
entity segment is singular (`invoice/{key}/download`), the only route family
in the API that departs from the plural CRUD path. And an unknown key is a
400 carrying `no record found`: the controllers return that directly instead
of raising `ModelNotFoundException`, so the status matches the rule for a
missing record but the message does not.
The smoke test's read tier now downloads an invoice by invitation key,
taking the key from the `invitations` array the list route already returns.
It is there to re-check the token claim against a real server rather than
against a second reading of the source. The `download_e_*` four are left
out: they render from e-invoice settings a test instance need not have.
Also in here, unrelated: a rustfmt fix to a `match` arm in
`examples/smoke_test.rs` that was already off-format on main, so
`cargo fmt --check` — and with it CI's `test` job — was failing before this
change. It could not be split out cleanly, being in a file this change also
touches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
f616da08db |
Wrap the per-resource extras, activities, email history and previews
Forty-one hand-written endpoints, taking the crate from 284 methods to 325: the per-resource extras (webhook retry, task sorting, default design, subscription steps, the company gateway and bank integration actions, and the client extras), the password-protected destructive actions (client and company purges, client and vendor merges, user invite and purge), the activity list, feed, per-entity feed and note, email history, and the six preview routes. The password-protected actions send `X-API-PASSWORD`, falling back to `X-API-PASSWORD-BASE64` when the password holds a control character, which cannot go in a header value. None of the destructive actions are replayed on a 5xx, and neither are the notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
9e7d81307d |
Add a smoke test for real servers, and show why transport errors happen
The suite runs against mock servers written from the upstream source, so
it cannot catch a place where a real server disagrees with that reading.
`examples/smoke_test.rs` can: it lists every resource, checks the
pagination walk against a single oversized request, probes the per_page
cap, and exercises statics, search and health_check. The upload wire
format (POST plus `_method=PUT`) was read from the framework source
rather than observed, so its write tier is the real verification of it.
It prints PASS/FAIL per check and never the contents of a record.
Writes are guarded. `IN_SMOKE_WRITE_HOST` must equal the host in
`IN_BASE_URL`, so a write can only reach a server that was named
explicitly. `IN_BASE_URL` has no default, unlike the other example,
because defaulting to the hosted app is the wrong failure for a tool
that can write. The write tier leaves a gap in the client number
sequence and a soft-deleted row even after cleanup, so it is documented
as disposable-instance only.
Running it against a dead port exposed a diagnosability gap:
`Error::Http` printed "error sending request for url (...)" and nothing
else. reqwest's Display stops at the outermost layer, and the reason
("Connection refused", a DNS failure, a TLS error) lives in `source()`.
The error now renders its whole cause chain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
e4f2373b53 |
Add multipart document uploads for the 16 resources that have the route
First item on the GOALS.md roadmap. `upload_{x}(id, files, is_public)`
attaches files through `/{resource}/{id}/upload` and returns the
refreshed record. Verified against upstream that all 16 controllers
answer with `itemResponse($x->fresh())`.
Three details that look wrong but are not, each checked in the upstream
source rather than taken from the published spec:
- The request is a POST carrying `_method=PUT`. The route is registered
PUT, but PHP does not parse a multipart body on a real PUT, so the
files never arrive. Laravel's kernel calls
`enableHttpMethodParameterOverride()`, turning the spoofed method back
into a PUT before routing. The spec documents POST with `_method=POST`,
which would 405 against these routes.
- `is_public` is sent as 1/0. Laravel's `boolean` rule accepts only
true, false, 0, 1, '0' and '1' under strict comparison, so "true"
fails validation.
- Uploads are never retried. An upload adds a document, so it is not
idempotent whatever its verb, and a replay after a lost response would
attach the file twice. This falls out of reqwest's multipart body being
un-cloneable, which nothing in the types enforces, so a test pins it
for 500, 503 and 429.
An upload with no files, a nameless file or an unparseable MIME type is
refused locally (`Error::InvalidUpload`), since the server answers an
empty upload with a success that changed nothing.
Corrects two earlier claims:
- GOALS.md and COVERAGE.md said uploads would make `create_document`
possible. They do not: `DocumentController::store` is an empty stub,
and an upload is the only way a document is created. There is no
`create_document`.
- The documented method count (265) had been stale since the typed
`statics`/`search`/`health_check` commit, which added three `_as`
methods without updating it. Recounted from the source: 284 = 210 +
18 + 12 + 16 uploads + 28 hand-written.
Also fixes a README example, `let statics: Value = client.statics()`,
that stopped compiling when `statics` lost its type parameter.
`edocument/upload` is left out: it imports an expense from an
e-document rather than attaching a file, so it moves to the
import/export roadmap item.
Enabling `multipart` adds mime, mime_guess and unicase; MSRV stays 1.88,
re-verified by running the suite on it. 101 tests, up from 91.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
8ff947455e |
Harden error reporting, retry bounds, and credential handling
Second hardening pass, over the code added in
|
||
|
|
23c660d2f9 |
Add retry with backoff, and narrow the untyped responses
Retry: - `RetryPolicy` (3 retries, full-jitter exponential backoff) applied in a new `execute` dispatch chokepoint, configurable via `with_retry_policy`. `Retry-After` is honoured, and one longer than `max_delay` is surfaced rather than slept through. - What may be replayed depends on the method, not just the status. A 429 means the server refused the request without processing it, so any method is safe. A 5xx or timeout gives no such guarantee, so only idempotent methods are retried: this is a billing API, and replaying a POST the server had already applied would duplicate an invoice, payment, or email. Response types: - `statics`, `search`, and `health_check` return `Statics`, `SearchResults`, and `HealthCheck` instead of being generic. `HealthCheck` covers both deployments — hosted returns a bare message rather than a report — and `SearchResults` covers both the Elasticsearch and non-Elasticsearch bucket sets. - Each keeps the `extra` convention, and `statics_as` / `search_as` / `health_check_as` remain for pinning shapes left untyped. Nested config blobs (gateways, templates, extensions) stay `Value`, same reasoning as `Company::settings`. - `send_email` previously decoded the response as a bare body, but the controller returns `itemResponse(...)` — a `data`-wrapped entity. It now unwraps, so `T` is the entity. Still generic, since the endpoint serves invoices, quotes, credits, and purchase orders. - `chart` stays generic on purpose: the eleven charts share no response shape. - Adds `id_as_string`, since `/statics` returns `payment_types.id` as a string and the other tables return numbers. Adds CHANGELOG.md (Keep a Changelog), and a CLAUDE.md note to keep it current. 17 new tests, 43 in client_requests.rs total. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
9976249210 |
Wrap all 34 resource groups, then harden the request plumbing
Expands coverage to every `Route::resource` group in the v5 API plus `users` (265 methods), and fixes several defects in the shared request path that the new breadth made more exposed. Coverage: - 25 new models, each keeping the `extra` flatten map and the `null_to_default` convention so unmodeled fields round-trip. - `crud_methods!` split out of `resource_methods!` for the three groups with no `/bulk` route, where a generated `bulk_*` would only 405. - COVERAGE.md added as the detailed inventory, verified against `routes/api.php` on v5-stable. Fixes, each confirmed against upstream source or an observed request: - `list_all_*` silently truncated. `BaseController` caps page size at `min(abs(per_page), 5000)`, so the "short page means last page" rule stopped early and returned a partial list as if complete. Termination now prefers `meta.pagination.total_pages` (decoded but previously discarded), and `per_page` is clamped so the fallback stays sound. - `per_page(0)` paged forever: the check `got < 0` has no solution. It now falls back to the default size, with a MAX_PAGES backstop that raises `Error::PaginationLimit` rather than quietly truncating. - Ids were interpolated into paths raw, so a `/` or `?` in an id retargeted the request. All sites now use `seg(id)?`. Escaping alone is insufficient: the WHATWG rules `url` implements decode `%2E` to `.` before resolving dot segments, so an escaped `..` still collapses the path. Empty and dot-only ids are rejected. - No request timeout. reqwest applies none, so a stalled connection hung the caller indefinitely. Adds a 60s default and `with_timeout`. - Base URL is validated at construction; a missing scheme used to 404 on every call instead of failing once. Adds tests/client_requests.rs: 26 tests over the request plumbing against a wiremock loopback server, including regressions for the truncation and non-termination bugs. No network or credentials needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |