fix(web): refresh internal boot onboarding state#1913
Conversation
WalkthroughAdds disk serialNum to GetInternalBootContext, switches onboarding components to use generated GraphQL documents (GetInternalBootContextDocument / GetInternalBootStepVisibilityDocument) with network-only fetchPolicy, replaces local types with generated types, updates tests and expands OnboardingInternalBootStep Props. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1913 +/- ##
==========================================
+ Coverage 50.94% 50.97% +0.02%
==========================================
Files 1023 1023
Lines 70542 70556 +14
Branches 7681 7699 +18
==========================================
+ Hits 35941 35964 +23
+ Misses 34477 34466 -11
- Partials 124 126 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts (1)
436-438:⚠️ Potential issue | 🟠 MajorUpdate stale label expectations to serial-based values.
Line 436 and Line 437 still assert old
emhttpDeviceIdlabels, which now fail after switching UI labels to disk serials.Proposed fix
- expect(deviceSelect.text()).toContain('eligible-disk'); - expect(deviceSelect.text()).toContain('usb-disk'); + expect(deviceSelect.text()).toContain('ELIGIBLE-1'); + expect(deviceSelect.text()).toContain('USB-1');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts` around lines 436 - 438, The test assertions in OnboardingInternalBootStep.test reference old emhttpDeviceId labels ('eligible-disk', 'usb-disk', 'cache-disk'); update the three expectations to assert against the disk serial values used in the test fixtures/mocks instead (use the serial fields from the mocked device objects used in this test, e.g. the entries in the devices/mockDevices array or the deviceFixtures referenced in the setup) so deviceSelect.text() checks for the actual serial strings for the eligible and usb devices and does not contain the cache device's serial.web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts (1)
1091-1105:⚠️ Potential issue | 🟡 MinorFix test assertions to match the new
displayIdlogic.The pipeline is failing because the test expects
'diskA - 500 GB (sda)'but the component now displays theserialNumfield ('DISK-A') per the newdisplayIdpriority logic (serialNum → emhttpDeviceId → device).Since the test data at lines 325 and 332 sets
serialNum: 'DISK-A'andserialNum: 'DISK-B', the assertions should match this behavior.🐛 Proposed fix
const { wrapper } = mountComponent(); - expect(wrapper.text()).toContain('diskA - 500 GB (sda)'); - expect(wrapper.text()).toContain('diskB - 250 GB (sdb)'); + expect(wrapper.text()).toContain('DISK-A - 500 GB (sda)'); + expect(wrapper.text()).toContain('DISK-B - 250 GB (sdb)'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts` around lines 1091 - 1105, Update the failing test assertions to expect the component's new displayId priority (serialNum → emhttpDeviceId → device): in the test that sets draftStore.internalBootSelection (uses serialNum 'DISK-A' and 'DISK-B'), change the two expects on wrapper.text() to assert for 'DISK-A - 500 GB (sda)' and 'DISK-B - 250 GB (sdb)' (keep the size and device name parts but replace the leading device id with the serialNum values); locate the assertions in the test case that calls mountComponent() and references draftStore and internalBootSelection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts`:
- Around line 436-438: The test assertions in OnboardingInternalBootStep.test
reference old emhttpDeviceId labels ('eligible-disk', 'usb-disk', 'cache-disk');
update the three expectations to assert against the disk serial values used in
the test fixtures/mocks instead (use the serial fields from the mocked device
objects used in this test, e.g. the entries in the devices/mockDevices array or
the deviceFixtures referenced in the setup) so deviceSelect.text() checks for
the actual serial strings for the eligible and usb devices and does not contain
the cache device's serial.
In `@web/__test__/components/Onboarding/OnboardingSummaryStep.test.ts`:
- Around line 1091-1105: Update the failing test assertions to expect the
component's new displayId priority (serialNum → emhttpDeviceId → device): in the
test that sets draftStore.internalBootSelection (uses serialNum 'DISK-A' and
'DISK-B'), change the two expects on wrapper.text() to assert for 'DISK-A - 500
GB (sda)' and 'DISK-B - 250 GB (sdb)' (keep the size and device name parts but
replace the leading device id with the serialNum values); locate the assertions
in the test case that calls mountComponent() and references draftStore and
internalBootSelection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 959d0404-e98c-4886-8035-64c075780b08
📒 Files selected for processing (8)
web/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/graphql/getInternalBootContext.query.tsweb/src/components/Onboarding/steps/OnboardingInternalBootStep.vueweb/src/components/Onboarding/steps/OnboardingSummaryStep.vueweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts (1)
178-189: Consider whether this test is too implementation-focused.This test verifies internal implementation details (exact query parameters and fetchPolicy) rather than observable behavior. Per coding guidelines, tests should focus on component behavior and output rather than implementation details.
A more behavior-focused alternative would test that fresh data is displayed after mount (e.g., by changing the mock result and verifying the component reflects it), rather than asserting on the exact
useQuerycall signature.That said, verifying
fetchPolicy: 'network-only'does validate an important PR requirement (fetching fresh data instead of using stale cache), so this may be acceptable as an exception.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts` around lines 178 - 189, The test is too implementation-focused by asserting the exact useQuery call (useQueryMock with GetInternalBootContextDocument and fetchPolicy), so either rewrite it to assert observable behavior (mountComponent then change the mock response and assert the rendered output updates to reflect fresh data), or if preserving the fetchPolicy assertion is required, add an explanatory comment and keep a minimal behavior assertion as well; update the test around mountComponent, useQueryMock, and GetInternalBootContextDocument to verify rendered output changes for different mocked query results (or retain the fetchPolicy check only with a comment noting the exception).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/__test__/components/Onboarding/OnboardingInternalBootStep.test.ts`:
- Around line 178-189: The test is too implementation-focused by asserting the
exact useQuery call (useQueryMock with GetInternalBootContextDocument and
fetchPolicy), so either rewrite it to assert observable behavior (mountComponent
then change the mock response and assert the rendered output updates to reflect
fresh data), or if preserving the fetchPolicy assertion is required, add an
explanatory comment and keep a minimal behavior assertion as well; update the
test around mountComponent, useQueryMock, and GetInternalBootContextDocument to
verify rendered output changes for different mocked query results (or retain the
fetchPolicy check only with a comment noting the exception).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97180410-9ec8-4614-8334-3066731794be
📒 Files selected for processing (2)
web/__test__/components/Onboarding/OnboardingInternalBootStep.test.tsweb/__test__/components/Onboarding/OnboardingSummaryStep.test.ts
1 participant
Summary
Testing
Summary by CodeRabbit
Tests
New Features
Refactor