Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 23 additions & 7 deletions src/v4/Endpoint/Reports/GenerateReportEndpoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<GenerateReportResponse>
*/
Expand All @@ -30,7 +36,7 @@ public function __construct(
#[\Override]
public function method(): HttpMethod
{
return HttpMethod::POST;
return HttpMethod::GET;
}

#[\Override]
Expand All @@ -43,11 +49,21 @@ protected function baseUri(): string
);
}

/** @return array<mixed, mixed>|null */
/**
* 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<string, mixed>
*/
#[\Override]
protected function jsonBody(): ?array
protected function queryParameters(): array
{
return $this->parameters === [] ? null : $this->parameters;
return array_map(
static fn (mixed $value): mixed => is_bool($value) ? ($value ? 'true' : 'false') : $value,
$this->parameters,
);
}

/** @param array<mixed, mixed> $decoded */
Expand Down
85 changes: 85 additions & 0 deletions tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

declare(strict_types=1);

namespace Bring\Api\Tests\Endpoint\Reports;

use Bring\Api\ApiClient;
use Bring\Api\Auth\Credentials;
use Bring\Api\Endpoint\Reports\GenerateReportEndpoint;
use Bring\Api\Tests\Support\RecordingClient;
use GuzzleHttp\Psr7\Response;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;

#[CoversClass(GenerateReportEndpoint::class)]
final class GenerateReportEndpointTest extends TestCase
{
private function client(): RecordingClient
{
return new RecordingClient([
new Response(200, ['Content-Type' => '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());
}

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);
}
}