From 1bc9fb5c24e1e6cc7f1deaf70dcde176b57e84f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 14:54:22 +0000 Subject: [PATCH 1/2] fix(reports): generate report via GET, not POST (Bring returns 405 on POST) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring's /reports/api/generate/{customer}/{reportType} route only accepts GET, with the report-type filters passed as query parameters (the sibling list-available routes on the same base path are GET too). The v4 rewrite issued a POST with a JSON body, which Bring's gateway rejected with 405 Method Not Allowed and an empty body — surfacing downstream as: Bring API returned HTTP 405 (-: HTTP 405 (empty error body)) This broke any caller that generates a report, including the import.orders_freight_bring schedule (listInvoiceNumbers -> generate -> status -> download). Switch GenerateReportEndpoint back to GET and emit the parameters as a query string, restoring v3 GenerateReport behaviour. Adds a regression test asserting the method, URL and query-string placement. --- CHANGELOG.md | 7 ++ .../Reports/GenerateReportEndpoint.php | 20 ++++-- .../Reports/GenerateReportEndpointTest.php | 66 +++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d4537e0..96862e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 expected this path. ### Fixed +- `GenerateReportEndpoint` now issues a **GET** with the report-type filters as + query parameters, not a POST with a JSON body. Bring's + `/reports/api/generate/{customer}/{reportType}` route only accepts GET (the + sibling `generate` / `generate/{customer}` list routes are GET too), so the + v4 POST was rejected with `405 Method Not Allowed` and an empty body — + surfacing as `Bring API returned HTTP 405 (-: HTTP 405 (empty error body))`. + Restores v3 `GenerateReport` behaviour. - `RetryClient` no longer burns retry attempts on permanent 4xx failures when wrapped around a Guzzle client with the default `http_errors=true` setting. A `BadResponseException` whose response is not in the retry diff --git a/src/v4/Endpoint/Reports/GenerateReportEndpoint.php b/src/v4/Endpoint/Reports/GenerateReportEndpoint.php index 384c3e1..46cb0da 100644 --- a/src/v4/Endpoint/Reports/GenerateReportEndpoint.php +++ b/src/v4/Endpoint/Reports/GenerateReportEndpoint.php @@ -8,12 +8,18 @@ use Bring\Api\Http\HttpMethod; /** - * POST https://www.mybring.com/reports/api/generate/{customerNumber}/{reportTypeId}.json + * GET https://www.mybring.com/reports/api/generate/{customerNumber}/{reportTypeId}.json + * + * Bring's report-generation route is a GET with the report-type filters + * passed as query parameters (the sibling list-available routes on the same + * /reports/api/generate base path are GET too). Issuing a POST here makes + * Bring's gateway reject the request with 405 Method Not Allowed and an + * empty body before the response ever carries an error envelope. * * Hardcoded to JSON: this endpoint extends AbstractJsonEndpoint, so * non-JSON responses would fail to parse here. The format of the - * eventual report file is controlled separately by the parameters - * payload, not by the request URL suffix. + * eventual report file is controlled separately by the parameters, + * not by the request URL suffix. * * @extends AbstractJsonEndpoint */ @@ -30,7 +36,7 @@ public function __construct( #[\Override] public function method(): HttpMethod { - return HttpMethod::POST; + return HttpMethod::GET; } #[\Override] @@ -43,11 +49,11 @@ protected function baseUri(): string ); } - /** @return array|null */ + /** @return array */ #[\Override] - protected function jsonBody(): ?array + protected function queryParameters(): array { - return $this->parameters === [] ? null : $this->parameters; + return $this->parameters; } /** @param array $decoded */ diff --git a/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php b/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php new file mode 100644 index 0000000..843dab0 --- /dev/null +++ b/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php @@ -0,0 +1,66 @@ + 'application/json'], '{"reportId":"abc"}'), + ]); + } + + public function testUsesGetNotPost(): void + { + // Regression: Bring's /reports/api/generate route only accepts GET. + // A POST is rejected with 405 Method Not Allowed (empty body). + $client = $this->client(); + $api = ApiClient::withCredentials(new Credentials('me@example.com', 'k'), $client); + + $api->reports()->generate('PARCELS_NORWAY-00012341234', 'MASTER-SPECIFIED_INVOICE'); + + self::assertSame('GET', $client->lastRequest()->getMethod()); + } + + public function testHardcodesJsonUrl(): void + { + $client = $this->client(); + $api = ApiClient::withCredentials(new Credentials('me@example.com', 'k'), $client); + + $api->reports()->generate('PARCELS_NORWAY-00012341234', 'MASTER-SPECIFIED_INVOICE'); + + self::assertStringStartsWith( + 'https://www.mybring.com/reports/api/generate/PARCELS_NORWAY-00012341234/MASTER-SPECIFIED_INVOICE.json', + (string) $client->lastRequest()->getUri(), + ); + } + + public function testParametersGoIntoQueryStringNotBody(): void + { + $client = $this->client(); + $api = ApiClient::withCredentials(new Credentials('me@example.com', 'k'), $client); + + $api->reports()->generate( + 'PARCELS_NORWAY-00012341234', + 'MASTER-SPECIFIED_INVOICE', + ['invoiceNumber' => '12345'], + ); + + $request = $client->lastRequest(); + self::assertStringContainsString('invoiceNumber=12345', (string) $request->getUri()); + // A GET carries no JSON body — the params must not leak into one. + self::assertSame('', (string) $request->getBody()); + } +} From 7af6de8e1666c9a6536df16f8b56914978f69aff Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 15:35:33 +0000 Subject: [PATCH 2/2] fix(reports): map boolean generate params to true/false strings Bring's report filters go over the wire as query parameters; PHP's http_build_query serialises booleans as 1/0, which Bring's validation rejects. Map bool values to the literal strings "true"/"false" before building the query string, mirroring the explicit bool handling in ListInvoiceNumbersEndpoint. Addresses PR review feedback. --- .../Reports/GenerateReportEndpoint.php | 14 ++++++++++++-- .../Reports/GenerateReportEndpointTest.php | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/v4/Endpoint/Reports/GenerateReportEndpoint.php b/src/v4/Endpoint/Reports/GenerateReportEndpoint.php index 46cb0da..2896d55 100644 --- a/src/v4/Endpoint/Reports/GenerateReportEndpoint.php +++ b/src/v4/Endpoint/Reports/GenerateReportEndpoint.php @@ -49,11 +49,21 @@ protected function baseUri(): string ); } - /** @return array */ + /** + * Report filters are passed through verbatim, except booleans: Bring + * expects the literal strings "true"/"false", but PHP's http_build_query + * would serialise a bool as 1/0, which Bring's validation rejects. Mirrors + * the explicit bool handling in {@see ListInvoiceNumbersEndpoint}. + * + * @return array + */ #[\Override] protected function queryParameters(): array { - return $this->parameters; + return array_map( + static fn (mixed $value): mixed => is_bool($value) ? ($value ? 'true' : 'false') : $value, + $this->parameters, + ); } /** @param array $decoded */ diff --git a/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php b/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php index 843dab0..ba149e9 100644 --- a/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php +++ b/tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php @@ -63,4 +63,23 @@ public function testParametersGoIntoQueryStringNotBody(): void // A GET carries no JSON body — the params must not leak into one. self::assertSame('', (string) $request->getBody()); } + + public function testBooleanParametersSerializeAsTrueFalseNotOneZero(): void + { + // PHP's http_build_query would emit 1/0; Bring expects literal true/false. + $client = $this->client(); + $api = ApiClient::withCredentials(new Credentials('me@example.com', 'k'), $client); + + $api->reports()->generate( + 'PARCELS_NORWAY-00012341234', + 'MASTER-SPECIFIED_INVOICE', + ['includeSpecification' => true, 'onlyProcessed' => false], + ); + + $uri = (string) $client->lastRequest()->getUri(); + self::assertStringContainsString('includeSpecification=true', $uri); + self::assertStringContainsString('onlyProcessed=false', $uri); + self::assertStringNotContainsString('includeSpecification=1', $uri); + self::assertStringNotContainsString('onlyProcessed=0', $uri); + } }