Resolve runtime product name in passkey flow templates#2997
Resolve runtime product name in passkey flow templates#2997RushanNanayakkara wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughStatic passkey templates now use a {{productName}} placeholder. Flow-builder, flows-meta, and login-flow hooks read config.brand.product_name via useConfig and resolve those placeholders at load time; tests mock useConfig to validate substitution and resource structure. ChangesPasskey Relying-Party Brand Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/apps/console/src/features/flows/api/__tests__/useGetFlowBuilderCoreResources.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. frontend/apps/console/src/features/flows/api/__tests__/useGetFlowsMeta.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. frontend/apps/console/src/features/flows/api/useGetFlowBuilderCoreResources.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/apps/console/src/features/login-flow/api/__tests__/useGetLoginFlowBuilderResources.test.ts (1)
23-38: ⚡ Quick winAdd a login-flow placeholder substitution assertion.
You now mock
product_name, but this suite still doesn’t assert that login-flow resources replace{{productName}}. Add one focused assertion so this path can’t regress silently.Suggested direction
+ it('should resolve {{productName}} placeholders in login-flow resources', () => { + const {result} = renderHook(() => useGetLoginFlowBuilderResources()); + const serialized = JSON.stringify(result.current.data); + + expect(serialized).not.toContain('{{productName}}'); + expect(serialized).toContain('TestProduct'); + });As per coding guidelines,
**/*.{ts,tsx}: "Follow React/TypeScript frontend development guidelines including component patterns, testing, and linting as documented in docs/content/community/contributing/contributing-code/frontend-development/overview.mdx".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/login-flow/api/__tests__/useGetLoginFlowBuilderResources.test.ts` around lines 23 - 38, The test mocks useConfig to set product_name to 'TestProduct' but doesn't assert that login-flow resources substitute the {{productName}} placeholder; update the test in useGetLoginFlowBuilderResources.test.ts to call the hook/function under test (useGetLoginFlowBuilderResources) and add an assertion that at least one returned resource string (e.g., labels, titles, or messages from the hook's result) contains 'TestProduct' instead of '{{productName}}', ensuring the placeholder replacement logic is exercised; reference the mocked useConfig (product_name) and the placeholder '{{productName}}' in your assertion.frontend/apps/console/src/features/flows/api/__tests__/useGetFlowBuilderCoreResources.test.ts (1)
105-110: ⚡ Quick winStrengthen the widgets assertion to prevent false positives.
This now only checks array type; an empty widgets payload would still pass. Add a non-empty assertion here like templates.
Suggested patch
it('should return data containing widgets resolved from JSON file', () => { const {result} = renderHook(() => useGetFlowBuilderCoreResources()); const {data} = result.current; expect(Array.isArray(data.widgets)).toBe(true); + expect(data.widgets.length).toBeGreaterThan(0); });As per coding guidelines,
**/*.{ts,tsx}: "Follow React/TypeScript frontend development guidelines including component patterns, testing, and linting as documented in docs/content/community/contributing/contributing-code/frontend-development/overview.mdx".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/flows/api/__tests__/useGetFlowBuilderCoreResources.test.ts` around lines 105 - 110, The test for useGetFlowBuilderCoreResources currently only asserts that data.widgets is an array, which allows an empty array to pass; update the test in the 'should return data containing widgets resolved from JSON file' spec to assert the array is non-empty (for example, check data.widgets.length > 0 or expect(data.widgets).not.toHaveLength(0)) and optionally validate one expected property on the first widget (e.g., presence of id or type) to ensure actual payload content; target the test in useGetFlowBuilderCoreResources.test.ts and modify the assertion after obtaining result.current.data to enforce non-empty widgets and a basic shape check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@frontend/apps/console/src/features/flows/api/__tests__/useGetFlowBuilderCoreResources.test.ts`:
- Around line 105-110: The test for useGetFlowBuilderCoreResources currently
only asserts that data.widgets is an array, which allows an empty array to pass;
update the test in the 'should return data containing widgets resolved from JSON
file' spec to assert the array is non-empty (for example, check
data.widgets.length > 0 or expect(data.widgets).not.toHaveLength(0)) and
optionally validate one expected property on the first widget (e.g., presence of
id or type) to ensure actual payload content; target the test in
useGetFlowBuilderCoreResources.test.ts and modify the assertion after obtaining
result.current.data to enforce non-empty widgets and a basic shape check.
In
`@frontend/apps/console/src/features/login-flow/api/__tests__/useGetLoginFlowBuilderResources.test.ts`:
- Around line 23-38: The test mocks useConfig to set product_name to
'TestProduct' but doesn't assert that login-flow resources substitute the
{{productName}} placeholder; update the test in
useGetLoginFlowBuilderResources.test.ts to call the hook/function under test
(useGetLoginFlowBuilderResources) and add an assertion that at least one
returned resource string (e.g., labels, titles, or messages from the hook's
result) contains 'TestProduct' instead of '{{productName}}', ensuring the
placeholder replacement logic is exercised; reference the mocked useConfig
(product_name) and the placeholder '{{productName}}' in your assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a1476248-9236-4fe5-9bf9-e76306a992b4
📒 Files selected for processing (8)
frontend/apps/console/src/features/flows/api/__tests__/useGetFlowBuilderCoreResources.test.tsfrontend/apps/console/src/features/flows/api/useGetFlowBuilderCoreResources.tsfrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/login-flow/api/__tests__/useGetLoginFlowBuilderResources.test.tsfrontend/apps/console/src/features/login-flow/api/useGetLoginFlowBuilderResources.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/apps/console/src/features/login-flow/data/templates.jsonfrontend/apps/console/src/features/login-flow/data/widgets.json
RushanNanayakkara
force-pushed
the
claude/passkey-rp-name-runtime-config
branch
from
e93f3ba to
fff0fb1
Compare
|
Thanks for the review @coderabbitai.
|
|
Tip For best results, initiate chat on the files or code changes.
Both points are well handled. 🐇 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
RushanNanayakkara
force-pushed
the
claude/passkey-rp-name-runtime-config
branch
from
fff0fb1 to
81010c4
Compare
|
Self-spotted a regression while doing end-to-end browser verification: a third consumer of Pushed the fix ( Confirmed end-to-end by creating a passkey-template flow against a deployment with |
Replaces the hardcoded "ThunderID" relyingPartyName string in flow
builder seed JSON with a {{productName}} placeholder, and resolves it
at hook-load time from config.brand.product_name. This is the only
end-user-visible surface that the existing brand configuration
mechanism could not previously reach -- the WebAuthn browser dialog
now reads the deployment's configured product name instead of the
literal "ThunderID".
Reuses the existing updateTemplatePlaceholderReferences util so no
new placeholder syntax or substitution machinery is introduced. The
hooks' return shapes are unchanged, and the admin-facing Passkey
properties panel continues to surface the resolved value as the
default (admins can still override it).
RushanNanayakkara
force-pushed
the
claude/passkey-rp-name-runtime-config
branch
from
81010c4 to
c54bf81
Compare
| * every consumer sees the correct value without further work. | ||
| * | ||
| * This function calls the GET method of the following endpoint to get the resources. | ||
| * - TODO: Fill this |
There was a problem hiding this comment.
shall we remove the todo comments?
There was a problem hiding this comment.
Since this is not part of the PR, better not to remove it without checking with the pending tasks.
2 participants
Summary
Fixes #2995.
Resolves the only end-user-visible surface that the existing
configuration.brand.productNamemechanism could not previously reach: the WebAuthn passkey relying-party name shipped in the flow-builder seed JSON.What changed
"ThunderID"with the"{{productName}}"placeholder in 12 lines across 4 seed JSON files (flows/data/templates.json,login-flow/data/{templates,executors,widgets}.json).useGetFlowBuilderCoreResourcesanduseGetLoginFlowBuilderResourcesnow resolve the placeholder at load time using the existingupdateTemplatePlaceholderReferencesutil andconfig.brand.product_namefromuseConfig().useConfig, and added a substitution assertion inuseGetFlowBuilderCoreResources.test.ts.No new util, no new placeholder syntax — reuses the
{{key}}mechanism already present for ID generation and i18n. Hook return shapes are unchanged.Test plan
pnpm test useGetFlowBuilderCoreResources useGetLoginFlowBuilderResources— 38/38 passingpnpm testandpnpm lintinfrontend/apps/console— covered by CIconfiguration.brand.productNameoverridden in Helm values, instantiate a passkey-based template and verify the relying-party-name field defaults to the configured product name in the Passkey properties panel."ThunderID").Summary by CodeRabbit
New Features
Tests