Fix user add to use onboarding flow - #4327
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces separate user creation and invitation wizard screens with embedded onboarding-flow renderers, adds dynamic validation and component rendering, introduces semantic typography colors, updates onboarding resources and translations, removes the standalone invite route, and adjusts end-to-end selectors and diagnostics. ChangesOnboarding flows and typography
Import formatting cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant UserPage
participant InviteUser
participant FlowRenderer
participant BackendFlow
Admin->>UserPage: open create or add-user page
UserPage->>InviteUser: initialize embedded onboarding flow
InviteUser->>FlowRenderer: provide components and render state
FlowRenderer->>Admin: render fields, breadcrumbs, and actions
Admin->>FlowRenderer: enter validated values and submit
FlowRenderer->>InviteUser: invoke flow submit handler
InviteUser->>BackendFlow: submit onboarding action
BackendFlow-->>InviteUser: return next components or error
InviteUser-->>UserPage: update flow state and progress
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/packages/design/src/constants/typographyColorMaps.ts (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or publish the unused typography color map.
COLOR_ENUM_TO_MUI_MAPis only referenced in its own file, while internal flow consumers usemapEmbeddedFlowTextColorinstead. Delete the constant/file, or reuse it as the source for that utility if it’ll be part of the design API.🤖 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/packages/design/src/constants/typographyColorMaps.ts` around lines 24 - 31, Remove the unused COLOR_ENUM_TO_MUI_MAP constant and its file, unless it is intended as part of the design API; in that case, make mapEmbeddedFlowTextColor reuse it and publish the map through the API. Ensure no internal consumer retains a separate duplicate mapping.
🤖 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.
Inline comments:
In `@frontend/packages/configure-users/src/pages/UserAddPage.tsx`:
- Around line 935-942: Update handleManualCreateFallback to navigate via the
existing routes object from useUserRoutes, using routes.add() instead of the
hardcoded '/users/create' path. Include the routes dependency in the callback
dependency list while preserving the existing logging and error handling.
In `@frontend/packages/configure-users/src/pages/UserCreatePage.tsx`:
- Line 193: Update the MenuItem placeholder in UserCreatePage to resolve “Select
an option” through the existing translation function, using the established t
symbol and fallback text instead of the hardcoded string.
- Line 279: The flowError state in UserCreatePage and its error branch in
UserCreateStepContent are currently unreachable. Either connect InviteUser’s
supported onError/onFlowChange callbacks to update flowError, matching
UserAddPage, or remove the unused state and conditional error UI so flow errors
are handled consistently.
- Around line 78-84: Update the auto-submit useEffect associated with
createAction so it submits only once for each resolved action, rather than
re-running when the renderProps object changes identity. Add a ref or equivalent
guard keyed to the current createAction, mark the action as submitted before
invoking handleSubmit, and reset the guard when the action changes or is
unavailable.
---
Nitpick comments:
In `@frontend/packages/design/src/constants/typographyColorMaps.ts`:
- Around line 24-31: Remove the unused COLOR_ENUM_TO_MUI_MAP constant and its
file, unless it is intended as part of the design API; in that case, make
mapEmbeddedFlowTextColor reuse it and publish the map through the API. Ensure no
internal consumer retains a separate duplicate mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e369bdf4-5eb7-46b6-b060-120faf4d7005
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
backend/cmd/server/bootstrap/01-default-resources.yamlfrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsxfrontend/apps/console/src/features/flows/constants/typographyVariantMaps.tsfrontend/apps/console/src/features/flows/models/elements.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/packages/configure-users/src/index.tsfrontend/packages/configure-users/src/pages/UserAddPage.tsxfrontend/packages/configure-users/src/pages/UserCreatePage.tsxfrontend/packages/configure-users/src/pages/UserInvitePage.tsxfrontend/packages/design/src/components/flow/adapters/TextAdapter.tsxfrontend/packages/design/src/constants/typographyColorMaps.tsfrontend/packages/design/src/index.tsfrontend/packages/design/src/themes/DefaultTheme.tsfrontend/packages/design/src/utils/mapEmbeddedFlowTextColor.tsfrontend/packages/i18n/src/locales/en-US.ts
💤 Files with no reviewable changes (3)
- frontend/packages/configure-users/src/pages/UserInvitePage.tsx
- frontend/apps/console/src/App.tsx
- frontend/packages/configure-users/src/index.ts
jeradrutnam
force-pushed
the
main
branch
2 times, most recently
from
July 24, 2026 18:33
cb970e1 to
a208ebf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx (6)
563-573: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertion can't distinguish footer close from header close.
Both the header
IconButtonand the footer button navigate to/users, and the button is picked by DOM order, so this test passes even if the header button is clicked. Scoping the query (e.g. adata-testidor container-scopedwithin(...)query on the footer) would make it actually cover the footer path.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 563 - 573, Update the test around UserAddPage to uniquely target the footer Close button instead of selecting the last matching button by DOM order. Scope the query to the footer container or use its dedicated test identifier, then click that element and preserve the existing /users navigation assertion.
936-951: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test is identical to the OU progress test above; it doesn't exercise nested detection.
Both this and lines 901-915 put
OU_SELECTdirectly inside a singleblock([...]), so the "nested sub-components" path (c.components?.some(sub => sub.type === 'OU_SELECT')inInviteUserFlowBridge) is covered by the earlier test and this adds nothing. Nest the OU select one level deeper to actually cover the second branch.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 936 - 951, Update the “OU detection in nested blocks” test so the OU_SELECT is nested inside a block sub-component rather than placed directly in the outer block. Keep the existing progress-bar assertion, ensuring it exercises InviteUserFlowBridge’s c.components nested-detection branch.
167-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFour near-identical input builders; consider one parameterized factory.
textInput/emailInput/phoneInput/passwordInputdiffer only intypeand the id prefix. These builders are also duplicated inUserCreatePage.test.tsx; extracting them into a shared test fixture module would keep both suites in sync as the embedded-flow component shape evolves.♻️ Parameterized builder
-/** Build a text input component */ -const textInput = ( - ref: string, - label: string, - opts?: {required?: boolean; placeholder?: string; id?: string}, -): EmbeddedFlowComponent => - ({ - type: 'TEXT_INPUT', - ref, - label, - required: opts?.required ?? false, - placeholder: opts?.placeholder ?? '', - id: opts?.id ?? `input-${ref}`, - }) as unknown as EmbeddedFlowComponent +/** Build an input component of the given embedded-flow type */ +const input = ( + type: 'TEXT_INPUT' | 'EMAIL_INPUT' | 'PHONE_INPUT' | 'PASSWORD_INPUT', + idPrefix: string, + ref: string, + label: string, + opts?: {required?: boolean; placeholder?: string; id?: string}, +): EmbeddedFlowComponent => + ({ + type, + ref, + label, + required: opts?.required ?? false, + placeholder: opts?.placeholder ?? '', + id: opts?.id ?? `${idPrefix}-${ref}`, + }) as unknown as EmbeddedFlowComponent; + +const textInput = (ref: string, label: string, opts?: Parameters<typeof input>[4]) => + input('TEXT_INPUT', 'input', ref, label, opts); +const emailInput = (ref: string, label: string, opts?: Parameters<typeof input>[4]) => + input('EMAIL_INPUT', 'email', ref, label, opts); +const phoneInput = (ref: string, label: string, opts?: Parameters<typeof input>[4]) => + input('PHONE_INPUT', 'phone', ref, label, opts); +const passwordInput = (ref: string, label: string, opts?: Parameters<typeof input>[4]) => + input('PASSWORD_INPUT', 'password', ref, label, opts);🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 167 - 224, Replace the duplicated textInput, emailInput, phoneInput, and passwordInput builders with one parameterized factory that accepts the component type and ID prefix while preserving their current defaults and return shape. Extract the shared factory into a test fixture module and update both UserAddPage.test.tsx and UserCreatePage.test.tsx to reuse it.
862-875: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name says "spinner" but only asserts
disabled; the empty-name query is fragile.
getByRole('button', {name: ''})matches any unlabeled button, so this silently drifts as the form gains controls (select dropdown, password toggle), and it never checks that aCircularProgressis present.♻️ Assert the spinner explicitly
render(<UserAddPage />); - // The submit button should contain a CircularProgress - const submitButton = screen.getByRole('button', {name: ''}); - expect(submitButton).toBeDisabled(); + // The submit button should contain a CircularProgress + const spinner = screen + .getAllByRole('progressbar') + .find((el) => el.closest('button') != null); + expect(spinner).toBeDefined(); + expect(spinner?.closest('button')).toBeDisabled();🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 862 - 875, Update the test case around UserAddPage to identify the submit button using its accessible name or another stable submit-specific selector, rather than an empty-name role query. Assert that the button is disabled and explicitly verify that a CircularProgress spinner is rendered while mockInviteUserRenderProps.isLoading is true.
983-995: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertion doesn't cover the stated behavior.
The test claims the first STACK action becomes the primary submit action, but only asserts that a
<form>ancestor exists. Submitting the form (e.g.fireEvent.submit(form)or Enter in a field) and assertingmockHandleSubmitwas called with the first action's identity would match the title.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 983 - 995, Update the test around UserAddPage to submit the form associated with the “Send Email” action, then assert mockHandleSubmit was called using the first STACK action’s identity, act-email. Keep the existing nested STACK setup and validity configuration, but replace the ancestor-existence-only assertion with a behavioral submission assertion.
96-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMock fires
onErroron every render, not once per test.The
InviteUsermock body runs on each render, so withsimulateInviteUserError = trueeach render queues anotheronErrortimer (and laterrerenders add more). Assertions are call-agnostic today, but a futuretoHaveBeenCalledTimescheck will be misleading. Gating on a one-shot flag makes the double-invocation explicit.♻️ One-shot error simulation
InviteUser: ({ children, onError, onFlowChange, }: { children: (props: TestInviteUserRenderProps) => JSX.Element; onError?: (error: Error) => void; onFlowChange?: (response: unknown) => void; }) => { // Capture onFlowChange so tests can invoke it capturedOnFlowChange = onFlowChange ?? null; - if (simulateInviteUserError && onError) { + if (simulateInviteUserError && onError) { + simulateInviteUserError = false; setTimeout(() => { onError(mockInviteUserError); }, 0); }🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 96 - 124, Update the mocked InviteUser component in the useThunderID mock so simulated errors are scheduled only once per test invocation, even across rerenders or React Strict Mode double renders. Add and reset a one-shot guard alongside simulateInviteUserError, and have the onError scheduling logic check and consume that guard before invoking the callback.frontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsx (1)
193-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSeveral tests assert something unrelated to their name.
displays auto-submit behavior when create action is detectedonly checks a progressbar exists,renders with AdditionalData containing rootOuIdonly checks the Close button,handles translation of form labelsrepeats the samegetByLabelText('Email')check asrenders embedded flow components, andrenders email input field(Lines 193-202) duplicates it too. These give coverage credit without covering the named behavior; either assert the real behavior (auto-submit →mockHandleSubmitcalled;rootOuId→ forwarded to the OU picker) or drop them. The repeatedrender(<UserCreateProvider><UserCreatePage /></UserCreateProvider>)block would also be better as a smallrenderPage()helper.Also applies to: 239-269
🤖 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/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsx` around lines 193 - 202, Align the affected UserCreatePage tests with their names: update the auto-submit test to assert mockHandleSubmit is called, verify rootOuId is forwarded to the OU picker, and make the translation test assert translated labels rather than repeating the email query. Remove redundant email/close-button tests if they do not add coverage, and extract repeated provider/page rendering into a small renderPage() helper.
🤖 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.
Inline comments:
In `@frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx`:
- Around line 1053-1064: Update the test case “should not render a BLOCK when
STACK has no submit actions” to assert that the BLOCK’s inner rendered elements
are absent, such as its form, button, or input, rather than relying on
queryByRole('form'). Keep the existing no-submit STACK setup and verify
UserAddPage does not render the component tree.
In
`@frontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsx`:
- Line 66: Update the InviteUser mock render-prop type in
UserCreatePage.test.tsx to use the already imported ReactNode type instead of
React.ReactNode, avoiding reliance on the React UMD global.
- Around line 220-237: Update the “handles form submission” test to assert that
the “Create User” button exists before clicking it, rather than conditionally
skipping the interaction when submitButtons is empty. Keep the click and
mockHandleSubmit expectation unconditional after establishing the button is
present.
In `@frontend/packages/design/src/themes/DefaultTheme.ts`:
- Line 170: Update the radial-gradient declaration in DefaultTheme so the final
color-stop positions are ascending, changing the multi-position stop from 60%
40% to 40% 60% while preserving the existing colors and gradient structure.
In `@frontend/packages/test-utils/src/setup.ts`:
- Around line 172-175: Update the shared mock’s handleSubmit in the setup
configuration to return a resolved Promise<void>, matching the
TestInviteUserRenderProps contract; leave the other handlers unchanged.
---
Nitpick comments:
In `@frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx`:
- Around line 563-573: Update the test around UserAddPage to uniquely target the
footer Close button instead of selecting the last matching button by DOM order.
Scope the query to the footer container or use its dedicated test identifier,
then click that element and preserve the existing /users navigation assertion.
- Around line 936-951: Update the “OU detection in nested blocks” test so the
OU_SELECT is nested inside a block sub-component rather than placed directly in
the outer block. Keep the existing progress-bar assertion, ensuring it exercises
InviteUserFlowBridge’s c.components nested-detection branch.
- Around line 167-224: Replace the duplicated textInput, emailInput, phoneInput,
and passwordInput builders with one parameterized factory that accepts the
component type and ID prefix while preserving their current defaults and return
shape. Extract the shared factory into a test fixture module and update both
UserAddPage.test.tsx and UserCreatePage.test.tsx to reuse it.
- Around line 862-875: Update the test case around UserAddPage to identify the
submit button using its accessible name or another stable submit-specific
selector, rather than an empty-name role query. Assert that the button is
disabled and explicitly verify that a CircularProgress spinner is rendered while
mockInviteUserRenderProps.isLoading is true.
- Around line 983-995: Update the test around UserAddPage to submit the form
associated with the “Send Email” action, then assert mockHandleSubmit was called
using the first STACK action’s identity, act-email. Keep the existing nested
STACK setup and validity configuration, but replace the ancestor-existence-only
assertion with a behavioral submission assertion.
- Around line 96-124: Update the mocked InviteUser component in the useThunderID
mock so simulated errors are scheduled only once per test invocation, even
across rerenders or React Strict Mode double renders. Add and reset a one-shot
guard alongside simulateInviteUserError, and have the onError scheduling logic
check and consume that guard before invoking the callback.
In
`@frontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsx`:
- Around line 193-202: Align the affected UserCreatePage tests with their names:
update the auto-submit test to assert mockHandleSubmit is called, verify
rootOuId is forwarded to the OU picker, and make the translation test assert
translated labels rather than repeating the email query. Remove redundant
email/close-button tests if they do not add coverage, and extract repeated
provider/page rendering into a small renderPage() helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c382abbe-c373-455c-9b42-bf483d97f350
📒 Files selected for processing (28)
backend/cmd/server/bootstrap/01-default-resources.yamldocs/src/theme/MDXComponents.tsxfrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsxfrontend/apps/console/src/features/flows/constants/typographyVariantMaps.tsfrontend/apps/console/src/features/flows/models/elements.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useUpdateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/utils/__tests__/mapConnectionToTrustedIssuer.test.tsfrontend/apps/console/src/features/trusted-issuers/utils/mapConnectionToTrustedIssuer.tsfrontend/packages/configure-users/src/index.tsfrontend/packages/configure-users/src/pages/UserAddPage.tsxfrontend/packages/configure-users/src/pages/UserCreatePage.tsxfrontend/packages/configure-users/src/pages/UserInvitePage.tsxfrontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserInvitePage.test.tsxfrontend/packages/design/src/components/flow/adapters/TextAdapter.tsxfrontend/packages/design/src/constants/typographyColorMaps.tsfrontend/packages/design/src/index.tsfrontend/packages/design/src/themes/DefaultTheme.tsfrontend/packages/design/src/utils/mapEmbeddedFlowTextColor.tsfrontend/packages/i18n/src/locales/en-US.tsfrontend/packages/test-utils/src/setup.ts
💤 Files with no reviewable changes (4)
- frontend/packages/configure-users/src/pages/tests/UserInvitePage.test.tsx
- frontend/apps/console/src/App.tsx
- frontend/packages/configure-users/src/pages/UserInvitePage.tsx
- frontend/packages/configure-users/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- frontend/packages/design/src/constants/typographyColorMaps.ts
- frontend/packages/design/src/index.ts
- frontend/apps/console/src/features/flows/models/elements.ts
- frontend/apps/console/src/features/flows/constants/typographyVariantMaps.ts
- frontend/packages/design/src/components/flow/adapters/TextAdapter.tsx
- frontend/packages/design/src/utils/mapEmbeddedFlowTextColor.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsx
- backend/cmd/server/bootstrap/01-default-resources.yaml
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsx
- frontend/packages/i18n/src/locales/en-US.ts
- frontend/packages/configure-users/src/pages/UserAddPage.tsx
- frontend/packages/configure-users/src/pages/UserCreatePage.tsx
jeradrutnam
force-pushed
the
main
branch
2 times, most recently
from
July 25, 2026 02:58
5c75ff0 to
b3e73b3
Compare
jeradrutnam
added
trigger-pr-builder
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsx (1)
41-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTighten the typography color contract.
color?: stringaccepts unsupported values, which are then silently mapped toundefined. Type this field as the supportedTypographyColorsunion, or validate values at the resource boundary, so invalid colors cannot be persisted without feedback.Also applies to: 76-77
🤖 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/components/resources/elements/adapters/TypographyAdapter.tsx` at line 41, Update the color fields in the typography resource adapters, including the declaration near the visible color property and the additional occurrences around the referenced area, to use the supported TypographyColors union instead of string. Ensure resource-boundary values are typed or validated so unsupported colors cannot be persisted or silently converted to undefined.
🤖 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/components/resources/elements/adapters/TypographyAdapter.tsx`:
- Line 41: Update the color fields in the typography resource adapters,
including the declaration near the visible color property and the additional
occurrences around the referenced area, to use the supported TypographyColors
union instead of string. Ensure resource-boundary values are typed or validated
so unsupported colors cannot be persisted or silently converted to undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48a46b9b-9fad-4b32-9256-f841d7028c40
📒 Files selected for processing (28)
backend/cmd/server/bootstrap/01-default-resources.yamldocs/src/theme/MDXComponents.tsxfrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsxfrontend/apps/console/src/features/flows/constants/typographyVariantMaps.tsfrontend/apps/console/src/features/flows/models/elements.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useUpdateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/utils/__tests__/mapConnectionToTrustedIssuer.test.tsfrontend/apps/console/src/features/trusted-issuers/utils/mapConnectionToTrustedIssuer.tsfrontend/packages/configure-users/src/index.tsfrontend/packages/configure-users/src/pages/UserAddPage.tsxfrontend/packages/configure-users/src/pages/UserCreatePage.tsxfrontend/packages/configure-users/src/pages/UserInvitePage.tsxfrontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserInvitePage.test.tsxfrontend/packages/design/src/components/flow/adapters/TextAdapter.tsxfrontend/packages/design/src/constants/typographyColorMaps.tsfrontend/packages/design/src/index.tsfrontend/packages/design/src/themes/DefaultTheme.tsfrontend/packages/design/src/utils/mapEmbeddedFlowTextColor.tsfrontend/packages/i18n/src/locales/en-US.tsfrontend/packages/test-utils/src/setup.ts
💤 Files with no reviewable changes (4)
- frontend/packages/configure-users/src/index.ts
- frontend/packages/configure-users/src/pages/tests/UserInvitePage.test.tsx
- frontend/apps/console/src/App.tsx
- frontend/packages/configure-users/src/pages/UserInvitePage.tsx
🚧 Files skipped from review as they are similar to previous changes (21)
- frontend/packages/design/src/index.ts
- frontend/packages/design/src/constants/typographyColorMaps.ts
- frontend/apps/console/src/features/trusted-issuers/utils/tests/mapConnectionToTrustedIssuer.test.ts
- frontend/packages/design/src/utils/mapEmbeddedFlowTextColor.ts
- frontend/apps/console/src/features/flows/models/elements.ts
- docs/src/theme/MDXComponents.tsx
- frontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.ts
- frontend/apps/console/src/features/trusted-issuers/api/useTrustedIssuer.ts
- frontend/apps/console/src/features/flows/constants/typographyVariantMaps.ts
- frontend/packages/i18n/src/locales/en-US.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsx
- frontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.ts
- frontend/packages/design/src/themes/DefaultTheme.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- frontend/packages/test-utils/src/setup.ts
- frontend/packages/design/src/components/flow/adapters/TextAdapter.tsx
- frontend/packages/configure-users/src/pages/UserCreatePage.tsx
- frontend/packages/configure-users/src/pages/tests/UserCreatePage.test.tsx
- backend/cmd/server/bootstrap/01-default-resources.yaml
- frontend/packages/configure-users/src/pages/UserAddPage.tsx
- frontend/packages/configure-users/src/pages/tests/UserAddPage.test.tsx
jeradrutnam
added
dependencies-approved
trigger-pr-builder
jeradrutnam
enabled auto-merge
July 25, 2026 03:12
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx (3)
983-995: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest asserts nothing about the primary action.
The name promises "first STACK action is the primary action for form submission", but the body only checks a
<form>ancestor exists. Consider submitting the form (e.g.fireEvent.submit(form)or Enter in a field) and assertingmockHandleSubmitwas called with the first action's payload/id.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 983 - 995, Strengthen the test “should use the first STACK action as the primary action for form submission” by submitting the located form and asserting mockHandleSubmit receives the first nested STACK action’s payload or id, act-email. Keep the existing setup and form lookup, but replace the ancestor-only assertion with a concrete primary-action submission assertion.
862-875: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
getByRole('button', {name: ''})is fragile.This relies on the submit button being the only accessible-name-less button in the tree; any future icon button without a label breaks the query. Prefer querying the submit button by its
type="submit"/ testid and asserting the spinner (progressbar) inside it.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 862 - 875, Replace the fragile empty accessible-name query in the loading-state test with a selector targeting the submit button by its submit type or established test ID. Assert that the selected button is disabled and contains the loading spinner via its progressbar role.
1042-1048: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTautological assertion.
sendEmailButtonis selected because it has thedisabledattribute, soexpect(sendEmailButton).toBeDisabled()can never fail. Target the button by its stable id/testid (act-email) and assert the label is replaced by the spinner instead.🤖 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/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx` around lines 1042 - 1048, Replace the tautological button lookup in the send-email loading test with a direct query for the stable `act-email` id/testid. Assert that this button’s label is replaced by the spinner while the send operation is in progress, rather than selecting it based on its disabled state; retain the disabled assertion only if it independently verifies the intended behavior.tests/e2e/pages/user-management/users.page.ts (1)
217-232: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace fixed
waitForTimeout(500)sleeps with state-based waits.Hard sleeps make the wizard steps both slower and flaky:
isLocatorVisibledoes no waiting, so a step that takes longer than 500ms to mount is silently skipped. Wait for the step's own signal instead (e.g.organizationUnitHeading.first().waitFor({state: 'visible', timeout})inside a tolerant try/catch, or wait for the previous step's heading to detach).🤖 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 `@tests/e2e/pages/user-management/users.page.ts` around lines 217 - 232, The wizard flow in the visible organization-unit and create-user handling uses fixed 500ms sleeps and non-waiting visibility checks, so delayed steps may be skipped. Replace the timeout calls and `isLocatorVisible` checks in this flow with state-based waits on `organizationUnitHeading` and `createUserActionButton`, using the existing visibility timeout and tolerant handling when a step is absent; preserve the continue, click, and transition sequencing.
🤖 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.
Inline comments:
In `@tests/e2e/pages/user-management/users.page.ts`:
- Around line 328-339: Update the timeout error handling in the user-creation
step to detect rendered MUI markup using emitted class names such as
MuiFormControl-root and MuiInputBase-input instead of React component names.
Guard the page.content() call so failures from a closed or unavailable page do
not replace the original timeout error, while preserving the existing diagnostic
error message.
---
Nitpick comments:
In `@frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsx`:
- Around line 983-995: Strengthen the test “should use the first STACK action as
the primary action for form submission” by submitting the located form and
asserting mockHandleSubmit receives the first nested STACK action’s payload or
id, act-email. Keep the existing setup and form lookup, but replace the
ancestor-only assertion with a concrete primary-action submission assertion.
- Around line 862-875: Replace the fragile empty accessible-name query in the
loading-state test with a selector targeting the submit button by its submit
type or established test ID. Assert that the selected button is disabled and
contains the loading spinner via its progressbar role.
- Around line 1042-1048: Replace the tautological button lookup in the
send-email loading test with a direct query for the stable `act-email`
id/testid. Assert that this button’s label is replaced by the spinner while the
send operation is in progress, rather than selecting it based on its disabled
state; retain the disabled assertion only if it independently verifies the
intended behavior.
In `@tests/e2e/pages/user-management/users.page.ts`:
- Around line 217-232: The wizard flow in the visible organization-unit and
create-user handling uses fixed 500ms sleeps and non-waiting visibility checks,
so delayed steps may be skipped. Replace the timeout calls and
`isLocatorVisible` checks in this flow with state-based waits on
`organizationUnitHeading` and `createUserActionButton`, using the existing
visibility timeout and tolerant handling when a step is absent; preserve the
continue, click, and transition sequencing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba44e0bf-b629-446c-8721-e034aa3cb487
📒 Files selected for processing (29)
backend/cmd/server/bootstrap/01-default-resources.yamldocs/src/theme/MDXComponents.tsxfrontend/apps/console/src/App.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsxfrontend/apps/console/src/features/flows/constants/typographyVariantMaps.tsfrontend/apps/console/src/features/flows/models/elements.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/api/useUpdateTrustedIssuer.tsfrontend/apps/console/src/features/trusted-issuers/utils/__tests__/mapConnectionToTrustedIssuer.test.tsfrontend/apps/console/src/features/trusted-issuers/utils/mapConnectionToTrustedIssuer.tsfrontend/packages/configure-users/src/index.tsfrontend/packages/configure-users/src/pages/UserAddPage.tsxfrontend/packages/configure-users/src/pages/UserCreatePage.tsxfrontend/packages/configure-users/src/pages/UserInvitePage.tsxfrontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserCreatePage.test.tsxfrontend/packages/configure-users/src/pages/__tests__/UserInvitePage.test.tsxfrontend/packages/design/src/components/flow/adapters/TextAdapter.tsxfrontend/packages/design/src/constants/typographyColorMaps.tsfrontend/packages/design/src/index.tsfrontend/packages/design/src/themes/DefaultTheme.tsfrontend/packages/design/src/utils/mapEmbeddedFlowTextColor.tsfrontend/packages/i18n/src/locales/en-US.tsfrontend/packages/test-utils/src/setup.tstests/e2e/pages/user-management/users.page.ts
💤 Files with no reviewable changes (4)
- frontend/packages/configure-users/src/index.ts
- frontend/apps/console/src/App.tsx
- frontend/packages/configure-users/src/pages/UserInvitePage.tsx
- frontend/packages/configure-users/src/pages/tests/UserInvitePage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- frontend/packages/design/src/index.ts
- frontend/packages/design/src/constants/typographyColorMaps.ts
- docs/src/theme/MDXComponents.tsx
- frontend/packages/design/src/utils/mapEmbeddedFlowTextColor.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsx
- frontend/apps/console/src/features/flows/models/elements.ts
- frontend/apps/console/src/features/flows/constants/typographyVariantMaps.ts
- frontend/apps/console/src/features/trusted-issuers/api/useUpdateTrustedIssuer.ts
- frontend/packages/design/src/components/flow/adapters/TextAdapter.tsx
- frontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.ts
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsx
- frontend/apps/console/src/features/trusted-issuers/utils/tests/mapConnectionToTrustedIssuer.test.ts
- frontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.ts
- frontend/packages/configure-users/src/pages/tests/UserCreatePage.test.tsx
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- backend/cmd/server/bootstrap/01-default-resources.yaml
- frontend/packages/configure-users/src/pages/UserCreatePage.tsx
- frontend/packages/i18n/src/locales/en-US.ts
- frontend/packages/test-utils/src/setup.ts
- frontend/packages/configure-users/src/pages/UserAddPage.tsx
Comment on lines
+328
to
+339
| } catch (error) { | ||
| // Provide debug information about what's actually on the page | ||
| const pageContent = await this.page.content(); | ||
| const hasInputs = pageContent.includes('<input'); | ||
| const hasFormControl = pageContent.includes('FormControl'); | ||
| const hasTextField = pageContent.includes('TextField'); | ||
|
|
||
| throw new Error( | ||
| `Timed out after ${timeout}ms while waiting for the next visible user-creation step. ` + | ||
| `Debug: hasInputs=${hasInputs}, hasFormControl=${hasFormControl}, hasTextField=${hasTextField}. ` + | ||
| `Error: ${error instanceof Error ? error.message : String(error)}` | ||
| ); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
hasFormControl / hasTextField will be false in virtually every failure.
page.content() returns rendered HTML; MUI never emits the React component names FormControl/TextField there (it emits classes like MuiFormControl-root, MuiInputBase-input). These indicators are therefore misleading noise. Match on the emitted class names instead, and guard the content() call so a closed page doesn't mask the original timeout.
🐛 Proposed fix for the debug indicators
} catch (error) {
// Provide debug information about what's actually on the page
- const pageContent = await this.page.content();
- const hasInputs = pageContent.includes('<input');
- const hasFormControl = pageContent.includes('FormControl');
- const hasTextField = pageContent.includes('TextField');
+ const pageContent = await this.page.content().catch(() => "");
+ const hasInputs = pageContent.includes('<input');
+ const hasFormControl = pageContent.includes('MuiFormControl-root');
+ const hasTextField = pageContent.includes('MuiInputBase-input');📝 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
| } catch (error) { | |
| // Provide debug information about what's actually on the page | |
| const pageContent = await this.page.content(); | |
| const hasInputs = pageContent.includes('<input'); | |
| const hasFormControl = pageContent.includes('FormControl'); | |
| const hasTextField = pageContent.includes('TextField'); | |
| throw new Error( | |
| `Timed out after ${timeout}ms while waiting for the next visible user-creation step. ` + | |
| `Debug: hasInputs=${hasInputs}, hasFormControl=${hasFormControl}, hasTextField=${hasTextField}. ` + | |
| `Error: ${error instanceof Error ? error.message : String(error)}` | |
| ); | |
| } catch (error) { | |
| // Provide debug information about what's actually on the page | |
| const pageContent = await this.page.content().catch(() => ""); | |
| const hasInputs = pageContent.includes('<input'); | |
| const hasFormControl = pageContent.includes('MuiFormControl-root'); | |
| const hasTextField = pageContent.includes('MuiInputBase-input'); | |
| throw new Error( | |
| `Timed out after ${timeout}ms while waiting for the next visible user-creation step. ` + | |
| `Debug: hasInputs=${hasInputs}, hasFormControl=${hasFormControl}, hasTextField=${hasTextField}. ` + | |
| `Error: ${error instanceof Error ? error.message : String(error)}` | |
| ); |
🤖 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 `@tests/e2e/pages/user-management/users.page.ts` around lines 328 - 339, Update
the timeout error handling in the user-creation step to detect rendered MUI
markup using emitted class names such as MuiFormControl-root and
MuiInputBase-input instead of React component names. Guard the page.content()
call so failures from a closed or unavailable page do not replace the original
timeout error, while preserving the existing diagnostic error message.
DonOmalVindula
approved these changes
Jul 26, 2026
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
2 participants
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.
This pull request introduces several improvements and refactors across the codebase, primarily focused on enhancing the customization and maintainability of text component colors in the flow builder, cleaning up resource definitions, and simplifying imports. The most notable changes are the addition of a color property for text components, updates to resource property panels to support color selection, and the removal of redundant icon components from resource YAML files.
Flow builder enhancements:
colorproperty to text (Typography) components, including theTypographyColorsenum and mapping logic to Material UI colors, enabling color customization for text elements in flows. (frontend/apps/console/src/features/flows/models/elements.ts[1]frontend/apps/console/src/features/flows/constants/typographyVariantMaps.ts[2] [3]frontend/apps/console/src/features/flows/components/resources/elements/adapters/TypographyAdapter.tsx[4] [5] [6] [7]ColorSelectcomponent for color selection in the resource property panel, and integrated it into the text resource property UI. (frontend/apps/console/src/features/flows/components/resource-property-panel/ColorSelect.tsx[1]frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx[2] [3]Resource YAML cleanup:
TEXTcomponents (with checkmark or warning icons) from various prompt resource definitions and replaced them with acolorproperty on the heading text, streamlining the UI configuration. (backend/cmd/server/bootstrap/01-default-resources.yaml[1] [2] [3] [4] [5]Codebase maintenance and refactoring:
frontend/apps/console/src/features/trusted-issuers/api/useCreateTrustedIssuer.ts[1]frontend/apps/console/src/features/trusted-issuers/api/useDeleteTrustedIssuer.ts[2]frontend/apps/console/src/features/trusted-issuers/api/useTrustedIssuer.ts[3]frontend/apps/console/src/features/trusted-issuers/api/useUpdateTrustedIssuer.ts[4]frontend/apps/console/src/features/trusted-issuers/utils/__tests__/mapConnectionToTrustedIssuer.test.ts[5]frontend/apps/console/src/features/trusted-issuers/utils/mapConnectionToTrustedIssuer.ts[6]docs/src/theme/MDXComponents.tsxdocs/src/theme/MDXComponents.tsxL20-R40)Routing and navigation:
UserInvitePageroute and its associated dynamic import from the console app, likely as part of a feature deprecation or redesign. (frontend/apps/console/src/App.tsx[1] [2]Summary by CodeRabbit
New Features
Bug Fixes
Style