fix: allow backward navigation in setup wizard (#38801)#38925
Open
Subhooo5 wants to merge 8 commits intoRocketChat:developfrom
Open
fix: allow backward navigation in setup wizard (#38801)#38925Subhooo5 wants to merge 8 commits intoRocketChat:developfrom
Subhooo5 wants to merge 8 commits intoRocketChat:developfrom
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 0bb3767 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
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:
WalkthroughRegisterUsername adds a Back button that uses the router to navigate back. useRedirectToSetupWizard now reads the current pathname and only navigates to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts`:
- Line 16: Remove the inline comment "//DO NOT Redirect if already inside setup
wizard (Fixes navigation)" from the useRedirectToSetupWizard hook
implementation; locate the comment inside the useRedirectToSetupWizard function
(or the surrounding hook file) and delete that line so the logic remains
unchanged but the file complies with the "no code comments" guideline.
In `@apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx`:
- Line 131: Replace the hardcoded "Back" label in the RegisterUsername
component's JSX with the i18n translation call used elsewhere (e.g., change the
literal "Back" to {t('Back')} or the appropriate translation key) so it uses the
same t(...) function already present in this file; locate the button/link
element in RegisterUsername.tsx and update its label to use t('Back') to
maintain consistency with other user-facing strings.
- Line 14: Remove the inline debug/authoring comments in RegisterUsername.tsx:
delete the trailing comment after the useRouter import (the "// ✅ ADDED" next to
useRouter), remove the similar inline comment at the other import/line
referenced (line with the second "// ✅ ADDED"), and remove the JSX comment
around the back button ("{/* ✅ BACK BUTTON ADDED */}") in the RegisterUsername
component; keep the imports and JSX structure unchanged, only delete those
comment tokens so no inline comments remain.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/client/views/root/MainLayout/RegisterUsername.tsxapps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.tsapps/meteor/client/views/root/MainLayout/RegisterUsername.tsx
🧠 Learnings (1)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts (1)
packages/ui-contexts/src/index.ts (1)
useRole(59-59)
apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx (1)
packages/ui-contexts/src/index.ts (1)
useRouter(63-63)
🔇 Additional comments (1)
apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx (1)
130-130: No action required –router.navigate(-1)is correctly typed.The
RouterContextValuetype in@rocket.chat/ui-contexts(packages/ui-contexts/src/RouterContext.ts) includes the numeric delta overload:navigate(delta: number): void. This allowsrouter.navigate(-1)to work as intended for back navigation. No type errors or runtime issues.
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
Outdated
Show resolved
Hide resolved
Subhooo5
force-pushed
the
fix/setup-wizard-back-navigation
branch
from
fff11fe to
5d2fce3
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts (1)
16-16: Inline comment still present — violates the "no code comments" guideline.As per coding guidelines, avoid code comments in the implementation for
*.tsfiles.♻️ Proposed fix
- //DO NOT Redirect if already inside setup wizard (Fixes navigation) if (mustRedirect && !currentPath?.startsWith('/setup-wizard')) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts` at line 16, Remove the inline code comment "//DO NOT Redirect if already inside setup wizard (Fixes navigation)" from the useRedirectToSetupWizard hook implementation; locate the comment in the function useRedirectToSetupWizard in the file and delete it (do not replace with another inline comment)—if a note is required for future readers, add a short descriptive identifier or extract logic into a well-named helper function instead (e.g., isInSetupWizard) so the code is self-documenting.
🧹 Nitpick comments (1)
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts (1)
15-17: Remove optional chaining oncurrentPath.
router.getLocationPathname()returnsLocationPathname, a non-nullable string type. The optional chaining incurrentPath?.startsWith(...)is unnecessary. Change tocurrentPath.startsWith(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts` around lines 15 - 17, In useRedirectToSetupWizard, remove the unnecessary optional chaining on currentPath: change the conditional that uses currentPath?.startsWith('/setup-wizard') to use currentPath.startsWith('/setup-wizard') instead, since router.getLocationPathname() returns a non-nullable LocationPathname; update the if statement that checks mustRedirect and currentPath to reference currentPath.startsWith(...) so the logic remains identical but without the redundant `?.`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts`:
- Line 16: Remove the inline code comment "//DO NOT Redirect if already inside
setup wizard (Fixes navigation)" from the useRedirectToSetupWizard hook
implementation; locate the comment in the function useRedirectToSetupWizard in
the file and delete it (do not replace with another inline comment)—if a note is
required for future readers, add a short descriptive identifier or extract logic
into a well-named helper function instead (e.g., isInSetupWizard) so the code is
self-documenting.
---
Nitpick comments:
In `@apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts`:
- Around line 15-17: In useRedirectToSetupWizard, remove the unnecessary
optional chaining on currentPath: change the conditional that uses
currentPath?.startsWith('/setup-wizard') to use
currentPath.startsWith('/setup-wizard') instead, since
router.getLocationPathname() returns a non-nullable LocationPathname; update the
if statement that checks mustRedirect and currentPath to reference
currentPath.startsWith(...) so the logic remains identical but without the
redundant `?.`.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/client/views/root/MainLayout/RegisterUsername.tsxapps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts
🧬 Code graph analysis (1)
apps/meteor/client/views/root/hooks/useRedirectToSetupWizard.ts (1)
packages/ui-contexts/src/index.ts (1)
useRole(59-59)
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.
Proposed changes
This PR fixes the backward navigation issue in the Organization / Workspace Registration Setup Wizard where users were unable to return to previous steps either through:
🔙 Browser back button
🔗 Manual URL navigation
The issue occurred because the useRedirectToSetupWizard hook was enforcing a redirect to /setup-wizard even when the user was already inside a setup wizard route. This resulted in users being automatically pushed forward to the required step, thereby preventing backward navigation between onboarding steps.
Changeset
Adjusts the setup wizard redirection logic to prevent forced navigation to /setup-wizard when the user is already inside a setup wizard route to allow backward navigation between onboarding steps.
Issue(s)
Closes: ➡️ #38801
Steps to test or reproduce
🔁 Reproduce the Issue (Before Fix)
1️⃣ Stop the running Rocket.Chat server: Ctrl + C
2️⃣ Clear the local Meteor database: rm -rf .meteor/local
3️⃣ Start the application again: yarn dev(more than or equal to 16GB memory), otherwise : yarn dsv
4️⃣ In a new terminal, navigate to the Meteor app directory:
cd apps/meteor
meteor shell
5️⃣ Force reset the Setup Wizard state:
_MongoInternals.defaultRemoteCollectionDriver().mongo.db.collection("rocketchat_settings").updateOne({ id: "Show_Setup_Wizard" }, { $set: { value: "pending" } });
6️⃣ Exit the Meteor shell: .exit
7️⃣ Restart the server:
cd ../..
yarn dev
8️⃣ Open the Setup Wizard manually: http://localhost:3000/setup-wizard
Behaviour Before Fix
Navigate forward to any setup wizard step
Try to manually go back via URL: /setup-wizard/server → /setup-wizard/organization
The application automatically redirects the user back to the next required step, preventing backward navigation.
Behaviour After Fix
-Forward navigation skipping is still restricted
-Backward navigation between steps is now allowed
-Manual URL change to previous setup steps works correctly
-Browser back button works as expected
Summary by CodeRabbit
New Features
Bug Fixes
Documentation