fix: fixed git config route protection using fetchGlobalGitConfigInit… #34873
Conversation
WalkthroughThe changes across the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant GitConfig
participant General
User->>App: Navigate to UserProfile
App->>General: fetchGlobalGitConfigInit()
General-->>App: Render UserProfile with Git config
User->>GitConfig: Attempt to access Git config
GitConfig-->>User: Access granted if authenticated
GitConfig-->>User: Redirect to Login if not authenticated
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/UserProfile/General.tsx (3 hunks)
Additional comments not posted (3)
app/client/src/pages/UserProfile/General.tsx (3)
1-1: IncludeuseEffectwithfetchGlobalGitConfigInitThe
useEffecthook now ensures that the global Git configuration is fetched when the component mounts, which is crucial for security.Ensure that
fetchGlobalGitConfigInitcorrectly handles unauthorized access and redirects users to the login page if they are not authenticated.
22-22: Verify import offetchGlobalGitConfigInitThe new import statement for
fetchGlobalGitConfigInitfromactions/gitSyncActionsis necessary for fetching the global Git configuration.
74-78: EnsureuseEffectdependency array is emptyThe
useEffecthook correctly fetches the global Git configuration on mount. Ensure the dependency array remains empty to avoid unnecessary re-renders.
|
Hey @Shivam-z |
…g it in parent component
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/pages/UserProfile/GitConfig.tsx (2 hunks)
- app/client/src/pages/UserProfile/index.tsx (4 hunks)
Additional comments not posted (3)
app/client/src/pages/UserProfile/index.tsx (2)
Line range hint
1-11: Imports are necessary and correct.The added imports (
useEffect,useState,useDispatch, andfetchGlobalGitConfigInit) are necessary for the new functionality.
51-54: useEffect hook correctly fetches global Git configuration.The
useEffecthook correctly dispatchesfetchGlobalGitConfigIniton component mount.app/client/src/pages/UserProfile/GitConfig.tsx (1)
Line range hint
1-16: Removed unnecessary import.The removal of
fetchGlobalGitConfigInitfrom the import statements is appropriate since the global Git configuration fetch logic has been moved to the parent component.
|
Hi @brayn003 , |
|
Hi @Shivam-z, apologies for the late response. The logical changes look good to me. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/UserProfile/index.test.tsx (1 hunks)
Additional comments not posted (5)
app/client/src/pages/UserProfile/index.test.tsx (5)
1-16: Imports look good.All necessary libraries and modules for testing, Redux, theming, routing, and the
UserProfilecomponent are appropriately imported.
18-84: Mock store setup looks comprehensive.The default state covers various entities and UI states required for the tests. Ensure that the state structure aligns with the actual application state.
86-93: Jest mocks are appropriate.The jest mocks for
design-system-old/build/constants/messagesandactions/gitSyncActionsare correctly set up to isolate the tests from external dependencies.
96-115: Test case for rendering UserProfile component looks good.The test case correctly mocks the store state and verifies that the
UserProfilecomponent renders as expected for logged-in users.
117-132: Test case for rendering Login page looks good.The test case correctly mocks the store state and verifies that the
Loginpage renders as expected for non-logged-in users.
|
@brayn003 added unit test case |
|
Let me trigger the tests |
|
CI tests here - #35249 |
|
All the CI tests are passing. Will merge this PR shortly |
Description:
In this PR I have fixed the GitConfig route
dev.appsmtih.com/profilewith the help offetchGlobalGitConfigInitmethod.Now If an unauthorized user try to access this URL , it will redirects to the login page.
Issue video
Screencast.from.11-07-24.02.37.08.PM.IST.webm
Solution video
Screencast.from.11-07-24.02.36.02.PM.IST.webm
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #34603
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests