feat: Add TPM licensing availability to registration#1908
Merged
Conversation
Contributor
WalkthroughSurfaces TPM GUID (tpmGuid) and flash GUID (flashGuid) from INI through API, GraphQL, store, and frontend; adds TPM transfer UI, localization, and test updates to handle the new fields. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Web UI (Registration)
participant Store as Client Store
participant GQL as GraphQL Client
participant API as Server GraphQL
participant FS as Config / INI Parser
UI->>Store: read flashGuid, tpmGuid, mdState
Store->>GQL: serverState query (include vars: flashGuid, tpmGuid, mdState)
GQL->>API: GraphQL request
API->>FS: read `var.ini` (flashGUID, tpmGUID)
FS-->>API: return vars with flashGUID/tpmGUID
API-->>GQL: response with vars
GQL-->>Store: update server state
Store-->>UI: re-render Registration UI (show/hide TPM transfer blocks)
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 #1908 +/- ##
==========================================
+ Coverage 50.81% 50.96% +0.14%
==========================================
Files 1023 1023
Lines 70545 70701 +156
Branches 7652 7699 +47
==========================================
+ Hits 35848 36031 +183
+ Misses 34573 34546 -27
Partials 124 124 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78c4b927b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+139
to
+143
| keyInstalled.value && | ||
| bootDeviceType.value === 'flash' && | ||
| flashGuid.value && | ||
| tpmGuid.value && | ||
| flashGuid.value !== tpmGuid.value |
There was a problem hiding this comment.
This condition allows the TPM transfer guidance to render for TRIAL/EEXPIRED servers whenever both GUIDs are present, because keyInstalled only excludes ENOKEYFILE. In those states there is no transferable paid key, so the UI can instruct users to "move this license" when no such transfer is valid. Since showTrialExpiration is already used elsewhere to suppress transfer actions, this notice should be similarly gated to avoid misleading trial/expired users.
Useful? React with 👍 / 👎.
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/src/store/server.ts (1)
268-299: AddflashGuidtoserverDebugPayloadfor parity withtpmGuid.Line 295 includes
tpmGuid, butflashGuidis also part of the new TPM-transfer decision path and is useful in diagnostics.♻️ Suggested small parity update
const serverDebugPayload = computed((): Server => { const payload = { apiVersion: apiVersion.value, avatar: avatar.value, connectPluginInstalled: connectPluginInstalled.value, connectPluginVersion: connectPluginVersion.value, description: description.value, deviceCount: deviceCount.value, email: email.value, expireTime: expireTime.value, + flashGuid: flashGuid.value, flashProduct: flashProduct.value, flashVendor: flashVendor.value, guid: guid.value, inIframe: inIframe.value, lanIp: lanIp.value, locale: locale.value, name: name.value, osVersion: osVersion.value, osVersionBranch: osVersionBranch.value, rebootType: rebootType.value, rebootVersion: rebootVersion.value, registered: registered.value, regGen: regGen.value, regGuid: regGuid.value, regTy: regTy.value, site: site.value, state: state.value, tpmGuid: tpmGuid.value, uptime: uptime.value, username: username.value, wanFQDN: wanFQDN.value, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/server.ts` around lines 268 - 299, The serverDebugPayload computed getter is missing flashGuid; add flashGuid: flashGuid.value to the payload alongside the existing tpmGuid entry so diagnostics include the flash GUID; update the payload object inside the serverDebugPayload computed function (the same block that currently lists tpmGuid: tpmGuid.value) to include flashGuid using the flashGuid reactive reference.
🤖 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/_webGui/testWebComponents.page`:
- Line 93: The mapping for "flashGuid" reads $var['flashGUID'] directly and can
emit undefined-index notices; change the right-hand side to use a guarded
fallback (e.g. use isset($var['flashGUID']) ? $var['flashGUID'] : '' or the
null-coalescing operator $var['flashGUID'] ?? '') so "flashGuid" always gets a
defined value when $var lacks 'flashGUID'.
---
Nitpick comments:
In `@web/src/store/server.ts`:
- Around line 268-299: The serverDebugPayload computed getter is missing
flashGuid; add flashGuid: flashGuid.value to the payload alongside the existing
tpmGuid entry so diagnostics include the flash GUID; update the payload object
inside the serverDebugPayload computed function (the same block that currently
lists tpmGuid: tpmGuid.value) to include flashGuid using the flashGuid reactive
reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7285b2b0-0b11-48ae-999d-583eb7baa987
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
api/dev/states/var.iniapi/generated-schema.graphqlapi/src/__test__/store/modules/emhttp.test.tsapi/src/__test__/store/state-parsers/var.test.tsapi/src/core/types/states/var.tsapi/src/store/services/__test__/state-file-loader.test.tsapi/src/store/state-parsers/var.tsapi/src/unraid-api/graph/resolvers/vars/vars.model.tsweb/__test__/components/Registration.test.tsweb/_webGui/testWebComponents.pageweb/src/_data/serverState.tsweb/src/components/Registration.standalone.vueweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/locales/en.jsonweb/src/store/server.fragment.tsweb/src/store/server.tsweb/types/server.ts
| "flashProduct" => $var['flashProduct'], | ||
| "flashVendor" => $var['flashVendor'], | ||
| "flashBackupActivated" => empty($flashbackup_status['activated']) ? '' : 'true', | ||
| "flashGuid" => $var['flashGUID'], |
Contributor
There was a problem hiding this comment.
Add a fallback for flashGuid to avoid undefined-index notices.
Line [93] directly reads $var['flashGUID'] without guarding. If the key is absent, PHP notices can leak into this test page output.
Suggested fix
- "flashGuid" => $var['flashGUID'],
+ "flashGuid" => $var['flashGUID'] ?? '',📝 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.
Suggested change
| "flashGuid" => $var['flashGUID'], | |
| "flashGuid" => $var['flashGUID'] ?? '', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/_webGui/testWebComponents.page` at line 93, The mapping for "flashGuid"
reads $var['flashGUID'] directly and can emit undefined-index notices; change
the right-hand side to use a guarded fallback (e.g. use isset($var['flashGUID'])
? $var['flashGUID'] : '' or the null-coalescing operator $var['flashGUID'] ??
'') so "flashGuid" always gets a defined value when $var lacks 'flashGUID'.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/__test__/components/Registration.test.ts (2)
305-343: Reduce brittleness from hardcoded copy assertions.These assertions are heavily tied to exact English phrasing. Prefer i18n-key-driven assertions (and structure checks like step count) so copy edits don’t fail behavior tests.
As per coding guidelines: "Test component behavior and output, not implementation details."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Registration.test.ts` around lines 305 - 343, The tests in Registration.test.ts are brittle because they assert exact English strings; update the three TPM-transfer tests (those manipulating serverStore and querying wrapper.find('[data-testid="tpm-transfer-available"]' / '[data-testid="tpm-transfer-ready"]')) to assert behavior via i18n keys and structure instead: replace expect(...text()).toContain('literal copy') with checks that the element contains the translated text by calling the component's $t key (or verifying a data-i18n attribute or the key prop used for each line), assert the presence/absence of specific step elements (e.g. step rows, checkmark icons, count of steps) rather than matching full sentences, and keep the minimal existence checks like expect(...exists()).toBe(true/false); reference wrapper, transferNotice, serverStore.guid/tpmGuid/regGuid and data-testid selectors to locate the changed assertions.
294-330: Make TPM preconditions explicit in tests.These cases depend on boot mode, but
bootDeviceTypeis not set in the scenario setup. Setting it explicitly will make the tests deterministic and less coupled to store defaults.Suggested patch
it('shows TPM transfer guidance when TPM licensing is available', async () => { serverStore.state = 'PRO'; + serverStore.bootDeviceType = 'flash'; serverStore.guid = '058F-6387-0000-0000F1F1E1C6'; serverStore.flashGuid = '058F-6387-0000-0000F1F1E1C6'; serverStore.tpmGuid = '03-V35H8S0L1QHK1SBG1XHXJNH7'; serverStore.keyfile = 'keyfile-present'; @@ it('shows checked TPM transfer steps after switching to TPM boot', async () => { serverStore.state = 'EGUID'; + serverStore.bootDeviceType = 'tpm'; serverStore.guid = '03-V35H8S0L1QHK1SBG1XHXJNH7'; serverStore.tpmGuid = '03-V35H8S0L1QHK1SBG1XHXJNH7'; serverStore.regGuid = '058F-6387-0000-0000F1F1E1C6';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/Registration.test.ts` around lines 294 - 330, The tests rely on boot mode but never set serverStore.bootDeviceType, making results dependent on store defaults; update each affected test (e.g., the cases manipulating serverStore.state/guid/flashGuid/tpmGuid/regGuid in Registration.test.ts) to explicitly set serverStore.bootDeviceType to the expected value for that scenario (for example 'USB' for USB-boot scenarios and 'TPM' or 'TPM_LEGACY' for TPM-boot scenarios) before awaiting wrapper.vm.$nextTick() so the assertions around wrapper.find('[data-testid="tpm-transfer-available"]') and checked TPM transfer steps are deterministic. Ensure you set it in the three blocks shown (PRO case, TRIAL case, and EGUID case) to match the intended preconditions.
🤖 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/Registration.test.ts`:
- Around line 305-343: The tests in Registration.test.ts are brittle because
they assert exact English strings; update the three TPM-transfer tests (those
manipulating serverStore and querying
wrapper.find('[data-testid="tpm-transfer-available"]' /
'[data-testid="tpm-transfer-ready"]')) to assert behavior via i18n keys and
structure instead: replace expect(...text()).toContain('literal copy') with
checks that the element contains the translated text by calling the component's
$t key (or verifying a data-i18n attribute or the key prop used for each line),
assert the presence/absence of specific step elements (e.g. step rows, checkmark
icons, count of steps) rather than matching full sentences, and keep the minimal
existence checks like expect(...exists()).toBe(true/false); reference wrapper,
transferNotice, serverStore.guid/tpmGuid/regGuid and data-testid selectors to
locate the changed assertions.
- Around line 294-330: The tests rely on boot mode but never set
serverStore.bootDeviceType, making results dependent on store defaults; update
each affected test (e.g., the cases manipulating
serverStore.state/guid/flashGuid/tpmGuid/regGuid in Registration.test.ts) to
explicitly set serverStore.bootDeviceType to the expected value for that
scenario (for example 'USB' for USB-boot scenarios and 'TPM' or 'TPM_LEGACY' for
TPM-boot scenarios) before awaiting wrapper.vm.$nextTick() so the assertions
around wrapper.find('[data-testid="tpm-transfer-available"]') and checked TPM
transfer steps are deterministic. Ensure you set it in the three blocks shown
(PRO case, TRIAL case, and EGUID case) to match the intended preconditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9c51e308-8b64-4d95-90e8-c994ef83ccce
📒 Files selected for processing (3)
web/__test__/components/Registration.test.tsweb/src/components/Registration.standalone.vueweb/src/locales/en.json
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/__test__/components/CallbackFeedback.test.ts (1)
313-336: Well-structured test for the reload behavior.The test correctly:
- Mocks
window.location.reload- Sets up success state conditions
- Triggers the Modal's close event
- Verifies both the callback status reset and the reload invocation
One minor suggestion: consider importing
nextTickfromvueand usingawait nextTick()instead ofwrapper.vm.$nextTick()for consistency with the coding guidelines pattern.♻️ Optional: Use imported nextTick
+import { nextTick, ref } from 'vue'; -import { ref } from 'vue'; ... wrapper.findComponent({ name: 'Modal' }).vm.$emit('close'); - await wrapper.vm.$nextTick(); + await nextTick();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/__test__/components/CallbackFeedback.test.ts` around lines 313 - 336, Replace the instance call to wrapper.vm.$nextTick() with the imported nextTick() utility: import { nextTick } from 'vue' at the top of the test file (if not already present) and change the await wrapper.vm.$nextTick(); line in the 'reloads the page when the modal is dismissed after a callback action' test to await nextTick(); to match project conventions (referencing the test function and wrapper usage around the Modal close emit).web/src/store/server.ts (1)
270-306: Consider addingflashGuidandmdStatetoserverDebugPayloadfor consistency.The debug payload includes
tpmGuidbut omitsflashGuidandmdState. If these fields are useful for debugging TPM transfer issues, consider including them here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/server.ts` around lines 270 - 306, The serverDebugPayload computed currently builds payload without flashGuid or mdState; update the payload object inside the serverDebugPayload computed to include flashGuid: flashGuid.value and mdState: mdState.value (keeping the existing Object.fromEntries filter logic so empty/null/undefined values are still removed) so these fields are present in the debug output alongside tpmGuid and other properties.
🤖 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/CallbackFeedback.test.ts`:
- Around line 313-336: Replace the instance call to wrapper.vm.$nextTick() with
the imported nextTick() utility: import { nextTick } from 'vue' at the top of
the test file (if not already present) and change the await
wrapper.vm.$nextTick(); line in the 'reloads the page when the modal is
dismissed after a callback action' test to await nextTick(); to match project
conventions (referencing the test function and wrapper usage around the Modal
close emit).
In `@web/src/store/server.ts`:
- Around line 270-306: The serverDebugPayload computed currently builds payload
without flashGuid or mdState; update the payload object inside the
serverDebugPayload computed to include flashGuid: flashGuid.value and mdState:
mdState.value (keeping the existing Object.fromEntries filter logic so
empty/null/undefined values are still removed) so these fields are present in
the debug output alongside tpmGuid and other properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a28dceaa-a045-4017-857c-73ae77b18db5
📒 Files selected for processing (10)
web/__test__/components/CallbackFeedback.test.tsweb/__test__/components/Registration.test.tsweb/src/components/Registration.standalone.vueweb/src/components/UserProfile/CallbackFeedback.vueweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/locales/en.jsonweb/src/store/server.fragment.tsweb/src/store/server.tsweb/types/server.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- web/types/server.ts
- web/src/locales/en.json
- web/src/composables/gql/graphql.ts
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
tpmGuidthrough the vars API/schema and thread it through the registration page stateTesting
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fix / UX