feat(onboarding): name the SDK that reported the first evaluation - #8147
feat(onboarding): name the SDK that reported the first evaluation#8147talissoncosta wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe onboarding connection hook now returns connection status together with the SDK label from the first evaluation, stopping polling once that evaluation is detected. The onboarding flow passes this data to the terminal, which displays the label in the LIVE receipt. Storybook includes a labelled connected state, and the E2E test verifies the displayed SDK label. 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 |
github-actions
Bot
added
the
front-end
Docker builds report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: 71fee9fa-8d8a-4c7c-bfdd-e0ae00500253
📒 Files selected for processing (5)
frontend/documentation/pages/onboarding/OnboardingTerminal.stories.tsxfrontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/OnboardingTerminal/OnboardingTerminal.tsxfrontend/web/components/pages/onboarding/hooks/useOnboardingConnection.ts
| // The console names the SDK that reported the evaluation, from the mocked | ||
| // first_evaluated_sdk_label above. | ||
| await expect(page.getByText(/flagsmith-js-sdk/)).toBeVisible(); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the assertion to the terminal receipt.
page.getByText(/flagsmith-js-sdk/) searches the whole page and only checks a substring, so it does not prove that the new first evaluation received receipt rendered the label. Assert the exact receipt text or scope the locator to the onboarding terminal.
Proposed fix
- await expect(page.getByText(/flagsmith-js-sdk/)).toBeVisible()
+ await expect(
+ page.getByText('flagsmith-js-sdk · first evaluation received', {
+ exact: true,
+ }),
+ ).toBeVisible()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // The console names the SDK that reported the evaluation, from the mocked | |
| // first_evaluated_sdk_label above. | |
| await expect(page.getByText(/flagsmith-js-sdk/)).toBeVisible(); | |
| // The console names the SDK that reported the evaluation, from the mocked | |
| // first_evaluated_sdk_label above. | |
| await expect( | |
| page.getByText('flagsmith-js-sdk · first evaluation received', { | |
| exact: true, | |
| }), | |
| ).toBeVisible() |
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18816 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #18816 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18817 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18817 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18816 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18816 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18817 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18817 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18815 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18815 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18815 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #18815 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
The verify console printed an invented receipt: "SDK initialized · flags loaded ·
<flag>: true", which asserted a flag value nothing had checked. The
onboarding-status endpoint already returns first_evaluated_sdk_label, so the
receipt now names the SDK that actually reported the evaluation:
flagsmith-python-sdk · first evaluation received
That is the one thing the console could not previously tell you: whether the
evaluation came from the user's own app.
Core sends 'unknown' when it cannot identify the SDK from the user agent, which
is nothing to show a user, so the hook normalises that to null and the line
falls back to "first evaluation received".
useOnboardingConnection now returns { status, sdkLabel }. The label is latched
alongside the status, because skipping the query drops `data`.
The e2e mock already returned a label, so the test asserts the console names it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a44bb36 to
977c780
Compare
github-actions
Bot
added
feature
2 participants
docs/if required so people know about the feature.Changes
Stacked on #8145, so the snippets a reviewer copies from the preview are the fixed ones. Follows @Zaimwa9's suggestion on #8132:
first_evaluated_sdk_labelarrives in the onboarding-status response and nothing used it.The console's receipt line was invented, and asserted a flag value nothing had checked:
It now reports what actually happened:
That's the one thing the console couldn't tell you before: whether the evaluation came from the user's own app rather than something else hitting the environment.
'unknown'when it can't identify the SDK from the user agent. The hook normalises that to null and the line falls back tofirst evaluation received.useOnboardingConnectionreturns{ status, sdkLabel }instead of a bare status string. The label is latched alongside the status, since skipping the query dropsdata.Not in scope, still open from the same review: the poll ignores
isError, so a 404 or 500 renders identically to "still listening".How did you test this code?
The e2e mock already returned
flagsmith-js-sdk, soonboarding-tests.pw.tsnow asserts the console names it after the flip.typecheckandlintclean on the changed filesConnectedWithSdkLabelvariant next toConnectedflagsmith-python-sdk · first evaluation receivedGET environments/{key}/onboarding-status/returnedfirst_evaluated_sdk_label: "flagsmith-python-sdk"Note on the null-label path:
features/views.py:1040only records the evaluation whenmap_request_to_sdk_labelrecognises the user agent, so'unknown'never reaches this endpoint from the SDK flow. The normalisation is defensive, and not reachable by hitting the API with plaincurl.Screenshots