feat(dashboard): add PRs page with project filter and run counts #688
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| import { ExternalLink } from 'lucide-react'; | ||
|
|
||
| interface PR { | ||
| prNumber: number; | ||
| repoFullName: string; | ||
| prUrl: string | null; | ||
| prTitle: string | null; | ||
| workItemId: string | null; | ||
| workItemUrl: string | null; | ||
| workItemTitle: string | null; | ||
| runCount: number; | ||
| } | ||
|
|
||
| interface PRsTableProps { | ||
| items: PR[]; | ||
| offset: number; | ||
| limit: number; | ||
| onPageChange: (offset: number) => void; | ||
| showRepoName?: boolean; | ||
| } | ||
|
|
||
| export function PRsTable({ | ||
| items, | ||
| offset, | ||
| limit, | ||
| onPageChange, | ||
| showRepoName = false, | ||
| }: PRsTableProps) { | ||
| const total = items.length; | ||
| const totalPages = Math.ceil(total / limit); | ||
| const currentPage = Math.floor(offset / limit) + 1; | ||
| const pageItems = items.slice(offset, offset + limit); | ||
|
|
||
| return ( | ||
| <div className="space-y-4"> | ||
| <div className="overflow-x-auto rounded-lg border border-border"> | ||
| <table className="w-full text-sm"> | ||
| <thead> | ||
| <tr className="border-b border-border bg-muted/50"> | ||
| <th className="px-4 py-3 text-left font-medium text-muted-foreground">PR</th> | ||
| {showRepoName && ( | ||
| <th className="px-4 py-3 text-left font-medium text-muted-foreground"> | ||
| Repository | ||
| </th> | ||
| )} | ||
| <th className="px-4 py-3 text-left font-medium text-muted-foreground">Work Item</th> | ||
| <th className="px-4 py-3 text-right font-medium text-muted-foreground">Runs</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {pageItems.length === 0 && ( | ||
| <tr> | ||
| <td | ||
| colSpan={showRepoName ? 4 : 3} | ||
| className="px-4 py-8 text-center text-muted-foreground" | ||
| > | ||
| No PRs found | ||
| </td> | ||
| </tr> | ||
| )} | ||
| {pageItems.map((item) => ( | ||
| <tr | ||
|
Collaborator
There was a problem hiding this comment. [SHOULD_FIX] Fix: use a composite key: key={`${item.repoFullName}-${item.prNumber}`}Also consider:
Collaborator
Author
There was a problem hiding this comment. Fixed! Changed the React key to use a composite key Also added a |
||
| key={`${item.repoFullName}-${item.prNumber}`} | ||
| className="border-b border-border transition-colors hover:bg-muted/30" | ||
| > | ||
| <td className="px-4 py-3"> | ||
| {item.prUrl ? ( | ||
| <a | ||
| href={item.prUrl} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="inline-flex items-center gap-1 text-primary hover:underline" | ||
| > | ||
| #{item.prNumber} | ||
| {item.prTitle && <span className="ml-1 text-foreground">{item.prTitle}</span>} | ||
| <ExternalLink className="h-3 w-3 shrink-0" /> | ||
| </a> | ||
| ) : ( | ||
| <span className="text-muted-foreground"> | ||
| #{item.prNumber} | ||
| {item.prTitle && <span className="ml-1 text-foreground">{item.prTitle}</span>} | ||
| </span> | ||
| )} | ||
| </td> | ||
| {showRepoName && ( | ||
| <td className="px-4 py-3 text-muted-foreground">{item.repoFullName}</td> | ||
| )} | ||
| <td className="px-4 py-3"> | ||
| {item.workItemUrl && item.workItemTitle ? ( | ||
| <a | ||
| href={item.workItemUrl} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="inline-flex items-center gap-1 text-primary hover:underline" | ||
| > | ||
| {item.workItemTitle} | ||
| <ExternalLink className="h-3 w-3 shrink-0" /> | ||
| </a> | ||
| ) : item.workItemTitle ? ( | ||
| <span>{item.workItemTitle}</span> | ||
| ) : ( | ||
| <span className="text-muted-foreground italic">Unlinked</span> | ||
| )} | ||
| </td> | ||
| <td className="px-4 py-3 text-right tabular-nums">{item.runCount}</td> | ||
| </tr> | ||
| ))} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
|
|
||
| {total > limit && ( | ||
| <div className="flex flex-col gap-2 sm:flex-row sm:items-center sm:justify-between"> | ||
| <div className="text-sm text-muted-foreground"> | ||
| Showing {offset + 1}–{Math.min(offset + limit, total)} of {total} | ||
| </div> | ||
| <div className="flex gap-2"> | ||
| <button | ||
| type="button" | ||
| onClick={() => onPageChange(Math.max(0, offset - limit))} | ||
| disabled={offset === 0} | ||
| className="inline-flex h-8 items-center rounded-md border border-input px-3 text-sm transition-colors hover:bg-accent disabled:pointer-events-none disabled:opacity-50" | ||
| > | ||
| Previous | ||
| </button> | ||
| <span className="inline-flex h-8 items-center px-2 text-sm text-muted-foreground"> | ||
| Page {currentPage} of {totalPages} | ||
| </span> | ||
| <button | ||
| type="button" | ||
| onClick={() => onPageChange(offset + limit)} | ||
| disabled={offset + limit >= total} | ||
| className="inline-flex h-8 items-center rounded-md border border-input px-3 text-sm transition-colors hover:bg-accent disabled:pointer-events-none disabled:opacity-50" | ||
| > | ||
| Next | ||
| </button> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[SHOULD_FIX] The
listPRsForOrgpath is not covered by unit tests. The mock setup inprs.test.tsdoesn't mocklistPRsForOrg, so callinglist({})(no projectId) would hit an undefined function. Add a test case for the org-wide path and mocklistPRsForOrg.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Added
listPRsForOrgto the mock setup and created a new test case "returns PRs across all projects when no projectId given" that tests the org-wide query path. The test verifies that whenlist({})is called without a projectId, it callslistPRsForOrgwith the org ID and skips the project access verification.