fix: Add dedicated TPM license move option#1909
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe onboarding modal no longer treats the login page specially and unauthenticated mock support was removed. A "Move License to TPM" flow was added: UI button triggers a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Registration UI
participant Account as Account Store
participant ServerStore as Server Store (serverReplacePayload)
participant Backend as send()/Backend
UI->>Account: click "Move License to TPM"
Account->>ServerStore: read serverReplacePayload
ServerStore-->>Account: serverReplacePayload (with replaced guid)
Account->>Backend: send ACCOUNT_CALLBACK {type: "replace", payload: serverReplacePayload}
Backend-->>Account: ack
Account-->>UI: update state / close UI
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 |
elibosley
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/__test__/store/server.test.ts (1)
671-696: Add tests for the remaining GUID-selection branches.These tests cover TPM-available and TPM-equals-flash paths, but Line 211 also branches on
guid.startsWith('03-')and missingtpmGuid. Add one case for each to guard regressions in TPM-boot and no-TPM states.Suggested test additions
+ it('should create serverReplacePayload with flash guid when booted from TPM guid', () => { + const store = getStore(); + + store.setServer({ + flashGuid: '058F-6387-0000-0000F1F1E1C6', + guid: '03-V35H8S0L1QHK1SBG1XHXJNH7', + state: 'PRO' as ServerState, + tpmGuid: '03-V35H8S0L1QHK1SBG1XHXJNH7', + }); + + expect(store.serverReplacePayload.guid).toBe('058F-6387-0000-0000F1F1E1C6'); + }); + + it('should create serverReplacePayload with flash guid when TPM guid is missing', () => { + const store = getStore(); + + store.setServer({ + flashGuid: '058F-6387-0000-0000F1F1E1C6', + guid: '058F-6387-0000-0000F1F1E1C6', + state: 'PRO' as ServerState, + tpmGuid: undefined, + }); + + expect(store.serverReplacePayload.guid).toBe('058F-6387-0000-0000F1F1E1C6'); + });Based on learnings: Applies to /test/store//*.ts : Test Pinia store getter dependencies are properly mocked.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/store/server.test.ts` around lines 671 - 696, Add two unit tests to cover the remaining GUID-selection branches in the store: (1) a case where the server.guid startsWith('03-') and tpmGuid is undefined — call getStore(), then store.setServer({ guid: '03-XXXX...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert store.serverReplacePayload.guid matches the server.guid (the '03-' GUID) to cover the TPM-boot branch; (2) a case where tpmGuid is missing (undefined) and guid does not start with '03-' — call getStore(), then store.setServer({ guid: '058F-...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert store.serverReplacePayload.guid equals the flashGuid to cover the no-TPM branch; reuse the existing pattern around getStore(), store.setServer(...) and assertions for serverReplacePayload.guid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/store/server.ts`:
- Around line 137-145: The computed replaceFlashGuid currently falls back to
flashGuid.value which can be missing/stale; change its logic to use guid.value
as the default and only substitute tpmGuid.value when bootDeviceType.value ===
'flash' && hasDistinctTpmGuid.value, i.e. return tpmGuid.value in that
distinct-TPM case otherwise return guid.value || undefined; update any related
usage in serverReplacePayload and the showTpmTransferInfo logic in
Registration.standalone.vue to use the same fallback (guid over flashGuid) and
only swap to tpmGuid for the distinct TPM path to avoid overwriting a valid
current guid with undefined.
---
Nitpick comments:
In `@web/__test__/store/server.test.ts`:
- Around line 671-696: Add two unit tests to cover the remaining GUID-selection
branches in the store: (1) a case where the server.guid startsWith('03-') and
tpmGuid is undefined — call getStore(), then store.setServer({ guid:
'03-XXXX...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert
store.serverReplacePayload.guid matches the server.guid (the '03-' GUID) to
cover the TPM-boot branch; (2) a case where tpmGuid is missing (undefined) and
guid does not start with '03-' — call getStore(), then store.setServer({ guid:
'058F-...', flashGuid: '058F-...', state: 'PRO' as ServerState }) and assert
store.serverReplacePayload.guid equals the flashGuid to cover the no-TPM branch;
reuse the existing pattern around getStore(), store.setServer(...) and
assertions for serverReplacePayload.guid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 11d971e9-4947-4426-94c1-a05033f9bcd0
📒 Files selected for processing (11)
web/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Registration.test.tsweb/__test__/store/account.test.tsweb/__test__/store/server.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vueweb/src/components/Onboarding/store/onboardingStatus.tsweb/src/components/Registration.standalone.vueweb/src/locales/en.jsonweb/src/store/account.tsweb/src/store/server.ts
💤 Files with no reviewable changes (2)
- web/src/components/Onboarding/standalone/OnboardingAdminPanel.standalone.vue
- web/test/components/Onboarding/OnboardingModal.test.ts
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1909 +/- ##
==========================================
- Coverage 50.97% 50.94% -0.03%
==========================================
Files 1023 1023
Lines 70701 70542 -159
Branches 7703 7681 -22
==========================================
- Hits 36038 35941 -97
+ Misses 34539 34477 -62
Partials 124 124 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Summary
Move License to TPMaction on the registration page when a distinct TPM GUID is availableReplace Keyflow unchanged while adding a TPM-specific account callback payloadTesting
Notes
Summary by CodeRabbit
Bug Fixes
New Features
UI/UX