From f96820a670ff636da819c133ebd912483b67c9e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 21:32:03 +0000 Subject: [PATCH] fix(reports): treat DONE as the ready status, not COMPLETED/READY ReportStatusResponse::isReady() checked the report status against COMPLETED / READY, but Bring's Reports API never returns those values. It reports NOT_DONE while a report is still generating and DONE once it is ready to download (the same contract the legacy StatusOfReport client encodes with `getStatus() === 'DONE'`). Because isReady() never matched a real response, every polled report looked perpetually unfinished: pending-report pollers downloaded and processed nothing and never flipped their tracking rows off `pending`, so the tracking table filled with reports that were never collected. Match the real API by treating DONE (case-insensitively) as ready. Also fall back through the real xmlUrl / xlsUrl fields when mapping the download URL, since Bring has no `downloadUrl` field. Adds a regression test covering the ready/not-ready statuses and the URL fallback. --- .../Endpoint/Reports/ReportStatusResponse.php | 20 +++- .../Reports/ReportStatusResponseTest.php | 95 +++++++++++++++++++ 2 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 tests/v4/Endpoint/Reports/ReportStatusResponseTest.php diff --git a/src/v4/Endpoint/Reports/ReportStatusResponse.php b/src/v4/Endpoint/Reports/ReportStatusResponse.php index 0e9f532..1cce092 100644 --- a/src/v4/Endpoint/Reports/ReportStatusResponse.php +++ b/src/v4/Endpoint/Reports/ReportStatusResponse.php @@ -14,17 +14,33 @@ public function __construct( ) { } + /** + * True once Bring has finished generating the report. + * + * Bring's Reports API reports progress as `NOT_DONE` while the report is + * still building and `DONE` once it is ready to download (see the legacy + * {@see \Crakter\BringApi\Clients\Reports\StatusOfReport::checkStatus()}, + * written against the live API). The earlier `COMPLETED`/`READY` check + * never matched a real response, so every polled report looked perpetually + * unfinished and callers' pending-report tables filled with rows that were + * never collected. + */ public function isReady(): bool { - return strtoupper($this->status) === 'COMPLETED' || strtoupper($this->status) === 'READY'; + return strtoupper($this->status) === 'DONE'; } /** @param array $decoded */ public static function fromArray(array $decoded): self { + // Bring returns the finished report's location as `xmlUrl` / `xlsUrl` + // (there is no `downloadUrl` field); fall back through them so the + // parsed URL isn't perpetually null on a real response. + $downloadUrl = $decoded['downloadUrl'] ?? $decoded['xmlUrl'] ?? $decoded['xlsUrl'] ?? null; + return new self( status: (string) ($decoded['status'] ?? ''), - downloadUrl: isset($decoded['downloadUrl']) ? (string) $decoded['downloadUrl'] : null, + downloadUrl: null !== $downloadUrl ? (string) $downloadUrl : null, raw: $decoded, ); } diff --git a/tests/v4/Endpoint/Reports/ReportStatusResponseTest.php b/tests/v4/Endpoint/Reports/ReportStatusResponseTest.php new file mode 100644 index 0000000..8c5ee8f --- /dev/null +++ b/tests/v4/Endpoint/Reports/ReportStatusResponseTest.php @@ -0,0 +1,95 @@ + 'DONE']); + + self::assertTrue($response->isReady()); + } + + public function testNotDoneReportIsNotReady(): void + { + // NOT_DONE is what Bring returns while the report is still generating. + $response = ReportStatusResponse::fromArray(['status' => 'NOT_DONE']); + + self::assertFalse($response->isReady()); + } + + /** + * Regression: the previous implementation only recognised COMPLETED / READY — + * values Bring never returns — so every polled report looked perpetually + * unfinished and pending-report tables filled with rows that were never + * collected. + */ + #[DataProvider('provideStatusesThatBringNeverReadiesOn')] + public function testStatusesBringNeverReturnsAreNotReady(string $status): void + { + self::assertFalse(ReportStatusResponse::fromArray(['status' => $status])->isReady()); + } + + /** @return iterable */ + public static function provideStatusesThatBringNeverReadiesOn(): iterable + { + yield 'completed (never emitted by Bring)' => ['COMPLETED']; + yield 'ready (never emitted by Bring)' => ['READY']; + yield 'empty' => ['']; + } + + public function testIsReadyIsCaseInsensitive(): void + { + self::assertTrue(ReportStatusResponse::fromArray(['status' => 'done'])->isReady()); + } + + public function testMissingStatusIsNotReady(): void + { + $response = ReportStatusResponse::fromArray([]); + + self::assertSame('', $response->status); + self::assertFalse($response->isReady()); + } + + public function testDownloadUrlFallsBackToXmlUrl(): void + { + // The real status.json exposes the finished report as xmlUrl / xlsUrl, + // not downloadUrl, so the parsed URL must fall back through them. + $response = ReportStatusResponse::fromArray([ + 'status' => 'DONE', + 'xmlUrl' => 'https://www.mybring.com/reports/api/report/abc.xml', + 'xlsUrl' => 'https://www.mybring.com/reports/api/report/abc.xls', + ]); + + self::assertSame('https://www.mybring.com/reports/api/report/abc.xml', $response->downloadUrl); + } + + public function testExplicitDownloadUrlWins(): void + { + $response = ReportStatusResponse::fromArray([ + 'status' => 'DONE', + 'downloadUrl' => 'https://example.test/explicit', + 'xmlUrl' => 'https://www.mybring.com/reports/api/report/abc.xml', + ]); + + self::assertSame('https://example.test/explicit', $response->downloadUrl); + } + + public function testDownloadUrlNullWhenAbsent(): void + { + $response = ReportStatusResponse::fromArray(['status' => 'NOT_DONE']); + + self::assertNull($response->downloadUrl); + } +}