fixed issue #965 Corrected redirection to Github and Nest profile and test files#998
fixed issue #965 Corrected redirection to Github and Nest profile and test files#998KaranNegi20Feb wants to merge 10 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
WalkthroughThe changes remove assertions verifying the author's name from multiple test cases across Home, ProjectDetails, and RepositoryDetails pages. Additionally, the GraphQL query for project data now includes a Changes
Possibly related PRs
✨ Finishing Touches
🪧 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
|
![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
🧹 Nitpick comments (1)
frontend/src/components/CardDetailsPage.tsx (1)
99-104: Consider extracting comment display to a separate componentFor better maintainability and code organization, consider extracting the comment display logic to a separate component, especially if this pattern is used in multiple places.
- {item?.commentsCount ? ( - <> - <FontAwesomeIcon icon={faFileCode} className="ml-4 mr-2 h-4 w-4" /> - <span>{item.commentsCount} comments</span> - </> - ) : null} + <CommentCount count={item?.commentsCount} />Where
CommentCountcould be defined as:const CommentCount = ({ count }) => { if (!count) return null; return ( <> <FontAwesomeIcon icon={faFileCode} className="ml-4 mr-2 h-4 w-4" /> <span>{count} comments</span> </> ); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/__tests__/e2e/pages/home.spec.ts(0 hunks)frontend/__tests__/unit/pages/ProjectDetails.test.tsx(0 hunks)frontend/__tests__/unit/pages/RepositoryDetails.test.tsx(0 hunks)frontend/src/api/queries/projectQueries.ts(1 hunks)frontend/src/components/CardDetailsPage.tsx(1 hunks)frontend/src/components/ItemCardList.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/tests/e2e/pages/home.spec.ts
- frontend/tests/unit/pages/ProjectDetails.test.tsx
- frontend/tests/unit/pages/RepositoryDetails.test.tsx
🔇 Additional comments (5)
frontend/src/api/queries/projectQueries.ts (2)
19-19: Enhanced data retrieval for issue navigationAdding the
urlfield to therecentIssuessection is a good enhancement that aligns with the PR objective of correcting redirection to GitHub issues.
24-24: Improved author profile linking capabilitiesAdding the
urlfield to the author object enables proper navigation to author profiles, which directly addresses the issue mentioned in PR #965 about correcting redirections.frontend/src/components/ItemCardList.tsx (2)
23-44: Restructured item card for better navigation and layoutThe changes properly implement the redirection fixes by:
- Adding proper author profile links using
/community/users/${item?.author?.login}- Using the newly added
urlfield from the GraphQL query to link to the issue- Improving the layout structure for better visual organization
These changes directly address the issue mentioned in PR #965 regarding corrected redirection to GitHub and Nest profiles.
27-28:Details
✅ Verification successful
Verify URL format for author profiles
Ensure this URL format
/community/users/${item?.author?.login}matches the expected route structure in your application's router.
🏁 Script executed:
#!/bin/bash # Check if the routing path for user profiles exists in the codebase echo "Searching for user profile route definition..." rg -A 2 -B 2 "path.*(/community/users|community/users)" --glob "*.{tsx,jsx,ts,js}"Length of output: 744
Routing URL Validation Confirmed
The route defined infrontend/src/App.tsxuses the/community/users/:userKeypattern, which is correctly matched by the link URL/community/users/${item?.author?.login}inItemCardList.tsx. No changes are required as the URL format is consistent with the expected routing structure.frontend/src/components/CardDetailsPage.tsx (1)
99-104: Improved UI with conditional comment displayThe conditional rendering for comments count prevents displaying empty or undefined comment sections, making the UI cleaner and more user-friendly. This is a good practice that enhances the user experience.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts (2)
68-69: Ensure author link test coverageWhile the test verifies the author image is visible, it doesn't explicitly check if clicking on the author image navigates to the correct profile page. Consider adding a test that verifies this navigation functionality.
await expect(page.getByRole('img', { name: 'Test User 1' })).toBeVisible() +// Add test for author link navigation +await page.getByRole('img', { name: 'Test User 1' }).click() +await expect(page.url()).toContain('/users/test-user-1') +await page.goto('/projects/test-project/repositories/test-repository') // Navigate back
73-78: Release author link check neededSimilar to the issue section, there's no test to verify that clicking on the release author's image navigates correctly to their profile page.
await expect(page.getByRole('img', { name: 'Test User 2' })).toBeVisible() await expect(page.getByText('Jan 1, 2024', { exact: true })).toBeVisible() +// Add test for release author link navigation +await page.getByRole('img', { name: 'Test User 2' }).click() +await expect(page.url()).toContain('/users/test-user-2')frontend/__tests__/e2e/pages/ProjectDetails.spec.ts (3)
27-28: Ensure URL is a clickable linkThe test verifies that the URL text is visible, but it doesn't check if it's a clickable link that would redirect to GitHub. Consider enhancing this test to verify the link functionality.
await expect(page.getByText('URL: https://github.com/')).toBeVisible() +// Check that the URL is a clickable link +await expect(page.getByRole('link', { name: 'https://github.com/' })).toBeVisible()
73-74: Verify author profile link navigationThe test checks if the author image is visible but doesn't verify navigation to the author's profile. Consider adding a test for this functionality to ensure it meets the PR objective.
await expect(page.getByRole('img', { name: 'Dave Debugger' })).toBeVisible() +// Test author profile navigation +await page.getByRole('img', { name: 'Dave Debugger' }).click() +await expect(page.url()).toContain('/users/dave-debugger') +await page.goto('/projects/test-project') // Navigate back to project page
98-99: Check for more repository linksThe test only verifies navigation for "Repo One" but ignores "Repo Two". Consider testing both repository links to ensure comprehensive coverage.
await page.getByText('Repo One').click() expect(page.url()).toContain('repositories/repo-1') +// Navigate back and test the second repository link +await page.goto('/projects/test-project') +await page.getByText('Repo Two').click() +expect(page.url()).toContain('repositories/repo-2')
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/__tests__/e2e/pages/ProjectDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts(1 hunks)
🔇 Additional comments (4)
frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts (2)
1-13: Test setup looks goodThe setup code correctly routes GraphQL requests to use mock data before each test, which is a solid testing practice for isolating tests from external dependencies.
65-71: Tests align with PR objectiveThis test correctly validates the Recent Issues section, checking for the presence of author images and comment counts, which aligns with the PR objective of fixing links to GitHub and profile pages.
frontend/__tests__/e2e/pages/ProjectDetails.spec.ts (2)
70-76: Good coverage of recent issues sectionThis test provides good coverage of the recent issues section, including author image and comment count, which aligns with the PR objective of fixing GitHub and profile redirections.
85-100: Effective repository navigation testThis test effectively validates that repository links are clickable and correctly redirect to the appropriate repository page, which directly addresses the PR objective of fixing redirection issues.
| await expect(page.getByText('Size: 1200 KB')).toBeVisible() | ||
| await expect(page.getByRole('link', { name: 'https://github.com/test-repo' })).toBeVisible() | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the GitHub URL format in the test
This test verifies that the repository URL is displayed and clickable, which directly relates to the PR's goal of fixing GitHub redirection. Ensure this URL format aligns with the actual URL pattern used in production.
🏁 Script executed:
#!/bin/bash
# Check how repository URLs are formatted in the actual code
rg -A 2 "github.com" --glob "*.tsx" --glob "*.ts" frontend/src/components/Length of output: 1894
Update the expected GitHub URL in the test to match the production format
The production code builds GitHub URLs using a two-segment structure (e.g., https://github.com/OWASP/Nest or dynamically as https://github.com/<owner>/<repo>) while the RepositoryDetails test currently asserts the presence of a link with text "https://github.com/test-repo". Please update the test to include both the owner and repository segments (for example, "https://github.com/testOwner/test-repo" or an equivalent that mirrors the actual URL pattern in production) to fully align with the redirect fix requirements.
- Action required: Modify the expected URL in
frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts(around lines 24-26) so that it reflects the correct two-segment URL format.
2 participants
Fixes Issue
Issue: #965
Describe the bug
Buttons don't link to the github issue.