feat: added PR count indicator to issue cards on Contribute and Mentee pages#4446
Open
pranav-871 wants to merge 3 commits intoOWASP:mainfrom
Open
feat: added PR count indicator to issue cards on Contribute and Mentee pages#4446pranav-871 wants to merge 3 commits intoOWASP:mainfrom
pranav-871 wants to merge 3 commits intoOWASP:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughCards and related UI now surface open pull request counts: Card accepts and renders Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 7 files
Confidence score: 3/5
- There is a concrete UI regression risk in
frontend/src/components/IssuesTable.tsx: the PR count cell can shift out of alignment with its header when optional columns are enabled together, which is user-visible and can confuse table interpretation. - Test coverage is incomplete in
frontend/__tests__/unit/components/IssuesTable.test.tsx; current checks validate column count but notpullRequestCountrendering branches, so badge/value display regressions may slip through. - Given the medium severity and high confidence on the alignment issue, this carries some merge risk until the column placement logic and targeted rendering assertions are tightened.
- Pay close attention to
frontend/src/components/IssuesTable.tsx,frontend/__tests__/unit/components/IssuesTable.test.tsx- fix column/header alignment and add branch-level assertions for PR-count display.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/__tests__/unit/components/IssuesTable.test.tsx">
<violation number="1" location="frontend/__tests__/unit/components/IssuesTable.test.tsx:532">
P2: New PR-count tests only check column count and miss actual `pullRequestCount` rendering branches, leaving regressions in badge/value display untested.</violation>
</file>
<file name="frontend/src/components/IssuesTable.tsx">
<violation number="1" location="frontend/src/components/IssuesTable.tsx:153">
P2: PR count cell is rendered in a different column position than its header, causing header/data misalignment when optional columns are enabled together.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/components/IssuesTable.tsx (1)
210-214: Consider adding "open PRs" text for consistency with Card component.In
Card.tsxandMenteeIssues.tsx, the badge displays"{count} open PRs"with full text. Here it only shows the number. While this may be intentional for table density, it creates an inconsistency in how PR counts are displayed across the application.If you want consistency, consider updating to match:
<span className="inline-flex items-center gap-1 rounded-full bg-blue-100 px-2 py-0.5 text-xs font-medium text-blue-700 dark:bg-blue-900/30 dark:text-blue-300"> <FaCodePullRequest className="h-3 w-3" /> - {issue.pullRequestCount} + {issue.pullRequestCount} open PR{issue.pullRequestCount === 1 ? '' : 's'} </span>Alternatively, if the compact display is intentional for table space, this is fine as-is since the column header "Open PRs" provides context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/IssuesTable.tsx` around lines 210 - 214, The PR count badge in IssuesTable (rendering block that uses issue.pullRequestCount and FaCodePullRequest) only shows the number, causing UI inconsistency with Card.tsx and MenteeIssues.tsx which render "{count} open PRs"; update the badge rendering in IssuesTable.tsx to include the descriptive text (e.g., "{issue.pullRequestCount} open PRs") while preserving the same styling and icon so it matches Card and MenteeIssues components, or intentionally document/leave it compact if table density is desired.frontend/__tests__/unit/components/IssuesTable.test.tsx (1)
528-540: Tests verify column counts correctly, but consider adding cell content tests.The column count tests are valid. Consider adding tests to verify:
- PR count badge renders when
pullRequestCount > 0- Em-dash renders when
pullRequestCountis 0 or undefined- Mobile view behavior with PR counts
🧪 Suggested additional test cases
it('renders PR count badge when pullRequestCount is provided', () => { const issueWithPRs: IssueRow = { objectID: 'pr-test', number: 200, title: 'Issue with PRs', state: 'open', labels: [], pullRequestCount: 3, } render(<IssuesTable issues={[issueWithPRs]} showPullRequestCount={true} />) expect(screen.getByText('3')).toBeInTheDocument() }) it('renders em-dash when pullRequestCount is 0', () => { const issueNoPRs: IssueRow = { objectID: 'no-pr-test', number: 201, title: 'Issue without PRs', state: 'open', labels: [], pullRequestCount: 0, } render(<IssuesTable issues={[issueNoPRs]} showPullRequestCount={true} />) expect(screen.getByText('—')).toBeInTheDocument() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/components/IssuesTable.test.tsx` around lines 528 - 540, Add tests to cover PR count cell rendering in IssuesTable: create IssueRow test fixtures (e.g., issueWithPRs and issueNoPRs) and render <IssuesTable issues={[...]} showPullRequestCount={true} /> then assert that when an item has pullRequestCount > 0 the numeric badge/text is present and when pullRequestCount is 0 or undefined the em-dash "—" is rendered; also add an optional test rendering the component in a narrow/mobile layout (e.g., by setting container width or using a mobile media query mock) to assert PR count visibility/formatting on small screens.frontend/src/components/Card.tsx (1)
141-149: Clean implementation of PR count badge.The conditional rendering correctly checks for both defined and positive values. The styling is consistent with the badge in
MenteeIssues.tsx.Same optional refactor applies for the pluralization condition on line 146 if you want to address the SonarCloud warning:
♻️ Optional refactor for positive condition
<span className="inline-flex items-center gap-1 rounded-full bg-blue-100 px-2 py-0.5 text-xs font-medium text-blue-700 dark:bg-blue-900/30 dark:text-blue-300"> <FaCodePullRequest className="h-3 w-3" /> - {pullRequestCount} open PR{pullRequestCount !== 1 ? 's' : ''} + {pullRequestCount} open PR{pullRequestCount === 1 ? '' : 's'} </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Card.tsx` around lines 141 - 149, The pluralization check in the PR badge uses "pullRequestCount !== 1 ? 's' : ''" which triggers a SonarCloud style warning; update the expression to use a clearer numeric comparison (pullRequestCount > 1) for pluralization and keep the existing conditional rendering (pullRequestCount !== undefined && pullRequestCount > 0) intact—change the ternary in the Card component where FaCodePullRequest and pullRequestCount are rendered so it reads the plural suffix based on pullRequestCount > 1.frontend/src/components/MenteeIssues.tsx (1)
89-95: Consider using a positive condition for pluralization (optional).SonarCloud flags the negated condition on line 93. While the current pattern is idiomatic for pluralization, you could use a ternary with positive logic for consistency with linting preferences:
♻️ Optional refactor for positive condition
<FaCodePullRequest className="h-3 w-3" /> - {issue.pullRequests.length} open PR - {issue.pullRequests.length !== 1 ? 's' : ''} + {issue.pullRequests.length} open PR{issue.pullRequests.length === 1 ? '' : 's'} </span>Alternatively, this is a valid pattern that's clear and readable—you may choose to suppress this warning if preferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/MenteeIssues.tsx` around lines 89 - 95, The pluralization uses a negated condition; update the ternary in the PR-count span (inside MenteeIssues.tsx where the FaCodePullRequest badge is rendered) to use a positive check instead: replace the "!== 1" branch with a positive equality check (issue.pullRequests.length === 1 ? '' : 's') so the logic is expressed positively while keeping the same rendered text and spacing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/IssuesTable.tsx`:
- Around line 207-219: The PR Count cell (controlled by showPullRequestCount and
rendering issue.pullRequestCount) is placed before Assignee and Deadline causing
header/body mismatch; remove the current PR Count block and re-insert it after
the Deadline cell in the table body so the order matches the header (Title →
Status → Labels → [Assignee] → [Deadline] → [Open PRs]); ensure you keep the
same conditional (showPullRequestCount) and rendering (issue.pullRequestCount
!== undefined && issue.pullRequestCount > 0 ? ... : "—") when moving it.
---
Nitpick comments:
In `@frontend/__tests__/unit/components/IssuesTable.test.tsx`:
- Around line 528-540: Add tests to cover PR count cell rendering in
IssuesTable: create IssueRow test fixtures (e.g., issueWithPRs and issueNoPRs)
and render <IssuesTable issues={[...]} showPullRequestCount={true} /> then
assert that when an item has pullRequestCount > 0 the numeric badge/text is
present and when pullRequestCount is 0 or undefined the em-dash "—" is rendered;
also add an optional test rendering the component in a narrow/mobile layout
(e.g., by setting container width or using a mobile media query mock) to assert
PR count visibility/formatting on small screens.
In `@frontend/src/components/Card.tsx`:
- Around line 141-149: The pluralization check in the PR badge uses
"pullRequestCount !== 1 ? 's' : ''" which triggers a SonarCloud style warning;
update the expression to use a clearer numeric comparison (pullRequestCount > 1)
for pluralization and keep the existing conditional rendering (pullRequestCount
!== undefined && pullRequestCount > 0) intact—change the ternary in the Card
component where FaCodePullRequest and pullRequestCount are rendered so it reads
the plural suffix based on pullRequestCount > 1.
In `@frontend/src/components/IssuesTable.tsx`:
- Around line 210-214: The PR count badge in IssuesTable (rendering block that
uses issue.pullRequestCount and FaCodePullRequest) only shows the number,
causing UI inconsistency with Card.tsx and MenteeIssues.tsx which render
"{count} open PRs"; update the badge rendering in IssuesTable.tsx to include the
descriptive text (e.g., "{issue.pullRequestCount} open PRs") while preserving
the same styling and icon so it matches Card and MenteeIssues components, or
intentionally document/leave it compact if table density is desired.
In `@frontend/src/components/MenteeIssues.tsx`:
- Around line 89-95: The pluralization uses a negated condition; update the
ternary in the PR-count span (inside MenteeIssues.tsx where the
FaCodePullRequest badge is rendered) to use a positive check instead: replace
the "!== 1" branch with a positive equality check (issue.pullRequests.length ===
1 ? '' : 's') so the logic is expressed positively while keeping the same
rendered text and spacing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb175730-ee56-4d8c-852c-0bb5d376dc9a
📒 Files selected for processing (7)
frontend/__tests__/unit/components/Card.test.tsxfrontend/__tests__/unit/components/IssuesTable.test.tsxfrontend/src/app/contribute/page.tsxfrontend/src/components/Card.tsxfrontend/src/components/IssuesTable.tsxfrontend/src/components/MenteeIssues.tsxfrontend/src/types/card.ts
Collaborator
|
@pranav-871 hi! Please address Sonar issues that were introduced in this PR: |
…SonarQube warnings
pranav-871
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
427edb0
|
Contributor
Author
|
Hi @kasya ! I've addressed both SonarQube issues, replaced negated conditions with positive equivalents in Card.tsx and MenteeIssues.tsx |
Contributor
|
Hii @pranav-871, nice work! Does this badge also show on mobile view? thanks! |
Contributor
Author
Hi! Yes, the badge is rendered inside a flex flex-wrap container so it adapts naturally to mobile screens as well. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Add an open pull request count indicator to issue cards on the Contribute page and Mentee issues view. When an issue has open pull requests, a blue badge showing the count is displayed on the card, helping contributors quickly identify issues that are already being worked on without clicking through to GitHub.
Resolves #4411
Changes made:
types/card.ts — added optional pullRequestCount?: number prop to CardProps
components/Card.tsx — renders a blue PR count badge when pullRequestCount > 0
app/contribute/page.tsx — passes issue.pullRequests?.length to the Card component
components/IssuesTable.tsx — added optional showPullRequestCount column with safe default of false
components/MenteeIssues.tsx — added PR count badge to the issue metadata row
Note: After investigating production Algolia records, pullRequests is not currently being indexed in the issues index. The frontend implementation is complete and handles missing data gracefully. A follow-up backend change will be needed to index this field where issues are synced to Algolia. Happy to work on that as well, please let me know if I should open a separate issue for the backend change!
Checklist