feat(onboarding): wire the verify console to the real first-evaluation signal - #8132
Conversation
…n signal
Adds an RTK Query endpoint for GET environments/{key}/onboarding-status/
(a Core endpoint) plus its request/response types, so the frontend can read
whether an environment has received its first SDK evaluation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe frontend adds typed request and response models plus an RTK Query service for environment onboarding status. The onboarding connection hook polls this endpoint every three seconds until Estimated code review effort: 3 (Moderate) | ~20 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9f52ac8f-6cc0-4401-b563-21b5ed937f91
📒 Files selected for processing (9)
frontend/common/code-help/init/init-react-native.jsfrontend/common/code-help/init/init-react.jsfrontend/common/constants.tsfrontend/common/services/useEnvironmentOnboardingStatus.tsfrontend/common/types/requests.tsfrontend/common/types/responses.tsfrontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/hooks/useOnboardingConnection.ts
✅ private-cloud · depot-ubuntu-latest-16 — run #18791 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-arm-16 — run #18791 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18791 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18789 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18789 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18789 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18789 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18788 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18788 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18788 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18788 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18786 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
…tion signal useOnboardingConnection now polls onboarding-status/ and flips to connected when first_evaluated_at lands, replacing the ?connected query-param stub so the connected state can never be faked. The e2e test mocks the endpoint to exercise the real polling path instead of the removed param. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
266c600 to
1ef00d8
Compare
|
@themis-blindfold review |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1597317-b977-4192-a358-bea8b8534c25
📒 Files selected for processing (3)
frontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/hooks/useOnboardingConnection.ts
⚖️ Themis review: 🧹 Ship it, nits insideThe stub is properly gone and the replacement is sound. The hook latches Two nits, both about edges the change opened up rather than the change itself. No blockers or majors.
🧹 Nits
⚖️ Acknowledged
📝 Walkthrough
🧪 How to verify
Automate: a small unit test around Product take: This is the change that makes the onboarding page honest — before it, the "verify your connection" step could only be satisfied by typing a query param, so the flag toggle and next-step cards were unreachable for a real signup. Solid, load-bearing improvement to the quickstart flow rather than a large capability. 🧭 Assumptions & unverified claims
Deleting a URL query param is rarely this much of an upgrade. · reviewed at 1ef00d8 |
The rename reload drops the connected latch, so the visual snapshot raced the first poll and could capture LISTENING where the baseline has LIVE. Wait for the flip before snapshotting; the later wait before the next-quest click is now redundant, since nothing reloads in between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Nit 1 fixed in 00f0f1ce8: moved the wait ahead of the snapshot and dropped the later one, which became redundant once the latch is held. Small correction on the mechanism, RTK Query fires its initial fetch on mount, so it wasn't a 3s gap, just a race with the first response. |
|
Your correction is right, and it doesn't change the fix. The 3s interval does apply to the mid-test flip at line 99, where the component is already mounted — that one's covered by I can't see 00f0f1ce8 in this checkout (head is 1ef00d8), so this is on your description: dropping only line 150 is correct, since nothing between the reload and there remounts the hook. Keep the |
Zaimwa9
left a comment
There was a problem hiding this comment.
Couple of non-blocking comments that could be left for later
⚖️ Themis review: 🧹 Ship it, nits insideDelta re-review of the one new commit. It adds
🧹 Nits
📝 Walkthrough
🧪 How to verify
Automate: the existing Product take: No user-visible change in this commit — it stabilises the e2e visual baseline for the onboarding console. The PR as a whole is a solid improvement: the verify console now reflects a real first evaluation instead of a URL query param. 🧭 Assumptions & unverified claims
A one-line |
docs/if required so people know about the feature.Changes
Contributes to #7767, consuming the first-evaluation signal from #7623.
The verify console was stubbed: it read a
?connectedquery param, so the connected state could be faked from the URL. It now reflects the real signal.useEnvironmentOnboardingStatus, callingGET environments/{key}/onboarding-status/. Unauthenticated, keyed by the environment's client-side API key, served by Core rather than the edge host.useOnboardingConnectiontakes the environment key and polls that endpoint every 3s, flipping toconnectedoncefirst_evaluated_atis no longer null. Edge reports the first evaluation asynchronously after the SDK's first request, so the flip is not instant; the value never reverts, so polling stops once it lands.?connectedhatch is gone.How did you test this code?
The e2e test (
onboarding-tests.pw.ts) mocksonboarding-status/and flipsfirst_evaluated_atmid-test, so it now asserts through the real polling path rather than the query param.Manually verified with the
onboarding_quickstart_flowflag on:/getting-startedholds on "listening" while nothing has evaluated?connectedon the URL no longer does anythingtscandeslintare clean on the changed files.