-
Notifications
You must be signed in to change notification settings - Fork 7.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: user homePath no effect sometimes #5166
Conversation
|
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
apps/backend-mock/utils/mock-data.tsOops! Something went wrong! :( ESLint: 9.16.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request introduces modifications to handle user-specific home paths across multiple router guard implementations. The changes primarily focus on enhancing the redirection logic when users log in or navigate between routes. A new optional Changes
Assessment against linked issues
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/backend-mock/utils/mock-data.ts
(2 hunks)apps/web-antd/src/router/guard.ts
(3 hunks)apps/web-ele/src/router/guard.ts
(3 hunks)apps/web-naive/src/router/guard.ts
(3 hunks)playground/src/router/guard.ts
(3 hunks)
🔇 Additional comments (9)
playground/src/router/guard.ts (3)
55-57
: LGTM: Login redirection logic properly prioritizes user's home path
The implementation correctly prioritizes:
- Redirect query parameter
- User's custom home path
- Default home path as fallback
75-78
: LGTM: Query parameter handling prevents redirect loops
The implementation correctly omits the redirect query when accessing the default home path, preventing potential redirect loops.
108-111
: Verify edge case: undefined userInfo
The logic looks correct, but let's verify the edge case where userInfo might be undefined despite having an accessToken.
✅ Verification successful
Edge case is properly handled
Looking at the complete guard implementation, the userInfo is safely handled:
- The userInfo is obtained through
userStore.userInfo || (await authStore.fetchUserInfo())
, ensuring it's always defined - The homePath access is used with the OR operator (
||
) to fallback toDEFAULT_HOME_PATH
if userInfo.homePath is undefined - The code is within a guard that already checks for accessToken, and the userInfo is fetched before being used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for places where userInfo might be null/undefined while accessToken exists
ast-grep --pattern 'if ($accessStore.accessToken) {
$$$
$userInfo
$$$
}'
Length of output: 80
Script:
#!/bin/bash
# Let's search for the userInfo variable declaration and usage
ast-grep --pattern '$userInfo = $$$'
# Also search for any null checks on userInfo
ast-grep --pattern 'if ($userInfo) { $$$ }'
# And search for the actual getUserInfo function
rg "getUserInfo" -A 5
Length of output: 4964
Script:
#!/bin/bash
# Let's check the router guard implementation to see how userInfo is obtained and used
rg -A 10 "userInfo.*homePath" playground/src/router/guard.ts
# Also check for error handling around getUserInfo
ast-grep --pattern 'try {
$$$
getUserInfo
$$$
} catch'
Length of output: 566
Script:
#!/bin/bash
# Let's check how userInfo is obtained in the guard context
rg -B 10 "userInfo.*homePath" playground/src/router/guard.ts
# And check the complete guard implementation for context
cat playground/src/router/guard.ts
Length of output: 4104
apps/web-naive/src/router/guard.ts (1)
57-59
: LGTM: Implementation consistent with playground version
The changes mirror the playground implementation, maintaining consistency across the codebase:
- Login redirection logic (57-59)
- Query parameter handling (77-80)
- Default path redirection (109-112)
Also applies to: 77-80, 109-112
apps/web-antd/src/router/guard.ts (1)
57-59
: LGTM: Implementation consistent across all variants
The changes maintain consistency with both playground and naive implementations, ensuring uniform behavior across different UI frameworks.
Also applies to: 77-80, 110-113
✅ Verification successful
Implementation verified as consistent across all variants
The search results confirm that the router guard implementation is identical across all UI framework variants (Antd, Naive, Element Plus, and Playground), with the same redirect logic using:
to.query?.redirect
userInfo?.homePath
DEFAULT_HOME_PATH
All implementations share the same fallback pattern and home path handling, ensuring uniform behavior across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if there are any Antd-specific router configurations that might affect this implementation
rg -g '!node_modules' -g '*.ts' -g '*.tsx' 'router.*config.*antd' .
Length of output: 69
Script:
#!/bin/bash
# Let's check for router guard implementations across different UI variants
rg -g '!node_modules' -g '*.ts' -g '*.tsx' 'userInfo\?\.homePath.*DEFAULT_HOME_PATH' .
# Also check for similar redirect logic in other variants
rg -g '!node_modules' -g '*.ts' -g '*.tsx' '(to\.query\?\.redirect|DEFAULT_HOME_PATH)' .
# Look for router guard files in playground and naive implementations
fd "guard.ts" .
Length of output: 3890
apps/backend-mock/utils/mock-data.ts (2)
7-7
: LGTM: Well-structured interface update
The optional homePath property is correctly added to the UserInfo interface, maintaining backward compatibility.
24-24
: Verify that the homePath values match existing routes
The homePath values look correct and match the available dashboard routes. Let's verify all paths exist in the routing configuration.
Also applies to: 32-32
✅ Verification successful
Both homePath values are correctly mapped to existing routes
The verification confirms that both '/workspace' and '/analytics' paths are properly defined as routes across multiple frontend implementations (web-antd, web-naive, web-ele) and are consistently documented. The homePath values in mock-data.ts align perfectly with the available routing configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the specified homePaths exist as valid routes
# Expected: Both '/workspace' and '/analytics' should be found in route definitions
echo "Checking for '/workspace' and '/analytics' routes..."
rg -l "path: '/workspace'"
rg -l "path: '/analytics'"
Length of output: 830
apps/web-ele/src/router/guard.ts (2)
57-59
: LGTM: Improved redirection logic
The redirection precedence is correctly implemented:
- Use redirect query parameter if present
- Fall back to user's homePath if available
- Finally use DEFAULT_HOME_PATH
77-80
: LGTM: Optimized query parameter handling
Good optimization to omit the redirect query parameter when navigating to the default home path, resulting in cleaner URLs.
感谢大佬的付出,经测试在hash模式下还有点问题 QQ20241218-111932.mp4如果输入 |
Description
修复登录后,在浏览器地址栏手输地址导航到"/"时,用户信息中的homePath未能覆盖默认首页地址的问题。
fix #5152
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
homePath
property to user information, allowing for personalized user redirection.Bug Fixes
Refactor