fix(reports): generate report via GET, not POST (Bring returns 405 on POST)#63
Conversation
… POST)
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.
There was a problem hiding this comment.
Code Review
This pull request updates GenerateReportEndpoint to use a GET request with query parameters instead of a POST request with a JSON body, resolving a 405 Method Not Allowed error. It also adds unit tests to verify this behavior. The feedback suggests mapping boolean query parameters to literal 'true' or 'false' strings to prevent potential API validation issues, as PHP serializes booleans as '1' or '0' by default.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| /** @return array<string, mixed> */ | ||
| #[\Override] | ||
| protected function jsonBody(): ?array | ||
| protected function queryParameters(): array | ||
| { | ||
| return $this->parameters === [] ? null : $this->parameters; | ||
| return $this->parameters; | ||
| } |
There was a problem hiding this comment.
When converting parameters to query string arguments via http_build_query (which is called in AbstractJsonEndpoint::uri()), PHP serializes boolean values as 1 (for true) and 0 (for false). Many APIs, including Bring, expect literal 'true' or 'false' strings for boolean query parameters.
To prevent potential API validation or parsing issues, we should explicitly map boolean values to their string representations ('true'/'false') before passing them to the query string builder.
/** @return array<string, mixed> */
#[\Override]
protected function queryParameters(): array
{
return array_map(
fn ($value) => is_bool($value) ? ($value ? 'true' : 'false') : $value,
$this->parameters
);
}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.
Problem
The
import.orders_freight_bringschedule fails with:That schedule runs a Reports API chain —
listInvoiceNumbers → generate → status → download. The v4 rewrite changed the "generate report" endpoint from GET to POST:Crakter\BringApi\Clients\Reports\GenerateReport):GET …/reports/api/generate/{cust}/{reportType}, filters as a query stringGenerateReportEndpoint):POSTwith a JSON bodyBring's
/reports/api/generate/...route only accepts GET (per Bring's docs: "To generate a report, do a GET to the supplied URL"). The POST is rejected by Bring's gateway with405 Method Not Allowedand an empty body — which is why the surfaced message has an empty error code (-) and no error envelope. The two sibling endpoints on the same base path (ListAvailableCustomersEndpoint,ListAvailableReportsForCustomerEndpoint) already use GET; onlyGenerateReportEndpointwas POST.Fix
src/v4/Endpoint/Reports/GenerateReportEndpoint.php:method()→GET(wasPOST)queryParameters()) instead of a JSON bodyPlus a regression test (
tests/v4/Endpoint/Reports/GenerateReportEndpointTest.php) asserting the GET method, the.jsonURL, and query-string placement, and aCHANGELOG.mdentry under Unreleased → Fixed.Verification
vendor/bin/phpunit— 201 tests pass (3 new + existing Reports tests green)Downstream
crakter/purchasepins this library to a master commit incomposer.lock; after merge, runcomposer update crakter/bringapithere to pick up the fix.https://claude.ai/code/session_01GwRgBSHYS7hXV7nBgDTanT
Generated by Claude Code