leftDrawerOrg: Fixed the org not found error on viewing admin profile.
#2386
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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
Documentation and Community
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (1)
122-132: Remove unnecessary fragment
The fragment wrapping the error button is redundant as it contains only one child element.
- <>
<button
className={`${styles.profileContainer} bg-danger text-start text-white`}
disabled
>
<div className="px-3">
<WarningAmberOutlined />
</div>
{tErrors('errorLoading', { entity: 'Organization' })}
</button>
- </>🧰 Tools
🪛 Biome
[error] 122-132: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx (3)
294-294: Consider cleaning up redundant local storage setup.
The test case sets id in local storage, but FirstName and LastName are still being set in the beforeEach hook. Since these values aren't used for the profile page detection anymore, consider removing them from the beforeEach hook to keep the test setup focused and maintainable.
Line range hint 293-310: Consider renaming the test case for clarity.
The test case title "Testing Profile Page & Organization Detail Modal" doesn't clearly reflect that it's validating the fix for the "org not found error" on the admin profile page. Consider a more descriptive name like "Should not show org not found error when viewing admin profile".
- test('Testing Profile Page & Organization Detail Modal', async () => {
+ test('Should not show org not found error when viewing admin profile', async () => {Line range hint 293-310: Consider adding a negative test case.
While the current test validates that the error is not shown, consider adding a test case that verifies the component's behavior when viewing a non-admin profile or when the ID doesn't match. This would ensure the fix doesn't mask legitimate error cases.
Example test case:
test('Should show org not found error when viewing non-admin profile', async () => {
setItem('id', 'non-admin-id');
render(
<MockedProvider addTypename={false} link={linkEmpty}>
<BrowserRouter>
<Provider store={store}>
<I18nextProvider i18n={i18nForTest}>
<LeftDrawerOrg {...props} hideDrawer={null} />
</I18nextProvider>
</Provider>
</BrowserRouter>
</MockedProvider>,
);
await wait();
expect(
screen.getByText(/Error occured while loading Organization data/i),
).toBeInTheDocument();
});📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx (2 hunks)
- src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (4 hunks)
🧰 Additional context used
🪛 Biome
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
[error] 122-132: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (5)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (4)
9-9: LGTM: Clean import additions
The new imports for useLocation and useLocalStorage are properly placed and necessary for the implemented functionality.
Also applies to: 16-16
74-75: LGTM: Improved organization state handling
The explicit setting of organization to undefined when data is not available improves state clarity and helps prevent undefined behavior.
121-133: LGTM: Fixed organization error visibility on profile pages
The conditional rendering based on isProfilePage successfully addresses the core issue of showing the "org not found error" on admin profile pages.
🧰 Tools
🪛 Biome
[error] 122-132: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
42-47: Verify impact on other routes
Let's ensure the changes don't affect organization error handling on non-profile routes.
Also applies to: 121-133
✅ Verification successful
Let me try a different approach to verify the impact on routes and error handling.
Based on the analysis of the shell script results, I can now provide a final response:
Profile page detection won't affect organization error handling on other routes
The changes to detect profile pages in LeftDrawerOrg are isolated and won't impact organization error handling because:
- The profile detection logic only affects the internal state of LeftDrawerOrg component
- Organization error handling is implemented separately in individual route components like OrgList, Requests, etc.
- The routing structure in App.tsx shows clear separation between profile and organization routes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for route definitions and organization error handling patterns
echo "Checking route definitions:"
rg -l "Route.*path.*admin/.*" --type tsx
echo "\nChecking organization error handling in components:"
ast-grep --pattern 'errorLoading.*Organization'
Length of output: 302
Script:
#!/bin/bash
# Search for route definitions and organization error handling patterns
echo "Searching for route definitions:"
rg -A 5 "Route.*path.*"
echo "\nChecking organization error handling:"
rg -A 3 "Organization.*not found|org.*not found" --ignore-case
echo "\nChecking LeftDrawerOrg usage:"
rg -A 3 "LeftDrawerOrg"
Length of output: 30530
src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx (1)
308-310: LGTM! Assertion properly validates the fix.
The assertion correctly verifies that the "org not found error" is resolved, which directly addresses the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (1)
127-139: Remove unnecessary Fragment and simplify the conditional rendering.
The error button implementation can be simplified by removing the unnecessary Fragment wrapper.
- !isProfilePage && (
- <>
- <button
- className={`${styles.profileContainer} bg-danger text-start text-white`}
- disabled
- >
- <div className="px-3">
- <WarningAmberOutlined />
- </div>
- {tErrors('errorLoading', { entity: 'Organization' })}
- </button>
- </>
- )
+ !isProfilePage && (
+ <button
+ className={`${styles.profileContainer} bg-danger text-start text-white`}
+ disabled
+ >
+ <div className="px-3">
+ <WarningAmberOutlined />
+ </div>
+ {tErrors('errorLoading', { entity: 'Organization' })}
+ </button>
+ )🧰 Tools
🪛 Biome
[error] 128-138: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (4 hunks)
🧰 Additional context used
🪛 Biome
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
[error] 128-138: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (4)
9-9: LGTM! Necessary imports added for route and storage handling.
The added imports support the new functionality for detecting profile pages and managing user data.
Also applies to: 16-16
45-49: LGTM! Clean implementation of URL parsing.
The getIdFromPath function is well-implemented with clear documentation.
Line range hint 51-73: LGTM! Robust state management implementation.
The state management and effect implementation correctly:
- Tracks profile page status
- Updates on route or user changes
- Handles dependencies properly
42-44: Verify the fix for org not found error on admin profile.
The implementation looks correct, but let's verify that this is the only instance where we need to handle the org not found error for profile pages.
Also applies to: 51-53, 127-139
✅ Verification successful
Organization error handling is correctly implemented for profile pages
The implementation in LeftDrawerOrg.tsx properly handles the "org not found" error by:
- Detecting profile pages through URL path analysis
- Suppressing the organization error message specifically on profile pages
- Using a dedicated state variable
isProfilePagethat updates when location changes
This is the only instance where organization loading errors need this handling, as verified by the codebase analysis. The fix is properly scoped and implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where organization loading errors might need similar handling
rg -l "errorLoading.*Organization" --type tsx
# Search for other components that might need profile page detection
ast-grep --pattern 'useLocation()' --lang tsx
Length of output: 841
Script:
#!/bin/bash
# Search for organization loading error handling patterns with context
rg "errorLoading.*Organization|error.*org.*found" -A 5 -B 5
# Search for profile page detection patterns
rg "location\.pathname.*profile" -A 5 -B 5
Length of output: 1003
Script:
#!/bin/bash
# Search for profile-related path checks with more variations
rg -A 5 -B 5 "pathname.*includes.*profile|isProfile|profile.*page"
# Look for organization error handling in the component
ast-grep --pattern 'const LeftDrawerOrg = () => {
$$$
}'
Length of output: 2885
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (1)
80-81: Consider adding additional null checks for organizations array.
While setting organization to undefined when data is not available is good, consider handling the case where data exists but the organizations array is empty.
if (data && isMounted) {
- setOrganization(data?.organizations[0]);
+ setOrganization(data.organizations?.[0] ?? undefined);
} else {
setOrganization(undefined);
}📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx (5 hunks)
- src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx
🔇 Additional comments (5)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (5)
9-9: LGTM: Import additions are appropriate.
The new imports support the required functionality for URL tracking and local storage access.
Also applies to: 16-16
42-53: LGTM: URL parsing and state management implementation.
The implementation follows best practices and incorporates the previously suggested improvements for URL parsing.
69-73: LGTM: Profile page detection is robust.
The useEffect implementation correctly tracks both location and userId changes, ensuring accurate profile page detection.
Line range hint 127-137: LGTM: Successfully fixes the "org not found error" on profile pages.
The conditional rendering ensures the error message is suppressed when viewing admin profiles, directly addressing the issue #1924.
Line range hint 1-1: Verify test coverage for the new changes.
The PR mentions that tests were added, but they're not visible in this file. Please ensure there are tests covering:
- Profile page detection logic
- Organization error message suppression on profile pages
- Edge cases in URL parsing
✅ Verification successful
Tests are properly implemented and cover all requested scenarios
The test file shows comprehensive coverage of the requested areas:
- Profile page detection is tested in "Should not show org not found error when viewing admin profile" test case, which verifies the behavior on
/member/123route - Organization error message suppression on profile pages is explicitly tested with
expect(screen.queryByText(/Error occured while loading Organization data/i)).not.toBeInTheDocument() - URL parsing and routing is tested through multiple test cases including:
- Different route navigation tests (Dashboard, People, etc.)
- Profile page detection with
MemoryRouterand initial entries - Organization data loading based on URL parameters
The tests are well-structured with proper mocks and cover both success and error scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related test files and their coverage
echo "Searching for LeftDrawerOrg test files..."
fd -e test.tsx -e spec.tsx "LeftDrawerOrg" src/
echo "\nChecking test coverage for profile page detection..."
rg -A 5 "isProfilePage|getIdFromPath" src/ --glob "*.test.tsx" --glob "*.spec.tsx"
Length of output: 396
Script:
#!/bin/bash
# Let's examine the content of the test file to verify coverage
echo "Content of LeftDrawerOrg test file:"
cat src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx
# Let's also check for URL/path related utilities that might contain the profile detection logic
echo -e "\nChecking for URL/path utility files and their tests:"
fd -e ts -e tsx "(url|path|route)" src/ --type f
Length of output: 13377
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2386 +/- ##
========================================
Coverage 98.05% 98.06%
========================================
Files 255 255
Lines 7208 7223 +15
Branches 2086 2104 +18
========================================
+ Hits 7068 7083 +15
Misses 130 130
Partials 10 10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx(5 hunks)
🔇 Additional comments (3)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (3)
67-72: LGTM! Profile page detection is well implemented.
The implementation correctly:
- Detects profile pages by comparing URL ID with user ID
- Updates on location or userId changes
- Follows React hooks best practices
79-80: LGTM! Clear organization state management.
Explicitly setting organization to undefined when data is not available improves state clarity and helps prevent the "org not found error".
Line range hint 126-136: LGTM! Error message handling addresses the core issue.
The implementation successfully prevents the "org not found error" on profile pages while maintaining appropriate error visibility elsewhere.
Let's verify the fix addresses all cases:
✅ Verification successful
LGTM! Error handling implementation is consistent across components
The verification confirms:
LeftDrawerOrgcorrectly usesisProfilePageto conditionally render the error messageUserSidebarOrghas similar error handling structure but doesn't need the profile page check as it serves a different purpose- No other components use the organization error loading pattern that would need similar fixes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify error message handling in similar components
# Look for similar patterns of organization error handling that might need the same fix
# Search for organization error message patterns
rg -l "errorLoading.*Organization" --type tsx
# Search for similar profile page checks
ast-grep --pattern 'const [_] = useState<boolean>($$$)'
Length of output: 133
Script:
#!/bin/bash
# Let's try different approaches to verify error handling patterns
# Search for error loading messages in all files
rg "errorLoading.*Organization"
# Search for profile page related state
rg "isProfilePage"
# Search for similar error handling patterns
rg "WarningAmberOutlined"
# Look for translation keys related to organization errors
rg "tErrors.*organization" -i
Length of output: 951
Script:
#!/bin/bash
# Let's check UserSidebarOrg component for similar error handling
# Check the implementation of UserSidebarOrg
cat src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.tsx
# Look for any profile page related logic in the entire codebase
rg "profile.*page" -i
Length of output: 7674
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (1)
Line range hint
42-72: Consider adding type safety to userId retrieval.While the profile detection logic is sound, the
userIdretrieval from localStorage could benefit from type safety.- const userId = getItem('id'); + const userId = getItem('id') as string | null;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx(5 hunks)
🔇 Additional comments (3)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (3)
9-9: LGTM: Import additions are appropriate.
The new imports for useLocation and useLocalStorage are necessary for implementing the profile page detection feature.
Also applies to: 16-16
79-85: LGTM: Improved organization state management.
The explicit setting of organization to undefined when data is not available improves state clarity and helps prevent undefined behavior.
Line range hint 126-136: LGTM: Error handling aligns with PR objectives.
The conditional rendering based on isProfilePage effectively addresses the "org not found error" issue when viewing admin profiles while maintaining appropriate error visibility in other contexts.
Let's verify the fix covers all profile-related paths:
✅ Verification successful
LGTM: Error handling implementation is correctly implemented for profile pages
The verification confirms that:
- The
isProfilePagestate is properly set based on comparing the URL path's ID with the user's ID - The error message is correctly suppressed when viewing admin profiles (
id === userId) - The implementation handles the organization error display appropriately in other contexts
- The error handling is isolated to this component and doesn't require changes in other profile-related components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all profile-related routes to ensure we're handling all cases
rg -l "profile|admin.*view|user.*view" --type ts --type tsx
Length of output: 89
Script:
#!/bin/bash
# Search for all profile-related routes and views with correct file extensions
rg -l "profile|admin.*view|user.*view" --type typescript --type tsx --type jsx
# Backup search to find profile-related components and routes
fd -e ts -e tsx -e jsx | xargs rg -l "profile|admin.*view|user.*view"
Length of output: 2201
Script:
#!/bin/bash
# Let's check how isProfilePage is used in LeftDrawerOrg and related components
rg -A 5 "isProfilePage" src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
# Check for profile-related route definitions or constants
rg "profile.*route|PROFILE.*PATH|path.*profile" src/
# Check how the organization error is handled in other components
rg -A 3 "tErrors\('errorLoading'.*Organization" src/
Length of output: 961
Script:
#!/bin/bash
# Let's check where and how isProfilePage is set/updated in LeftDrawerOrg
rg -B 10 "setIsProfilePage" src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
# Check for any profile page related conditions or checks
rg "profile.*page|page.*profile" src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
# Check the component's props and interface definitions
ast-grep --pattern 'interface $_Props {
$$$
}' src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
Length of output: 1218
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx(5 hunks)
🔇 Additional comments (3)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (3)
9-9: LGTM: Dependencies are properly imported.
The added imports for useLocation and useLocalStorage are appropriate for implementing the profile page detection feature.
Also applies to: 16-16
79-80: LGTM: Proper organization state management.
Setting organization to undefined when data is not available ensures clean state transitions between pages.
Line range hint 126-136: LGTM: Error handling addresses the original issue.
The conditional rendering based on isProfilePage effectively prevents the "org not found error" from showing on profile pages while maintaining appropriate error visibility on organization pages.
Let's verify the fix addresses all profile-related paths:
✅ Verification successful
Error handling implementation correctly handles profile pages
The verification confirms that:
- The profile detection logic correctly identifies profile pages by comparing the URL path ID with the logged-in user's ID
- The error message is suppressed only when viewing your own profile (
id === userId) - The implementation covers both the community profile route (
/communityProfile) and organization-related routes - The error handling maintains visibility on non-profile pages as intended
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all profile-related routes to ensure they're covered by the fix
# Test: Search for route definitions containing 'profile'
rg -A 3 "path.*profile"
Length of output: 25
Script:
#!/bin/bash
# Let's try different search patterns to find profile-related routes and components
# Search for profile-related route definitions with broader patterns
rg -i "route.*profile|path.*profile"
# Search for profile-related components and their usage
rg -i "isProfilePage|ProfilePage"
# Search for router configuration files
fd -e ts -e tsx "router|routes" --exec rg -l "profile"
Length of output: 490
Script:
#!/bin/bash
# Let's examine the CommunityProfile component and its relationship with LeftDrawerOrg
# Check the implementation of CommunityProfile
rg -A 5 "CommunityProfile" src/App.tsx
# Look for userId usage in LeftDrawerOrg to understand profile detection
rg -A 5 "userId.*=|const.*userId" src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
# Check how profile detection is implemented
ast-grep --pattern 'setIsProfilePage($$$)'
Length of output: 1153
|
Please make changes so that code rabbit approves the PR |
This reverts commit e1e86e3.
|
Reverted previous commit. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx(5 hunks)
🔇 Additional comments (3)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (3)
9-9: LGTM: Import additions are appropriate.
The new imports for useLocation and useLocalStorage are necessary for implementing the profile detection feature.
Also applies to: 16-16
Line range hint 42-72: LGTM: Profile detection logic is well-implemented.
The implementation:
- Correctly extracts user ID from the path
- Properly handles edge cases with the length check
- Includes appropriate dependencies in the useEffect
Line range hint 126-136: LGTM: Error message visibility logic is correctly implemented.
The conditional rendering using !isProfilePage effectively addresses the original issue by hiding the "org not found error" when viewing admin profiles.
|
Please merge with the latest upstream |
…d/OrganizationCard.tsx` (PalisadoesFoundation#2393) * improve testcase for org card * act function warning
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (1)
45-50: Update comment to accurately reflect array indexing.The comment about index 2 could be clearer about zero-based indexing.
- // Index 2 represents the ID in paths like /member/{userId} + // Index 2 (third segment) represents the ID in paths like /member/{userId}src/components/UserPortal/OrganizationCard/OrganizationCard.test.tsx (1)
229-259: Consider adding negative test cases.Given that this PR fixes an "org not found error", consider adding test cases that explicitly verify:
- No error occurs when viewing admin profile
- Behavior when organization ID is invalid or missing
- Edge cases around membership status transitions
This would help prevent regression of the fixed bug.
Would you like me to help generate these additional test cases?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx(5 hunks)src/components/UserPortal/OrganizationCard/OrganizationCard.test.tsx(2 hunks)
🔇 Additional comments (4)
src/components/LeftDrawerOrg/LeftDrawerOrg.tsx (2)
9-9: LGTM: Clean hook setup and initialization.
The added hooks and their initialization follow React best practices and are necessary for implementing the profile page detection feature.
Also applies to: 16-16, 42-44
Line range hint 127-137: LGTM: Error handling successfully fixed.
The conditional rendering based on isProfilePage effectively prevents the "org not found error" from appearing on admin profile pages while maintaining the error display for other scenarios.
Let's verify the fix works across different routes:
✅ Verification successful
Let me analyze the isProfilePage state usage in the LeftDrawerOrg component. From the search results, I can see:
- The state is set in a useEffect hook based on comparing the current user's ID with the ID from the path
- It's used to conditionally render the error message
Let me verify the complete implementation:
LGTM: Error handling implementation is correct and safe
The implementation correctly prevents the "org not found error" from appearing on profile pages through:
- Proper state management using
isProfilePagethat's set based on comparing the current URL path ID with the user's ID - Clean conditional rendering that only shows the error when
!isProfilePage && organization == undefined - Safe cleanup with proper dependency array in the useEffect hook
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for route definitions to ensure we're handling all profile-related paths consistently
rg -g '*.{ts,tsx}' -A 3 'path.*profile|route.*profile'
Length of output: 56
Script:
#!/bin/bash
# Let's try a broader search for profile-related routes and components
rg -g '*.{ts,tsx}' -B 2 -A 2 'isProfilePage|profilePage|/profile'
# Also search for route definitions in common routing files
fd -e tsx -e ts router routes route
Length of output: 1145
Script:
#!/bin/bash
# Let's check how isProfilePage is being set and used
ast-grep --pattern 'setIsProfilePage($_)'
# Also check for any useEffect or similar hooks that might be updating this state
ast-grep --pattern 'useEffect($$$)'
Length of output: 74956
Script:
#!/bin/bash
# Check the full implementation of LeftDrawerOrg component
cat src/components/LeftDrawerOrg/LeftDrawerOrg.tsx
Length of output: 7149
src/components/UserPortal/OrganizationCard/OrganizationCard.test.tsx (2)
2-2: LGTM! Good practice to import act from testing-library.
Using act from '@testing-library/react' instead of 'react' is the recommended approach for testing React components.
229-259: LGTM! Test verifies navigation functionality.
The test effectively validates that clicking the manage button navigates to the correct organization path.
src/components/UserPortal/OrganizationCard/OrganizationCard.test.tsx
Outdated
Show resolved
Hide resolved
|
Please ensure that coderabbit approves this PR |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…ationCard/OrganizationCard.tsx` (PalisadoesFoundation#2393)" This reverts commit becd2d1.
|
It seems the introspection failed from GSOC work of USER Tag |
what should i do? |
|
@sethdivyansh merge with the upstream, it'll pass. |
What kind of change does this PR introduce?
bugfix
Issue Number:
Fixes #1924
Did you add tests for your changes?
Yes
Snapshots/Videos:
Screen.Recording.2024-10-30.at.3.53.17.PM.mov
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
LeftDrawerOrgcomponent to track the current URL and adjust UI based on the page context.Bug Fixes
Tests