diff --git a/CHANGELOG.md b/CHANGELOG.md index a895370..170cb3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Invitation-key downloads, eight methods: + `download_invoice_by_invitation_key` and + `download_e_invoice_by_invitation_key`, plus the same pair for quotes, + credits, and purchase orders. These take the per-contact invitation key + from a client-portal link rather than a record id, and return the PDF or + the XML e-document as bytes. Prefer them over the id-based `download_*` + when the document is for a particular contact: the server renders in that + contact's locale, so a client with contacts in two languages gets a + different document per key. An unknown key is a `400` carrying `no record + found`. `COVERAGE.md` previously listed these as out of scope on the + grounds that they need no auth token; that was a misreading — they are + ordinary authenticated `/api/v1` routes. - Document uploads: `upload_client`, `upload_invoice`, `upload_expense`, and thirteen more, one for each resource with an `/{id}/upload` route. They take a `Vec` and an optional `is_public`, and return the diff --git a/CLAUDE.md b/CLAUDE.md index f5f4107..cc4cb1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -157,6 +157,10 @@ Beyond "does it decode", the read tier fetches the same rows raw and fails if a typed row lost a key (`keys kept: `, names only, never values), and the write tier creates a purchase order, location, webhook and recurring invoice from bare `Default` models, since those are what broke. +The read tier also downloads an invoice by invitation key, which is there to +re-check the claim that those routes are token-authenticated; the matching +`download_e_*` is left out, since it renders from e-invoice settings a test +instance need not have. With `IN_API_PASSWORD` set as well, a password tier merges two throwaway clients and purges the survivor (a real, permanent delete); it was run against a disposable 5.13.50 instance on 2026-10-24 and all four checks @@ -241,6 +245,17 @@ changes, doc tweaks) does not need an entry. elsewhere without checking `routes/api.php`. In particular the official PHP SDK puts bulk `download` on *every* resource, which is wrong; only PDF-rendering entities support it. +- `invitation_download_methods!` generates `download_{x}_by_invitation_key` + and `download_e_{x}_by_invitation_key` for the same four resources. Two + traps, both pinned by tests: the entity segment is **singular** + (`invoice/{key}/download`, the only route family in the API that is), and + the routes are **authenticated** despite being the link a client follows — + they sit in `routes/api.php`'s `api/v1` group under `token_auth`. The + genuinely token-less invitation downloads are the separate client-portal + routes in `routes/client.php`, which are not under `/api/v1` and are not + wrapped. `COVERAGE.md` used to list the `/api/v1` eight as unwrapped for + that reason, having conflated the two families; don't reintroduce the + claim. - `upload_methods!` generates `upload_{x}` for the 16 resources with a `/{id}/upload` route; documents can only be created this way, because `DocumentController::store` is an empty stub upstream (hence no @@ -322,18 +337,20 @@ changes, doc tweaks) does not need an entry. ## Coverage **All 34 of the API's `Route::resource` groups are wrapped**, plus `users` -(which isn't one). 325 methods: 30 resources with CRUD + bulk, 3 with CRUD -only, 4 with entity actions, 16 with document uploads, and 69 hand-written. +(which isn't one). 333 methods: 30 resources with CRUD + bulk, 3 with CRUD +only, 4 with entity actions, 4 with invitation-key downloads, 16 with +document uploads, and 69 hand-written. Plus the non-CRUD surface: documents, entity actions and PDF downloads for -the four action-capable resources, `report`, `chart`, `search`, `statics`, -`send_email`, `refund_payment`, `ping`, and `health_check`. +the four action-capable resources, their invitation-key PDF and e-document +downloads, `report`, `chart`, `search`, `statics`, `send_email`, +`refund_payment`, `ping`, and `health_check`. **`COVERAGE.md` is the detailed inventory** — every wrapped endpoint, every gap, and the reason for each gap. The main remaining gaps are the -integration and auth routes, import/export, the public invitation-key -downloads (which take no auth token, so they don't fit the client's -request path), and a few company and user extras. +integration and auth routes, import/export, the client-portal routes (which +are not under `/api/v1`, so they don't fit the client's `base_url`), and a +few company and user extras. Keep `COVERAGE.md` current when adding or removing an endpoint: it names exact counts and route groups, so it goes stale quietly. The three coverage diff --git a/COVERAGE.md b/COVERAGE.md index 38fd845..aacd20b 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -18,7 +18,7 @@ declares 34 `Route::resource` groups and 274 explicit route declarations > exists or what it returns. **All 34 `Route::resource` groups are now wrapped**, plus `users` (which is -not a resource group). 325 methods in total. +not a resource group). 333 methods in total. ## Resources with full CRUD + bulk @@ -127,6 +127,47 @@ same reason: the response isn't JSON. Convenience wrappers: `mark_invoice_sent` and `mark_invoice_paid`. +## Invitation-key downloads + +The same four resources also answer two downloads keyed by **invitation key** +rather than record id — the per-contact token that appears in a client-portal +link. Eight routes, all wrapped: + +| Route | Method | Returns | +| --- | --- | --- | +| `GET /invoice/{key}/download` | `download_invoice_by_invitation_key` | `application/pdf` | +| `GET /invoice/{key}/download_e_invoice` | `download_e_invoice_by_invitation_key` | `application/xml` | +| `GET /quote/{key}/download` | `download_quote_by_invitation_key` | `application/pdf` | +| `GET /quote/{key}/download_e_quote` | `download_e_quote_by_invitation_key` | `application/xml` | +| `GET /credit/{key}/download` | `download_credit_by_invitation_key` | `application/pdf` | +| `GET /credit/{key}/download_e_credit` | `download_e_credit_by_invitation_key` | `application/xml` | +| `GET /purchase_order/{key}/download` | `download_purchase_order_by_invitation_key` | `application/pdf` | +| `GET /purchase_order/{key}/download_e_purchase_order` | `download_e_purchase_order_by_invitation_key` | `application/xml` | + +Two things about these are easy to get wrong, and both are pinned by tests. +The entity segment is **singular** — `invoice/…`, not `invoices/…` — which +is the only place in the API that departs from the plural CRUD path. And +despite being the download a *client* follows, they are **authenticated**: +the routes sit inside `routes/api.php`'s `api/v1` group under its +`token_auth` middleware, so they need the API token like any other call. + +This file previously listed them as unwrapped on the grounds that they take +no auth token. That was wrong — it conflated them with the client-portal +routes in `routes/client.php` (`/client/invoice/{key}/download_pdf` and +friends), which are a separate route family, are not under `/api/v1`, and +remain unwrapped for that reason. + +The smoke test's read tier exercises the invoice PDF, taking a key from the +`invitations` array the list route already returns. The `download_e_*` four +are not exercised: they render from the company's e-invoice settings, which a +test instance need not have configured. + +An unknown or deleted key is a `400` carrying `no record found`. The +controllers return that directly rather than raising +`ModelNotFoundException`, so the message differs from the `No query results +for model` text a missing record produces elsewhere, but the status is the +same 400. + ## Document uploads `PUT /{resource}/{id}/upload` exists for 16 resources, each getting @@ -346,12 +387,13 @@ check `routes/api.php`). `/migration/start` - **Invoice extras** — `delivery_note`, `payment_schedule`, `update_reminders` -- **Public invitation-key downloads** — `GET /{entity}/{invitation_key}/download` - for `invoice`, `quote`, `credit`, and `purchase_order` (note the singular - path segment), plus the matching e-invoice variants `download_e_invoice`, - `download_e_quote`, `download_e_credit`, - `download_e_purchase_order`. These are invitation-key based and take no - auth token, so they don't fit the client's authenticated request path. +- **Client-portal invitation downloads** — the `routes/client.php` family: + `/client/invoice/{key}/download_pdf`, the `download_e_*` variants for + quotes and credits, `/client/{entity}/{key}/download`, and + `/client/documents/{hash}/hashed`. These are not under `/api/v1`, so they + don't fit a `base_url` that ends there. Note these are *not* the + invitation-key downloads documented above, which are authenticated + `/api/v1` routes and are wrapped. - **Integrations** — Stripe, QuickBooks, Apple in-app purchase, PostMark, einvoice (17 routes), calendar_connection, self-update, templates, hooks, Yodlee (`/yodlee/*`), Nordigen (`/nordigen/institutions`) @@ -406,7 +448,8 @@ top of a shared `BaseEntity` offering `all`, `get`, `create`, `update`, `recurring_quotes`, `recurring_expenses`, `bank_transaction_rules`, `company_gateways`, `designs`, `locations`, `tags`, `task_schedulers`, `tokens`, `system_logs`, documents, per-entity actions (`mark_sent`, -`mark_paid`, `clone_to_*`, …), per-entity PDF download, reports, charts, +`mark_paid`, `clone_to_*`, …), per-entity PDF download by id and by +invitation key, the XML e-documents, reports, charts, search, `send_email`, `refund_payment`, `ping`, `health_check`, automatic pagination (`list_all_*`), and `X-API-PASSWORD` support for the password-protected routes. diff --git a/GOALS.md b/GOALS.md index 4bdf701..e1e41dc 100644 --- a/GOALS.md +++ b/GOALS.md @@ -159,22 +159,33 @@ suggests. That is the case for keeping the real-server tier. **Done:** the password-protected actions — client purge and merge, vendor merge, company purge, user invite and purge, and the blank-user template (8 -methods). They are verified against the upstream controllers and mocks only; -the smoke test has a password tier for the first real-server run -(`IN_API_PASSWORD`), which has not happened yet. The claim here that -`with_api_password` already made them reachable was not quite true: a -password with a control character was refused locally. +methods). The smoke test's password tier (`IN_API_PASSWORD`) has since been +run against a disposable 5.13.50 instance, on 2026-10-24, with all four +checks passing — but only after its own post-purge assertion was fixed, which +had checked for a 404 and passed solely because it had never been run. The +claim here that `with_api_password` already made them reachable was not quite +true: a password with a control character was refused locally. **Done:** activities, email history and previews (14 methods, two new models). The email history stays raw JSON, since no instance here had sent mail to take a shape from; promoting it to a model needs a sample. Several previews are unverified against a real server (see `COVERAGE.md`). +**Done:** the invitation-key downloads — the PDF and the XML e-document for +invoices, quotes, credits and purchase orders, keyed by a contact's +invitation key (8 methods). This item used to read "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 reading the route *names* +rather than the route file. All eight sit inside `routes/api.php`'s `api/v1` +group under its `token_auth` middleware — the controllers' own OpenAPI blocks +declare `X-API-TOKEN` — so they needed no new entry point and were two dozen +lines of macro. 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. + Still to do: -5. **Public invitation-key downloads** — eight routes that take no auth - token, so they do not fit the authenticated request path and need a - different entry point. 6. **Import/export and the integrations** — `/import`, `/export`, `edocument/upload`, Stripe, QuickBooks, einvoice (17 routes), Yodlee, Nordigen, calendar. The largest block and the least SDK-shaped; worth diff --git a/README.md b/README.md index add3dff..fe5295f 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ IN_API_TOKEN=... cargo run --example list_clients ## Coverage -Every one of the API's 34 resource groups, plus users — 325 methods. Each +Every one of the API's 34 resource groups, plus users — 333 methods. Each resource has `list`, `list_all`, `get`, `create`, `update`, `delete`, and (where the API offers one) `bulk`: @@ -114,8 +114,9 @@ resource has `list`, `list_all`, `get`, `create`, `update`, `delete`, and | Admin | Companies, Users, Group Settings, Tokens, System Logs | | Settings | Tax Rates, Payment Terms, Webhooks, Designs, Locations, Tags | -Plus documents, entity actions and PDF downloads, reports, charts, search, -statics, email sending, refunds, and health checks. +Plus documents, entity actions and PDF downloads — by record id, or by +invitation key for the PDF and XML a given contact receives — reports, +charts, search, statics, email sending, refunds, and health checks. **See [COVERAGE.md](COVERAGE.md) for the full endpoint-by-endpoint inventory**, including everything that is deliberately *not* wrapped and why. diff --git a/examples/smoke_test.rs b/examples/smoke_test.rs index d13d577..3b97ccc 100644 --- a/examples/smoke_test.rs +++ b/examples/smoke_test.rs @@ -332,6 +332,42 @@ async fn read_checks(client: &InvoiceNinjaClient, raw: &Raw, report: &mut Report Err(e) => report.fail("live_preview", describe(&e)), } + // The invitation-key download, on the singular `invoice/{key}/download` + // route. Worth a real-server check for one specific reason: this was + // listed as unwrappable for months on the belief that it takes no auth + // token, and the route file says otherwise. If that reading is still + // wrong, the token we send is either ignored or rejected, and this is + // where it shows. The key comes from the invoice's own `invitations` + // array, which the list route returns without an `include`. + // + // Only the PDF, not the e-document: `download_e_invoice` renders from + // the company's e-invoice settings, so an instance with none configured + // would fail here for a reason that is not the client's fault. + match client.list_invoices(&small).await { + Ok(rows) => match rows.iter().find_map(|i| { + i.extra + .get("invitations")? + .as_array()? + .first()? + .get("key")? + .as_str() + }) { + Some(key) => match client.download_invoice_by_invitation_key(key).await { + Ok(pdf) => report.check( + "download_invoice_by_invitation_key", + pdf.starts_with(b"%PDF"), + format!("{} byte(s), starts with the PDF magic", pdf.len()), + ), + Err(e) => report.fail("download_invoice_by_invitation_key", describe(&e)), + }, + None => report.pass( + "download_invoice_by_invitation_key", + "skipped: no invoice with an invitation on this instance", + ), + }, + Err(e) => report.fail("download_invoice_by_invitation_key", describe(&e)), + } + list!(report, "list_clients", client.list_clients(&small)); list!(report, "list_invoices", client.list_invoices(&small)); list!(report, "list_quotes", client.list_quotes(&small)); @@ -817,11 +853,12 @@ async fn password_checks(client: &InvoiceNinjaClient, report: &mut Report, passw // ModelNotFoundException to a 400 and keeps 404 for a missing route. The // message is checked too, so a 400 raised for some other reason fails. match client.get_client(&keep).await { - Err(Error::Api { status, message, .. }) - if status.as_u16() == 400 && message.contains("No query results") => - { - report.pass("get_client after purge", "400 no query results, the row is gone") - } + Err(Error::Api { + status, message, .. + }) if status.as_u16() == 400 && message.contains("No query results") => report.pass( + "get_client after purge", + "400 no query results, the row is gone", + ), Err(e) => report.fail("get_client after purge", describe(&e)), Ok(_) => report.fail("get_client after purge", "still readable after a purge"), } diff --git a/src/client.rs b/src/client.rs index aea334a..86346e1 100644 --- a/src/client.rs +++ b/src/client.rs @@ -767,6 +767,52 @@ macro_rules! entity_action_methods { }; } +/// Generates the two invitation-key downloads for one entity. +/// +/// A different route family from [`entity_action_methods`], in two ways that +/// are easy to trip over. The path segment is **singular** — `invoice/…`, +/// not `invoices/…`, the only place in the API that does this. And the value +/// in it is an *invitation key*, the per-contact token that appears in a +/// client-portal link, not a record id; passing a record id gets the 400 +/// below, since no invitation has that key. +/// +/// They are still authenticated. The routes sit inside `routes/api.php`'s +/// `api/v1` group, under its `token_auth` middleware, so they go through the +/// client's ordinary request path like any other call. The invitation +/// downloads that genuinely take no token are the separate client-portal +/// routes in `routes/client.php`, which are not wrapped — they are not under +/// `/api/v1`, so they do not fit a `base_url` that ends there. +macro_rules! invitation_download_methods { + ($download:ident, $download_e:ident, $path:literal, $e_action:literal) => { + /// Downloads the rendered PDF for the record an invitation key + /// points at. + /// + /// This renders for one *contact* rather than one record, which is + /// the reason to prefer it over the id-based `download_*` method: + /// upstream sets the locale from the contact the invitation belongs + /// to, so a client with two contacts in different languages gets a + /// different document per key. + /// + /// An unknown or already-deleted key is a `400` carrying + /// `no record found`, not a `404`. + pub async fn $download(&self, invitation_key: &str) -> Result> { + self.get_raw(&format!("{}/{}/download", $path, seg(invitation_key)?)) + .await + } + + /// Downloads the XML e-document for the record an invitation key + /// points at. + /// + /// The bytes are `application/xml`, not a PDF; which standard they + /// follow (Factur-X, ZUGFeRD, Peppol, …) depends on the company's + /// e-invoice settings. Errors are as above. + pub async fn $download_e(&self, invitation_key: &str) -> Result> { + self.get_raw(&format!("{}/{}/{}", $path, seg(invitation_key)?, $e_action)) + .await + } + }; +} + impl InvoiceNinjaClient { resource_methods!( list_clients, @@ -1051,6 +1097,34 @@ impl InvoiceNinjaClient { "purchase_orders", PurchaseOrder ); + + invitation_download_methods!( + download_invoice_by_invitation_key, + download_e_invoice_by_invitation_key, + "invoice", + "download_e_invoice" + ); + + invitation_download_methods!( + download_quote_by_invitation_key, + download_e_quote_by_invitation_key, + "quote", + "download_e_quote" + ); + + invitation_download_methods!( + download_credit_by_invitation_key, + download_e_credit_by_invitation_key, + "credit", + "download_e_credit" + ); + + invitation_download_methods!( + download_purchase_order_by_invitation_key, + download_e_purchase_order_by_invitation_key, + "purchase_order", + "download_e_purchase_order" + ); } /// Documents, reports, and the remaining endpoints that do not follow the diff --git a/tests/client_requests.rs b/tests/client_requests.rs index 00fe225..829a88b 100644 --- a/tests/client_requests.rs +++ b/tests/client_requests.rs @@ -2221,3 +2221,147 @@ async fn notes_are_not_replayed_on_5xx() { .expect_err("500"); assert_eq!(server.received_requests().await.unwrap().len(), 1); } + +// --- invitation-key downloads ------------------------------------------- + +/// Mounts one GET route answering 200 with raw bytes. Expects exactly one +/// request, so a call that built a different path fails when the server drops. +async fn raw_route_server(route: &str, body: &'static [u8]) -> MockServer { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path(route)) + .respond_with(ResponseTemplate::new(200).set_body_bytes(body.to_vec())) + .expect(1) + .mount(&server) + .await; + server +} + +/// One test per invitation-key download. The route each mounts is the one +/// `routes/api.php` registers — note the singular entity segment, which is +/// what these would get wrong if the plural CRUD path were reused. +macro_rules! invitation_download_test { + ($name:ident, $route:literal, $body:literal, |$c:ident| $call:expr) => { + #[tokio::test] + async fn $name() { + let server = raw_route_server($route, $body).await; + let $c = client(&server); + let got = $call.await.expect("the download should succeed"); + assert_eq!(got, $body); + } + }; +} + +invitation_download_test!( + download_invoice_by_invitation_key_hits_the_singular_route, + "/invoice/key1/download", + b"%PDF-1.4", + |c| c.download_invoice_by_invitation_key("key1") +); + +invitation_download_test!( + download_e_invoice_by_invitation_key_hits_the_singular_route, + "/invoice/key1/download_e_invoice", + b"", + |c| c.download_e_invoice_by_invitation_key("key1") +); + +invitation_download_test!( + download_quote_by_invitation_key_hits_the_singular_route, + "/quote/key1/download", + b"%PDF-1.4", + |c| c.download_quote_by_invitation_key("key1") +); + +invitation_download_test!( + download_e_quote_by_invitation_key_hits_the_singular_route, + "/quote/key1/download_e_quote", + b"", + |c| c.download_e_quote_by_invitation_key("key1") +); + +invitation_download_test!( + download_credit_by_invitation_key_hits_the_singular_route, + "/credit/key1/download", + b"%PDF-1.4", + |c| c.download_credit_by_invitation_key("key1") +); + +invitation_download_test!( + download_e_credit_by_invitation_key_hits_the_singular_route, + "/credit/key1/download_e_credit", + b"", + |c| c.download_e_credit_by_invitation_key("key1") +); + +invitation_download_test!( + download_purchase_order_by_invitation_key_hits_the_singular_route, + "/purchase_order/key1/download", + b"%PDF-1.4", + |c| c.download_purchase_order_by_invitation_key("key1") +); + +invitation_download_test!( + download_e_purchase_order_by_invitation_key_hits_the_singular_route, + "/purchase_order/key1/download_e_purchase_order", + b"", + |c| c.download_e_purchase_order_by_invitation_key("key1") +); + +/// These are authenticated like any other call, which is the whole reason +/// they fit the client at all. A token-less variant would need a separate +/// entry point. +#[tokio::test] +async fn an_invitation_download_still_sends_the_api_token() { + let server = raw_route_server("/invoice/key1/download", b"%PDF-1.4").await; + client(&server) + .download_invoice_by_invitation_key("key1") + .await + .unwrap(); + + let headers = &server.received_requests().await.unwrap()[0].headers; + assert_eq!(headers["X-Api-Token"], "test-token"); +} + +/// An unknown key is a `400` carrying `no record found`, not a `404`: the +/// controllers return that directly rather than raising +/// `ModelNotFoundException`. +#[tokio::test] +async fn an_unknown_invitation_key_is_a_400_not_a_404() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/invoice/gone/download")) + .respond_with( + ResponseTemplate::new(400).set_body_json(json!({ "message": "no record found" })), + ) + .mount(&server) + .await; + + let err = client(&server) + .download_invoice_by_invitation_key("gone") + .await + .expect_err("an unknown key should fail"); + assert!( + matches!(&err, Error::Api { status, message, .. } + if status.as_u16() == 400 && message == "no record found"), + "{err:?}" + ); +} + +/// `invoice//download` is a different route, so an empty key must not reach +/// the server. +#[tokio::test] +async fn an_empty_invitation_key_is_refused_before_the_request() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(b"%PDF-1.4".to_vec())) + .mount(&server) + .await; + + let err = client(&server) + .download_invoice_by_invitation_key("") + .await + .expect_err("an empty key must not be sent"); + assert!(matches!(err, Error::InvalidPathSegment(_)), "got {err:?}"); + assert!(server.received_requests().await.unwrap().is_empty()); +}