Revamp top contributor list component #1730
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes increase the default and displayed number of top contributors across backend and frontend, remove the display and retrieval of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)frontend/src/components/TopContributorsList.tsx (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (36)
backend/apps/github/graphql/queries/repository_contributor.py(1 hunks)frontend/__tests__/e2e/data/mockHomeData.ts(0 hunks)frontend/__tests__/e2e/pages/About.spec.ts(1 hunks)frontend/__tests__/e2e/pages/ChapterDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/CommitteeDetails.spec.ts(0 hunks)frontend/__tests__/e2e/pages/Home.spec.ts(1 hunks)frontend/__tests__/e2e/pages/ProjectDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts(1 hunks)frontend/__tests__/e2e/pages/UserDetails.spec.ts(0 hunks)frontend/__tests__/unit/data/mockAboutData.ts(0 hunks)frontend/__tests__/unit/data/mockChapterData.ts(0 hunks)frontend/__tests__/unit/data/mockChapterDetailsData.ts(0 hunks)frontend/__tests__/unit/data/mockCommitteeData.ts(0 hunks)frontend/__tests__/unit/data/mockCommitteeDetailsData.ts(0 hunks)frontend/__tests__/unit/data/mockHomeData.ts(0 hunks)frontend/__tests__/unit/data/mockOrganizationData.ts(0 hunks)frontend/__tests__/unit/data/mockProjectData.ts(0 hunks)frontend/__tests__/unit/data/mockProjectDetailsData.ts(0 hunks)frontend/__tests__/unit/data/mockRepositoryData.ts(0 hunks)frontend/__tests__/unit/data/mockSnapshotData.ts(0 hunks)frontend/__tests__/unit/pages/About.test.tsx(2 hunks)frontend/__tests__/unit/pages/ChapterDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/CommitteeDetails.test.tsx(0 hunks)frontend/__tests__/unit/pages/ProjectDetails.test.tsx(1 hunks)frontend/__tests__/unit/pages/RepositoryDetails.test.tsx(1 hunks)frontend/src/app/about/page.tsx(1 hunks)frontend/src/app/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(1 hunks)frontend/src/components/ContributorAvatar.tsx(2 hunks)frontend/src/components/TopContributorsList.tsx(2 hunks)frontend/src/server/queries/committeeQueries.ts(0 hunks)frontend/src/server/queries/homeQueries.ts(1 hunks)frontend/src/server/queries/organizationQueries.ts(0 hunks)frontend/src/server/queries/projectQueries.ts(0 hunks)frontend/src/server/queries/repositoryQueries.ts(0 hunks)frontend/src/types/contributor.ts(1 hunks)
💤 Files with no reviewable changes (19)
- frontend/tests/e2e/pages/UserDetails.spec.ts
- frontend/tests/unit/data/mockCommitteeData.ts
- frontend/tests/unit/data/mockProjectData.ts
- frontend/tests/unit/data/mockCommitteeDetailsData.ts
- frontend/tests/unit/data/mockRepositoryData.ts
- frontend/tests/unit/data/mockProjectDetailsData.ts
- frontend/src/server/queries/repositoryQueries.ts
- frontend/tests/e2e/pages/CommitteeDetails.spec.ts
- frontend/src/server/queries/organizationQueries.ts
- frontend/tests/unit/data/mockChapterData.ts
- frontend/tests/e2e/data/mockHomeData.ts
- frontend/tests/unit/data/mockHomeData.ts
- frontend/tests/unit/pages/CommitteeDetails.test.tsx
- frontend/tests/unit/data/mockOrganizationData.ts
- frontend/tests/unit/data/mockChapterDetailsData.ts
- frontend/src/server/queries/projectQueries.ts
- frontend/tests/unit/data/mockAboutData.ts
- frontend/src/server/queries/committeeQueries.ts
- frontend/tests/unit/data/mockSnapshotData.ts
🧰 Additional context used
🧠 Learnings (1)
frontend/src/components/CardDetailsPage.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
🧬 Code Graph Analysis (1)
frontend/src/components/TopContributorsList.tsx (2)
frontend/src/utils/urlFormatter.ts (1)
getMemberUrl(1-1)frontend/src/utils/capitalize.ts (1)
capitalize(1-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (26)
frontend/src/app/page.tsx (1)
308-309: LGTM! Consistent implementation of increased contributor display.The increase from 9 to 12 initial contributors aligns well with the PR objectives to revamp the top contributor list component. The change provides better visibility while maintaining reasonable performance.
frontend/src/app/about/page.tsx (1)
128-129: LGTM! Maintains consistency with other component updates.The change maintains consistency with the main page and other components using
TopContributorsList. The increased initial display count enhances user experience by showing more contributors upfront.frontend/__tests__/e2e/pages/ChapterDetails.spec.ts (1)
45-48: LGTM! Improved test precision with exact matching.Adding
{ exact: true }to contributor assertions improves test reliability by ensuring precise matching of contributor names and images. This is particularly important after removing contribution counts from the UI.frontend/__tests__/e2e/pages/About.spec.ts (1)
45-46: LGTM! Consistent test improvement across page tests.The exact matching approach is consistently applied across different page tests, ensuring reliable contributor name assertions. This maintains testing consistency and improves overall test quality.
frontend/src/components/CardDetailsPage.tsx (1)
203-204: LGTM! Consistent implementation across the shared component.The change maintains consistency with other
TopContributorsListusages throughout the application. SinceCardDetailsPageis used across multiple detail pages, this ensures a uniform user experience with increased contributor visibility.frontend/src/types/contributor.ts (1)
3-3: LGTM: Proper handling of optional contributionsCount.Making the
contributionsCountproperty optional is the right approach for this migration, allowing components to gracefully handle the absence of contribution count data while maintaining backward compatibility.backend/apps/github/graphql/queries/repository_contributor.py (1)
16-16: LGTM: Increased default limit aligns with frontend changes.The increase from 15 to 20 contributors aligns with the coordinated changes to display more contributors by default across the application.
frontend/__tests__/e2e/pages/Home.spec.ts (1)
60-61: LGTM: Improved test precision with exact matching.Adding
{ exact: true }to the contributor assertions makes the tests more precise and less prone to false positives. This is a good testing practice.frontend/__tests__/e2e/pages/RepositoryDetails.spec.ts (1)
58-61: LGTM: Consistent test improvements with exact matching.The addition of
{ exact: true }follows the same pattern as other test files, ensuring consistent and precise test assertions across the application.frontend/__tests__/e2e/pages/ProjectDetails.spec.ts (1)
62-65: LGTM: Completes the consistent test improvement pattern.These changes complete the coordinated effort to add exact matching to contributor assertions across all e2e test files, maintaining consistency and improving test precision.
frontend/__tests__/unit/pages/ChapterDetails.test.tsx (2)
87-89: LGTM! Test expectations updated correctly.The test expectations have been properly updated to reflect the increased
maxInitialDisplayfrom 9 to 12, now expecting Contributors 10 and 12 to be visible initially, and Contributor 13 to not be visible.
105-110: LGTM! Mock data simplified appropriately.The removal of
contributionsCountfrom the mock contributor object is consistent with the broader changes to remove contribution counters from the UI.frontend/src/components/TopContributorsList.tsx (3)
17-17: LGTM! Increased initial display count improves user experience.Doubling the
maxInitialDisplayfrom 6 to 12 provides better visibility of contributors without requiring user interaction.
46-46: LGTM! Grid layout updated to accommodate more contributors.The change from 3 to 4 columns on medium screens appropriately accommodates the increased initial display count while maintaining good visual balance.
49-65: LGTM! Simplified rendering removes problematic contribution counts.The simplified rendering that removes contribution counts addresses the core issue mentioned in the PR description about inconsistent values. The addition of
titleattributes for accessibility is a nice touch.frontend/src/components/ContributorAvatar.tsx (2)
13-13: LGTM! Type guard simplified appropriately.Removing the
contributionsCountcheck from the type guard makes sense since this property is now optional in theContributortype.
33-35: LGTM! Conditional tooltip content handles both cases gracefully.The conditional tooltip content elegantly handles contributors both with and without
contributionsCount, maintaining backward compatibility while adapting to the new optional nature of this property.frontend/__tests__/unit/pages/About.test.tsx (3)
206-208: LGTM! Test expectations updated for increased initial display.The test expectations have been correctly updated to reflect the increased
maxInitialDisplayfrom 9 to 12, now expecting Contributors 10 and 12 to be visible initially.
217-227: LGTM! Toggle functionality tests updated consistently.The toggle functionality tests have been properly updated to reflect the new display boundaries - expecting Contributor 12 to be visible initially, and Contributors 13, 14, 15 to appear after clicking "Show more".
234-234: LGTM! "Show less" functionality test updated correctly.The test correctly expects Contributor 13 to not be visible after clicking "Show less", consistent with the new display boundaries.
frontend/__tests__/unit/pages/RepositoryDetails.test.tsx (3)
104-105: Test updates correctly reflect the new initial display limit.The test expectations have been properly updated to verify that the initial state now shows 12 contributors instead of the previous smaller number, aligning with the increased display limit in the TopContributorsList component.
112-114: Expanded state test coverage looks comprehensive.The test properly verifies that clicking "Show more" reveals contributors 13, 14, and 15, ensuring the expanded view functionality works as expected with the new display limits.
121-121: Collapse functionality test is correctly updated.The test appropriately verifies that clicking "Show less" hides contributor 13, returning to the initial collapsed state with 12 visible contributors.
frontend/__tests__/unit/pages/ProjectDetails.test.tsx (3)
111-112: Test updates maintain consistency across components.The test expectations are correctly updated to match the new initial display limit of 12 contributors, maintaining consistency with the RepositoryDetails test updates.
119-121: Comprehensive testing of expanded contributor view.The test properly verifies that all expected contributors (13, 14, and 15) become visible after clicking "Show more", ensuring the functionality works correctly across different page components.
128-128: Collapse state test correctly implemented.The test appropriately verifies the return to the initial collapsed state, ensuring contributor 13 is no longer visible after clicking "Show less".
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
frontend/src/server/queries/chapterQueries.ts(0 hunks)frontend/src/server/queries/snapshotQueries.ts(0 hunks)frontend/src/types/user.ts(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/src/server/queries/chapterQueries.ts
- frontend/src/server/queries/snapshotQueries.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/types/user.ts (3)
frontend/src/types/issue.ts (1)
Issue(3-19)frontend/src/types/release.ts (1)
Release(3-14)frontend/src/types/project.ts (1)
RepositoryCardProps(51-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run Code Scan
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
frontend/src/types/user.ts (2)
12-16: Clarify the inconsistency with PR objectives regarding contributionsCount.The addition of the required
contributionsCountfield seems inconsistent with the PR's objective of removing contribution counters due to inconsistent values. If contribution counters are being removed from the UI, consider making this field optional or explain why it's needed as a required field.The new optional profile fields (
bio,company,
20-24: LGTM on the additional user profile fields.The addition of
issuesCount,location, andnameas optional fields enhances the User type definition and provides more comprehensive user data modeling.
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
Looks great! Left one suggestion ⬇️
Co-authored-by: Kate Golovanova <[email protected]>
|
Remove the contributions counters as it has different values depending on the page (main, organization, repository as well as the main point -- project based contributions vs all member contributions)
Some before/after screenshots:
