Report an unresolvable user onboarding flow instead of redirecting - #5419
Dilusha-Madushan wants to merge 1 commit into
Conversation
Signed-off-by: Dilusha-Madushan <dilushamadushan9912@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (8)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe manual user creation page and route were removed. UserAddPage now handles missing onboarding flows by stopping the wizard and rendering an error message. Route definitions, exports, tests, and English translations were updated. ChangesUser creation flow
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant InviteUser
participant UserAddPage
participant ErrorAlert
InviteUser->>UserAddPage: report missing onboarding flow
UserAddPage->>UserAddPage: set isFlowUnavailable and log error
UserAddPage->>ErrorAlert: render unavailable-flow message
Merge Risk: ⚪ Minimal · up to The removed manual-creation entry points have no remaining repository consumers, and the retained onboarding route handles unavailable flows with the intended error state. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
frontend/packages/configure-users/src/pages/UserAddPage.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. frontend/packages/configure-users/src/pages/__tests__/UserAddPage.test.tsxESLint skipped: the matched ESLint configuration already failed (missing-dependency). frontend/packages/i18n/src/locales/en-US.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration 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 |
this is how error shows now the flow is missing. |
1 participant
Purpose
When the user onboarding flow cannot be resolved, the Add User page navigated to a "manual user creation" page. That page executed the same onboarding flow that had just been reported missing, and carried no handling for that failure.
The result was that a missing flow produced a silent redirect followed by a second, less specific failure. The fallback made the outcome worse than no fallback at all.
Three checks established that the fallback could never have worked:
UserCreatePagecontains no user-creation call. NouseCreateUser, no POST to/users, nomutation. It renders the same
InviteUsercomponent, which pinsflowType: USER_ONBOARDING.UserCreatePagecontains no missing-flow handling.isMissingOnboardingFlowappears zero times.UserCreatePagewas reachable only from this fallback. The Add User button callsroutes.add();routes.addCreate()had exactly one caller.Approach
Report the condition in place rather than navigating away. The Add User page now sets an unavailable state when the flow cannot be resolved, and returns early with a message naming the two things to check: that a user onboarding flow is configured, and that the flow it names exists.
The early return replaces the wizard rather than overlaying it, so no step remains interactive behind the message. A test asserts the step heading is gone, not merely covered.
One state rather than two. The agent onboarding page distinguishes
NOT_CONFIGUREDfromFLOW_MISSING, because it resolves the configured handle itself before starting. The Add User page cannot: it detects the condition reactively from the flow execution error, and the backend returns a single code,FLM-1003"Flow not found", carrying no cause. Splitting the message would mean adding a server-config fetch to this page, which is new data-fetching behaviour rather than a fix, so the message covers both causes instead.Detection is unchanged.
isMissingOnboardingFlowalready recognised the condition at both theonErrorandonFlowChangecall sites. Only the response to it changed, so the matching logic keeps its existing coverage.Removal.
UserCreatePageand its test are deleted, along with theindex.tsexport, the lazy import and route inApp.tsx, andaddCreatefromRouteConfig.tsanduseUserRoutes.ts. Two stale references were cleaned up in passing:App.test.tsxmocked../features/users/pages/UserCreatePage, a path that stopped existing when the pages moved into the package, andRouteConfig.test.tsasserted the now-removed route.Tests. Two existing tests asserted the redirect and were rewritten. Two were added: that the wizard is replaced rather than rendered alongside the message, and that an error which is not a missing flow leaves the wizard intact so a recoverable failure stays recoverable. The new tests were verified by mutation: removing the state assignment fails three of the four.
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Bug Fixes
Changes
Documentation