fix(review): paginate getLatestDeploymentStatus's deployments and statuses reads - #7829
Closed
shin-core wants to merge 1 commit into
Closed
fix(review): paginate getLatestDeploymentStatus's deployments and statuses reads#7829shin-core wants to merge 1 commit into
shin-core wants to merge 1 commit into
Conversation
Contributor
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7829 +/- ##
==========================================
+ Coverage 80.62% 88.82% +8.19%
==========================================
Files 729 89 -640
Lines 74671 21351 -53320
Branches 22791 3722 -19069
==========================================
- Hits 60205 18964 -41241
+ Misses 11654 2196 -9458
+ Partials 2812 191 -2621
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…tuses reads getLatestDeploymentStatus fetched only page 1 of a head SHA/ref's Deployments list (per_page=10) and page 1 of each deployment's statuses -- never following GitHub's Link: rel="next", unlike this file's own findPreviewUrlFromPrComments and getPreviewBuildState. A ref with more than 10 deployments (repeated CI re-runs, multiple environments, a long-lived branch) could carry the deployment with the real environment_url outside page 1, so the function under-reported a missing preview exactly as if none existed -- the same false-negative class the file's own header warns about, and the class JSONbored#7469 already fixed for the comment/check-run reads. Reuse the existing findAcrossPages helper for both reads, the way the sibling functions do: walk deployment pages, and per deployment walk its status pages, returning the first usable environment_url. findAcrossPages now awaits its probe so the outer deployments scan can fetch each deployment's statuses (a sync probe is unaffected). The sawFailure/sawPending bookkeeping and DeploymentLookup return contract are preserved. Closes JSONbored#7805
shin-core
force-pushed
the
fix/paginate-deployment-status-7805
branch
from
July 21, 2026 14:27
94006b8 to
48e1c9e
Compare
Contributor
|
This repository reviews pull requests one-shot: the PR must be correct as originally opened. Pushing an additional commit closes it automatically instead of restarting review — open a fresh pull request with every fix included. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
getLatestDeploymentStatus(src/review/visual/preview-url.ts) fetched only page 1 of a head SHA/ref's Deployments list (?...&per_page=10) and page 1 of each deployment's statuses — never following GitHub'sLink: rel="next", unlike this same file'sfindPreviewUrlFromPrCommentsandgetPreviewBuildState. A ref with more than 10 deployment records (repeated CI re-runs, multiple environments per push, a long-lived branch) can carry the deployment with the realenvironment_urloutside page 1, so the function returned{ url: null }as if no preview existed — the same false-negative class the file's own header warns about, and the class#7469already fixed for the comment/check-run reads (#7805is the third, still-unpaginated instance).Fix
Reuse the existing
findAcrossPageshelper (same file) for both reads, exactly the way the two sibling functions already do:environment_url.success/in_progressstatus with anenvironment_url.findAcrossPagesnowawaits itsprobeso the outer deployments scan can fetch each deployment's statuses per page (a synchronous probe — the two existing callers — is unaffected, sinceawaiton a non-promise is a no-op). ThesawFailure/sawPendingbookkeeping and theDeploymentLookupreturn contract are preserved exactly; the failed/pending verdict keys off each deployment'sstatuses[0]"latest" status as before, and now accumulates across all deployments/pages.Tests
test/unit/preview-url.test.ts, mirroring the file's existingNEXT_LINK/isPage2page-2 tests:environment_urlsits on page 2 of the deployments list — the URL is still found.successstatus on page 2 of a deployment's statuses — the URL is still found.Both are proven to fail when pagination is capped to page 1.
Validation
npx vitest run test/unit/preview-url.test.ts— 25/25; broadfindAcrossPages-consumer sweep (preview-url + visual-capture) — 321/321 (the async-probe widening breaks no existing caller).npm run typecheck— exit 0.git diff --checkclean.Closes #7805