Show user onboarding wizard based on claim, and give option to re-access it#9669
Open
savindi7 wants to merge 7 commits intowso2:masterfrom
Open
Show user onboarding wizard based on claim, and give option to re-access it#9669savindi7 wants to merge 7 commits intowso2:masterfrom
savindi7 wants to merge 7 commits intowso2:masterfrom
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:
📝 WalkthroughWalkthroughReplaces local/app-list gating with a server-driven SCIM claim for onboarding visibility, removes select governance/login scopes, adds an Onboarding FAB (component, hook, and public export) across admin pages, makes onboarding callbacks async, and adds eligibility checks (account/org type, feature flag, scopes). Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/admin.onboarding.v1/components/onboarding-wizard.tsx (1)
403-408:⚠️ Potential issue | 🟠 MajorHandle rejected
onComplete/onSkippromises with user feedback.Both awaited callbacks can reject and currently have no local error handling, which can result in silent failures.
🛠️ Proposed fix
const handleNext: () => Promise<void> = useCallback(async (): Promise<void> => { const nextStep: OnboardingStep = getNextStep(currentStep, onboardingData); if (currentStep === OnboardingStep.SUCCESS) { - await onComplete(onboardingData); + try { + await onComplete(onboardingData); + } catch (error: unknown) { + const errorMessage: string = error instanceof Error + ? error.message + : "Failed to complete onboarding. Please try again."; + + dispatch(addAlert({ + description: errorMessage, + level: AlertLevels.ERROR, + message: "Onboarding Completion Failed" + })); + } + + return; } else if ( currentStep === OnboardingStep.DESIGN_LOGIN || (currentStep === OnboardingStep.SELECT_APPLICATION_TEMPLATE && isM2M) ) { setDirection("forward"); await createApplication(); } else { setDirection("forward"); setCurrentStep(nextStep); } - }, [ currentStep, onboardingData, isM2M, createApplication, onComplete ]); + }, [ currentStep, onboardingData, isM2M, createApplication, onComplete, dispatch ]); const handleSkip: () => Promise<void> = useCallback(async (): Promise<void> => { - await onSkip(); - }, [ onSkip ]); + try { + await onSkip(); + } catch (error: unknown) { + const errorMessage: string = error instanceof Error + ? error.message + : "Failed to skip onboarding. Please try again."; + + dispatch(addAlert({ + description: errorMessage, + level: AlertLevels.ERROR, + message: "Skip Failed" + })); + } + }, [ onSkip, dispatch ]);As per coding guidelines "Dispatch error notifications via Redux with
dispatch(addAlert({ description, level: AlertLevels.ERROR, message }))in catch blocks."Also applies to: 430-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.onboarding.v1/components/onboarding-wizard.tsx` around lines 403 - 408, The awaited calls to onComplete and onSkip in handleNext (the async callback defined as handleNext) can reject and currently lack error handling; wrap each await (the onComplete(onboardingData) branch and the onSkip() branch) in try/catch blocks and in the catch dispatch an error alert using dispatch(addAlert({ description: err?.message || String(err), level: AlertLevels.ERROR, message: 'Onboarding error' })); ensure dispatch and AlertLevels are imported/available in this component and do not swallow errors silently (return/exit the handler after dispatching the alert as appropriate).
🧹 Nitpick comments (1)
features/admin.onboarding.v1/pages/onboarding-page.tsx (1)
89-89: Use the standard feature-status helper for onboarding enablement checks.Line 89 uses a raw boolean read; switching to
isFeatureEnabled(...)keeps this consistent with feature status handling conventions.♻️ Suggested refactor
-import { useRequiredScopes } from "@wso2is/access-control"; +import { isFeatureEnabled, useRequiredScopes } from "@wso2is/access-control"; @@ - const isFeatureEnabled: boolean = !!featureConfig?.onboarding?.enabled; + const isOnboardingFeatureEnabled: boolean = isFeatureEnabled(featureConfig, "onboarding"); @@ - if (!isFeatureEnabled || !hasRequiredCreateScopes) { + if (!isOnboardingFeatureEnabled || !hasRequiredCreateScopes) { history.push(AppConstants.getAppHomePath()); @@ - }, [ isLoading, isFeatureEnabled, shouldShowOnboarding, hasRequiredCreateScopes, isIntentionalAccess ]); + }, [ isLoading, isOnboardingFeatureEnabled, shouldShowOnboarding, hasRequiredCreateScopes, isIntentionalAccess ]); @@ - if (!isFeatureEnabled || !hasRequiredCreateScopes) { + if (!isOnboardingFeatureEnabled || !hasRequiredCreateScopes) { return null; }As per coding guidelines, use
isFeatureEnabledfrom@wso2is/access-controlto evaluate feature enablement.Also applies to: 98-99, 107-107, 133-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.onboarding.v1/pages/onboarding-page.tsx` at line 89, Replace the raw boolean checks against featureConfig?.onboarding?.enabled with the central helper from `@wso2is/access-control`: import isFeatureEnabled and call it to evaluate onboarding enablement (e.g., const onboardingEnabled = isFeatureEnabled(featureConfig, "onboarding")), update all usages that currently read featureConfig?.onboarding?.enabled (and avoid shadowing the imported name by renaming the local variable from isFeatureEnabled to onboardingEnabled), and use onboardingEnabled wherever the file currently performs the raw checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.onboarding.v1/api/get-onboarding-claim.ts`:
- Around line 61-62: The current boolean expression using
!systemSchemaData?.[SHOW_ONBOARDING_WIZARD_ATTRIBUTE] incorrectly treats false
as true; update the claim evaluation so the onboarding wizard is considered
visible only when the stored attribute is not explicitly false. Replace the
expression in the get-onboarding-claim logic that references systemSchemaData
and SHOW_ONBOARDING_WIZARD_ATTRIBUTE with an explicit check such as
systemSchemaData?.[SHOW_ONBOARDING_WIZARD_ATTRIBUTE] !== false (or equivalent)
so false correctly resolves to hiding the wizard.
In `@features/admin.onboarding.v1/components/shared/onboarding-fab.tsx`:
- Around line 110-112: Replace the hardcoded "Setup Guide" label inside the
StyledFab (next to Rocket) with an i18next translation lookup; import and call
useTranslation from react-i18next in the component (e.g., const { t } =
useTranslation()) and render t('<namespace>:<key.path>') instead of the literal
string (for example t('admin:onboarding.setupGuide')); ensure the chosen key
follows the <namespace>:<key.path> format and that the translation entry is
added to your locale files.
- Around line 55-67: The pulseGlow keyframes currently hardcode box-shadow
colors and geometry; update the animation to use theme tokens instead: replace
hardcoded rgba(...) color with the appropriate theme color token (e.g.,
theme.colors.primary or theme.colors.accent/amber) and replace numeric shadow
values with theme.shadows (or computed values derived from theme.spacing/size
tokens) so the glow and shadow follow branding/mode changes; apply the same
tokenization refactor to the other keyframe/box-shadow block referenced in this
file (the second animation near the same component) and ensure imports/access to
the theme (styled-system/theme-ui/ThemeContext) are used where pulseGlow is
defined.
In `@features/admin.onboarding.v1/components/steps/welcome-step.tsx`:
- Around line 113-120: The JSX in welcome-step.tsx hardcodes user-facing strings
for the subtitle and title (the subtitle prop and title prop conditionals using
isReturningUser and greeting); extract these literal strings into i18n keys and
replace the hardcoded text with translations via `@wso2is/i18n` (use
namespace:path.to.key format), adding keys for both returning and new-user
variants and interpolations for greeting, then call t('namespace:key', {
greeting }) (or equivalent) where subtitle and title are set so all text goes
through the i18n system.
In `@features/admin.onboarding.v1/hooks/use-onboarding-fab-visibility.ts`:
- Around line 52-54: The call to useRequiredScopes is currently masking possible
undefined with an unsafe cast ("onboardingFeatureConfig?.scopes?.create as
string[]"); change it to pass a safe default array so the hook receives a
string[] at compile time (e.g.,
useRequiredScopes(onboardingFeatureConfig?.scopes?.create ?? [])); update the
expression that sets hasRequiredCreateScopes to use this default instead of the
as-cast so TypeScript knows the value cannot be undefined while preserving
runtime behavior of useRequiredScopes.
---
Outside diff comments:
In `@features/admin.onboarding.v1/components/onboarding-wizard.tsx`:
- Around line 403-408: The awaited calls to onComplete and onSkip in handleNext
(the async callback defined as handleNext) can reject and currently lack error
handling; wrap each await (the onComplete(onboardingData) branch and the
onSkip() branch) in try/catch blocks and in the catch dispatch an error alert
using dispatch(addAlert({ description: err?.message || String(err), level:
AlertLevels.ERROR, message: 'Onboarding error' })); ensure dispatch and
AlertLevels are imported/available in this component and do not swallow errors
silently (return/exit the handler after dispatching the alert as appropriate).
---
Nitpick comments:
In `@features/admin.onboarding.v1/pages/onboarding-page.tsx`:
- Line 89: Replace the raw boolean checks against
featureConfig?.onboarding?.enabled with the central helper from
`@wso2is/access-control`: import isFeatureEnabled and call it to evaluate
onboarding enablement (e.g., const onboardingEnabled =
isFeatureEnabled(featureConfig, "onboarding")), update all usages that currently
read featureConfig?.onboarding?.enabled (and avoid shadowing the imported name
by renaming the local variable from isFeatureEnabled to onboardingEnabled), and
use onboardingEnabled wherever the file currently performs the raw checks.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/console/src/public/deployment.config.jsonfeatures/admin.applications.v1/package.jsonfeatures/admin.applications.v1/pages/applications.tsxfeatures/admin.home.v1/package.jsonfeatures/admin.home.v1/pages/home-page.tsxfeatures/admin.onboarding.v1/api/get-onboarding-claim.tsfeatures/admin.onboarding.v1/api/update-onboarding-claim.tsfeatures/admin.onboarding.v1/components/onboarding-wizard.tsxfeatures/admin.onboarding.v1/components/shared/onboarding-fab.tsxfeatures/admin.onboarding.v1/components/steps/welcome-step.tsxfeatures/admin.onboarding.v1/constants/component-ids.tsfeatures/admin.onboarding.v1/hooks/use-onboarding-fab-visibility.tsfeatures/admin.onboarding.v1/hooks/use-onboarding-status.tsfeatures/admin.onboarding.v1/pages/onboarding-page.tsxfeatures/admin.onboarding.v1/public-api.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
features/admin.onboarding.v1/hooks/use-onboarding-status.ts (2)
53-64: TypeuseSelectorcalls explicitly instead of relying on inferred selector return types.Current selectors depend on inferred callback returns; the repo rules require explicit hook typing as well.
As per coding guidelines: "Hooks must use explicit generics on `useState`, `useRef`, and explicit types on `useSelector`."Example typed selectors
-const uuid: string = useSelector((state: AppState) => state.profile.profileInfo.id); +const uuid: string = useSelector<AppState, string>( + (state: AppState): string => state.profile.profileInfo.id +); -const profileInfo: ProfileInfoInterface = useSelector( - (state: AppState) => state.profile.profileInfo -); +const profileInfo: ProfileInfoInterface = useSelector<AppState, ProfileInfoInterface>( + (state: AppState): ProfileInfoInterface => state.profile.profileInfo +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.onboarding.v1/hooks/use-onboarding-status.ts` around lines 53 - 64, Selectors in this file rely on inferred return types from useSelector; update each useSelector call (for uuid, username, profileInfo, organizationType, featureConfig) to pass explicit generic type arguments to useSelector (e.g., useSelector<AppState, string> or the appropriate interface type) so the hook return types are explicit; identify the calls by the variable names uuid, username, profileInfo, userAccountType (derived from profileInfo), organizationType, and featureConfig and replace their current useSelector invocations with explicitly typed generics matching AppState and the expected return type.
33-37: Rename hook return interface to follow repository naming rules.
UseOnboardingStatusReturnshould use theInterfacesuffix.As per coding guidelines: "Interface names must use the `Interface` suffix."Proposed rename
-interface UseOnboardingStatusReturn { +interface UseOnboardingStatusReturnInterface { isLoading: boolean; markOnboardingComplete: () => Promise<void>; shouldShowOnboarding: boolean; } -export const useOnboardingStatus = (): UseOnboardingStatusReturn => { +export const useOnboardingStatus = (): UseOnboardingStatusReturnInterface => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.onboarding.v1/hooks/use-onboarding-status.ts` around lines 33 - 37, The interface name UseOnboardingStatusReturn violates the naming rule—rename it to UseOnboardingStatusInterface and update all references: change the declaration interface UseOnboardingStatusReturn { ... } to interface UseOnboardingStatusInterface { isLoading: boolean; markOnboardingComplete: () => Promise<void>; shouldShowOnboarding: boolean; } and update any type annotations, function return types, exports, and imports that reference UseOnboardingStatusReturn (e.g., the hook that returns that interface and any consumers using isLoading, markOnboardingComplete, or shouldShowOnboarding) to use UseOnboardingStatusInterface instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.onboarding.v1/hooks/use-onboarding-status.ts`:
- Around line 112-137: In both evaluateScimClaim and markOnboardingComplete
replace silent failure handling with Redux error alerts: import useDispatch from
react-redux and addAlert and AlertLevels from
`@wso2is/admin.core.v1/hooks/use-snackbar`, create a dispatch via useDispatch(),
and in evaluateScimClaim's catch dispatch addAlert({ description: (include error
message or generic text), level: AlertLevels.ERROR, message: "Failed to evaluate
onboarding claim" }) instead of setShouldShowOnboarding(false); likewise in
markOnboardingComplete's catch dispatch addAlert({ description: (include error
message or generic text), level: AlertLevels.ERROR, message: "Failed to update
onboarding wizard claim" }) instead of console.warn, keeping existing behavior
(e.g., proceed with navigation) after dispatch.
---
Nitpick comments:
In `@features/admin.onboarding.v1/hooks/use-onboarding-status.ts`:
- Around line 53-64: Selectors in this file rely on inferred return types from
useSelector; update each useSelector call (for uuid, username, profileInfo,
organizationType, featureConfig) to pass explicit generic type arguments to
useSelector (e.g., useSelector<AppState, string> or the appropriate interface
type) so the hook return types are explicit; identify the calls by the variable
names uuid, username, profileInfo, userAccountType (derived from profileInfo),
organizationType, and featureConfig and replace their current useSelector
invocations with explicitly typed generics matching AppState and the expected
return type.
- Around line 33-37: The interface name UseOnboardingStatusReturn violates the
naming rule—rename it to UseOnboardingStatusInterface and update all references:
change the declaration interface UseOnboardingStatusReturn { ... } to interface
UseOnboardingStatusInterface { isLoading: boolean; markOnboardingComplete: () =>
Promise<void>; shouldShowOnboarding: boolean; } and update any type annotations,
function return types, exports, and imports that reference
UseOnboardingStatusReturn (e.g., the hook that returns that interface and any
consumers using isLoading, markOnboardingComplete, or shouldShowOnboarding) to
use UseOnboardingStatusInterface instead.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
features/admin.onboarding.v1/api/get-onboarding-claim.tsfeatures/admin.onboarding.v1/hooks/use-onboarding-status.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- features/admin.onboarding.v1/api/get-onboarding-claim.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/admin.applications.v1/pages/applications.tsx (1)
796-796: Consider passingdata-componentidfor testability.The
OnboardingFabis rendered without props. If the component acceptsIdentifiableComponentInterface, consider passing adata-componentidto maintain hierarchical component identification for testing.♻️ Suggested change
- <OnboardingFab /> + <OnboardingFab data-componentid="applications-onboarding-fab" />Based on learnings: "In the identity-apps repository, prefer using the data-componentid attribute for component identification in tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.applications.v1/pages/applications.tsx` at line 796, OnboardingFab is rendered without a data-componentid which reduces testability; update the JSX where OnboardingFab is instantiated in applications.tsx to pass a data-componentid prop (e.g. data-componentid="ApplicationsPage.OnboardingFab") assuming OnboardingFab implements IdentifiableComponentInterface (or add that prop to its props/type if missing), so tests can reliably target the component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@features/admin.applications.v1/pages/applications.tsx`:
- Line 796: OnboardingFab is rendered without a data-componentid which reduces
testability; update the JSX where OnboardingFab is instantiated in
applications.tsx to pass a data-componentid prop (e.g.
data-componentid="ApplicationsPage.OnboardingFab") assuming OnboardingFab
implements IdentifiableComponentInterface (or add that prop to its props/type if
missing), so tests can reliably target the component.
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
3 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.
Purpose
userUserOnboardingWizardclaim.Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.