diff --git a/__tests__/components/pr/PRList.test.tsx b/__tests__/components/pr/PRList.test.tsx index decf04a0..ec8c0321 100644 --- a/__tests__/components/pr/PRList.test.tsx +++ b/__tests__/components/pr/PRList.test.tsx @@ -19,6 +19,7 @@ vi.mock("@/components/ui/button", () => ({ vi.mock("lucide-react", () => ({ GitPullRequest: () => , + Lightbulb: () => , })); // Mock PRListItem @@ -345,4 +346,151 @@ describe("PRList", () => { expect(screen.queryByText("Previous")).toBeNull(); expect(screen.queryByText("Next")).toBeNull(); }); + + // ── Mode-aware labels (#65) ───────────────────────────────────── + describe("standard mode", () => { + it("relabels the Merged/Closed tabs as Accepted/Rejected", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(screen.getByText("Accepted")).toBeDefined(); + expect(screen.getByText("Rejected")).toBeDefined(); + expect(screen.queryByText("Merged")).toBeNull(); + expect(screen.queryByText("Closed")).toBeNull(); + }); + + it("keeps developer wording in developer mode", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(screen.getByText("Merged")).toBeDefined(); + expect(screen.getByText("Closed")).toBeDefined(); + }); + + it("counts items as suggestions", async () => { + mockList.mockResolvedValue(makeListResponse([makePR()], 3)); + await act(async () => { + render(); + }); + expect(screen.getByText("3 suggestions")).toBeDefined(); + }); + + it("uses the singular noun for a single suggestion", async () => { + mockList.mockResolvedValue(makeListResponse([makePR()], 1)); + await act(async () => { + render(); + }); + expect(screen.getByText("1 suggestion")).toBeDefined(); + }); + }); + + // ── Author scoping (#65, admin/owner view) ────────────────────── + describe("author scoping", () => { + it("forwards authorId to the API so a 'My …' list only contains the viewer's", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(mockList).toHaveBeenCalledWith( + "proj-1", + undefined, + "open", + "user-42", + 0, + 20 + ); + }); + + it("omits authorId for reviewers so they see every contributor's", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(mockList).toHaveBeenCalledWith( + "proj-1", + undefined, + "open", + undefined, + 0, + 20 + ); + }); + + it("addresses the viewer directly in the empty state when scoped", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(screen.getByText("You have no open suggestions.")).toBeDefined(); + }); + + it("speaks about the project in the empty state when unscoped", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect( + screen.getByText("There are no open suggestions for this project.") + ).toBeDefined(); + }); + + it("refetches when the author scope changes", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + const { rerender } = render(); + await waitFor(() => expect(mockList).toHaveBeenCalledTimes(1)); + await act(async () => { + rerender(); + }); + await waitFor(() => expect(mockList).toHaveBeenCalledTimes(2)); + expect(mockList).toHaveBeenLastCalledWith( + "proj-1", + undefined, + "open", + "user-42", + 0, + 20 + ); + }); + }); + + // ── Mode-aware empty-state icon ───────────────────────────────── + it("uses the lightbulb icon in the standard-mode empty state", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(screen.getByTestId("icon-lightbulb")).toBeDefined(); + expect(screen.queryByTestId("icon-git-pr")).toBeNull(); + }); + + it("keeps the pull-request icon in the developer-mode empty state", async () => { + mockList.mockResolvedValue(makeListResponse([], 0)); + await act(async () => { + render(); + }); + expect(screen.getByTestId("icon-git-pr")).toBeDefined(); + expect(screen.queryByTestId("icon-lightbulb")).toBeNull(); + }); + + it("resets pagination when the author scope changes", async () => { + mockList.mockResolvedValue(makeListResponse([makePR()], 40)); + const { rerender } = render(); + await waitFor(() => expect(mockList).toHaveBeenCalledTimes(1)); + + await act(async () => { + fireEvent.click(screen.getByText("Next")); + }); + await waitFor(() => + expect(mockList).toHaveBeenLastCalledWith("proj-1", undefined, "open", undefined, 20, 20) + ); + + await act(async () => { + rerender(); + }); + await waitFor(() => + expect(mockList).toHaveBeenLastCalledWith("proj-1", undefined, "open", "user-42", 0, 20) + ); + }); }); diff --git a/app/projects/[id]/editor/page.tsx b/app/projects/[id]/editor/page.tsx index 186e3773..b64739a7 100644 --- a/app/projects/[id]/editor/page.tsx +++ b/app/projects/[id]/editor/page.tsx @@ -927,15 +927,26 @@ export default function EditorPage() { )} - {/* Suggestions link */} - {isSuggestionMode && ( - - - - )} + {/* Suggestions / PRs link — unified, mode-aware */} + + + {/* Review Suggestions link (editors/admins only) */} {canEdit && pendingSuggestionCount > 0 && ( @@ -1022,19 +1033,6 @@ export default function EditorPage() { )} - {/* PR Link */} - - - - )} {/* PR List */} - + {/* Create Modal */} {session?.accessToken && ( diff --git a/components/pr/PRActions.tsx b/components/pr/PRActions.tsx index e36afe09..c6b6b5e7 100644 --- a/components/pr/PRActions.tsx +++ b/components/pr/PRActions.tsx @@ -22,12 +22,14 @@ import { } from "lucide-react"; import Link from "next/link"; import { branchesApi } from "@/lib/api/revisions"; +import type { EditorMode } from "@/lib/stores/editorModeStore"; interface PRActionsProps { projectId: string; pr: PullRequest; accessToken: string; userRole?: string; + mode?: EditorMode; onUpdate: (pr: PullRequest) => void; className?: string; } @@ -37,9 +39,11 @@ export function PRActions({ pr, accessToken, userRole, + mode = "developer", onUpdate, className, }: PRActionsProps) { + const isSuggestionMode = mode === "standard"; const [isSubmitting, setIsSubmitting] = useState(false); const [error, setError] = useState(null); const [showReviewForm, setShowReviewForm] = useState(false); @@ -160,7 +164,8 @@ export function PRActions({

- This pull request was merged into {pr.target_branch}. + {isSuggestionMode ? "This suggestion was accepted into " : "This pull request was merged into "} + {pr.target_branch}.

@@ -252,7 +257,7 @@ export function PRActions({ open={showReopenDialog} onOpenChange={setShowReopenDialog} onConfirm={handleReopen} - title="Reopen Pull Request" + title={isSuggestionMode ? "Reopen Suggestion" : "Reopen Pull Request"} description={`Are you sure you want to reopen "${pr.title}"?`} confirmLabel="Reopen" variant="default" @@ -341,7 +346,7 @@ export function PRActions({ Review - {/* Merge button */} + {/* Merge / Accept button */} {canMerge && ( )} - {/* Close button */} + {/* Close / Reject button */}
)} @@ -382,9 +389,11 @@ export function PRActions({ open={showMergeDialog} onOpenChange={setShowMergeDialog} onConfirm={handleMerge} - title="Merge Pull Request" - description={`Are you sure you want to merge "${pr.title}" into ${pr.target_branch}?`} - confirmLabel="Merge" + title={isSuggestionMode ? "Accept Suggestion" : "Merge Pull Request"} + description={isSuggestionMode + ? `Are you sure you want to accept "${pr.title}" into ${pr.target_branch}?` + : `Are you sure you want to merge "${pr.title}" into ${pr.target_branch}?`} + confirmLabel={isSuggestionMode ? "Accept" : "Merge"} variant="default" > @@ -405,16 +415,20 @@ export function PRActions({ open={showCloseDialog} onOpenChange={setShowCloseDialog} onConfirm={handleClose} - title="Close Pull Request" - description={`Are you sure you want to close "${pr.title}"? This will not delete the branch and can be reopened later.`} - confirmLabel="Close PR" + title={isSuggestionMode ? "Reject Suggestion" : "Close Pull Request"} + description={isSuggestionMode + ? `Are you sure you want to reject "${pr.title}"? This will not delete the branch and can be reopened later.` + : `Are you sure you want to close "${pr.title}"? This will not delete the branch and can be reopened later.`} + confirmLabel={isSuggestionMode ? "Reject" : "Close PR"} variant="danger" /> {/* Merge status */} {!pr.can_merge && canMerge && (

- This pull request requires additional approvals before it can be merged. + {isSuggestionMode + ? "This suggestion requires additional approvals before it can be accepted." + : "This pull request requires additional approvals before it can be merged."}{" "} Current: {pr.approval_count} approvals

)} diff --git a/components/pr/PRDetail.tsx b/components/pr/PRDetail.tsx index 828d19d6..c961fa8e 100644 --- a/components/pr/PRDetail.tsx +++ b/components/pr/PRDetail.tsx @@ -13,6 +13,7 @@ import { PRActions } from "./PRActions"; import { PRCommentThread } from "./PRCommentThread"; import { Button } from "@/components/ui/button"; import { cn } from "@/lib/utils"; +import type { EditorMode } from "@/lib/stores/editorModeStore"; import { GitPullRequest, GitMerge, @@ -39,6 +40,7 @@ interface PRDetailProps { accessToken?: string; userRole?: string; currentUserId?: string; + mode?: EditorMode; className?: string; } @@ -48,8 +50,10 @@ export function PRDetail({ accessToken, userRole, currentUserId, + mode = "developer", className, }: PRDetailProps) { + const isSuggestionMode = mode === "standard"; const [pr, setPR] = useState(null); const [reviews, setReviews] = useState([]); const [comments, setComments] = useState([]); @@ -204,13 +208,13 @@ export function PRDetail({ case "merged": return ( - Merged + {isSuggestionMode ? "Accepted" : "Merged"} ); case "closed": return ( - Closed + {isSuggestionMode ? "Rejected" : "Closed"} ); default: @@ -267,7 +271,7 @@ export function PRDetail({ {pr.status === "merged" && pr.merged_at - ? `merged ${formatDate(pr.merged_at)}` + ? `${isSuggestionMode ? "accepted" : "merged"} ${formatDate(pr.merged_at)}` : `opened ${formatDate(pr.created_at)}`} {pr.github_pr_url && ( @@ -301,6 +305,7 @@ export function PRDetail({ pr={pr} accessToken={accessToken} userRole={userRole} + mode={mode} onUpdate={setPR} /> )} diff --git a/components/pr/PRList.tsx b/components/pr/PRList.tsx index 8094e1f8..7da91c33 100644 --- a/components/pr/PRList.tsx +++ b/components/pr/PRList.tsx @@ -8,12 +8,20 @@ import { } from "@/lib/api/pullRequests"; import { PRListItem } from "./PRListItem"; import { cn } from "@/lib/utils"; -import { GitPullRequest } from "lucide-react"; +import { GitPullRequest, Lightbulb } from "lucide-react"; +import type { EditorMode } from "@/lib/stores/editorModeStore"; interface PRListProps { projectId: string; accessToken?: string; defaultStatus?: PRStatus | "all"; + mode?: EditorMode; + /** + * Restrict the list to one author. Reviewers (owner/admin) leave this unset + * to see everyone's; everyone else passes their own id so a list titled + * "My Suggestions" / "My Pull Requests" actually only contains theirs. + */ + authorId?: string; className?: string; } @@ -21,8 +29,11 @@ export function PRList({ projectId, accessToken, defaultStatus = "open", + mode = "developer", + authorId, className, }: PRListProps) { + const isSuggestionMode = mode === "standard"; const [prs, setPrs] = useState([]); const [total, setTotal] = useState(0); const [isLoading, setIsLoading] = useState(true); @@ -31,35 +42,53 @@ export function PRList({ const [skip, setSkip] = useState(0); const limit = 20; - const loadPRs = useCallback(async () => { - if (!projectId) return; + // A reviewer on page 3 who switches to their own (much shorter) list would + // otherwise keep the stale offset and land on an empty page. + const [scopeKey, setScopeKey] = useState(authorId); + if (scopeKey !== authorId) { + setScopeKey(authorId); + setSkip(0); + } - setIsLoading(true); - setError(null); + const loadPRs = useCallback( + async (signal?: { cancelled: boolean }) => { + if (!projectId) return; - try { - const status = statusFilter === "all" ? undefined : statusFilter; - const response = await pullRequestsApi.list( - projectId, - accessToken, - status, - undefined, - skip, - limit - ); - setPrs(response.items); - setTotal(response.total); - } catch (err) { - const message = - err instanceof Error ? err.message : "Failed to load pull requests"; - setError(message); - } finally { - setIsLoading(false); - } - }, [projectId, accessToken, statusFilter, skip]); + setIsLoading(true); + setError(null); + + try { + const status = statusFilter === "all" ? undefined : statusFilter; + const response = await pullRequestsApi.list( + projectId, + accessToken, + status, + authorId, + skip, + limit + ); + // A slower request for a previous scope must not overwrite the newer one. + if (signal?.cancelled) return; + setPrs(response.items); + setTotal(response.total); + } catch (err) { + if (signal?.cancelled) return; + const message = + err instanceof Error ? err.message : "Failed to load pull requests"; + setError(message); + } finally { + if (!signal?.cancelled) setIsLoading(false); + } + }, + [projectId, accessToken, statusFilter, authorId, skip] + ); useEffect(() => { - loadPRs(); + const signal = { cancelled: false }; + loadPRs(signal); + return () => { + signal.cancelled = true; + }; }, [loadPRs]); const handleStatusChange = (newStatus: PRStatus | "all") => { @@ -67,10 +96,39 @@ export function PRList({ setSkip(0); }; + // The list is either scoped to the signed-in user (authorId set) or shows the + // whole project, so the empty state has to speak in the matching voice — + // "You have no…" vs "There are no…". + const isScopedToViewer = !!authorId; + const noun = isSuggestionMode ? "suggestion" : "pull request"; + const emptyStateMessage = (() => { + if (statusFilter === "open") { + return isScopedToViewer + ? `You have no open ${noun}s.` + : `There are no open ${noun}s for this project.`; + } + if (statusFilter === "merged") { + const verb = isSuggestionMode ? "accepted" : "merged"; + return isScopedToViewer + ? `None of your ${noun}s have been ${verb} yet.` + : `No ${noun}s have been ${verb} yet.`; + } + if (statusFilter === "closed") { + const verb = isSuggestionMode ? "rejected" : "closed"; + return isScopedToViewer + ? `None of your ${noun}s have been ${verb}.` + : `No ${noun}s have been ${verb}.`; + } + const verb = isSuggestionMode ? "submitted" : "created"; + return isScopedToViewer + ? `You have not ${verb} any ${noun}s yet.` + : `No ${noun}s have been ${verb} for this project.`; + })(); + const statusTabs: { value: PRStatus | "all"; label: string }[] = [ { value: "open", label: "Open" }, - { value: "merged", label: "Merged" }, - { value: "closed", label: "Closed" }, + { value: "merged", label: isSuggestionMode ? "Accepted" : "Merged" }, + { value: "closed", label: isSuggestionMode ? "Rejected" : "Closed" }, { value: "all", label: "All" }, ]; @@ -96,7 +154,7 @@ export function PRList({
- {total} pull request{total !== 1 ? "s" : ""} + {total} {total === 1 ? noun : `${noun}s`}
@@ -111,26 +169,22 @@ export function PRList({ ) : prs.length === 0 ? (
- + {isSuggestionMode ? ( + + ) : ( + + )}

- No pull requests + {isSuggestionMode ? "No suggestions" : "No pull requests"}

-

- {statusFilter === "open" - ? "There are no open pull requests for this project." - : statusFilter === "merged" - ? "No pull requests have been merged yet." - : statusFilter === "closed" - ? "No pull requests have been closed." - : "No pull requests have been created for this project."} -

+

{emptyStateMessage}

) : ( <> {/* PR List */}
{prs.map((pr) => ( - + ))}
diff --git a/components/pr/PRListItem.tsx b/components/pr/PRListItem.tsx index a386b76c..25f3f4cc 100644 --- a/components/pr/PRListItem.tsx +++ b/components/pr/PRListItem.tsx @@ -12,14 +12,17 @@ import { User, } from "lucide-react"; import Link from "next/link"; +import type { EditorMode } from "@/lib/stores/editorModeStore"; interface PRListItemProps { pr: PullRequest; projectId: string; + mode?: EditorMode; className?: string; } -export function PRListItem({ pr, projectId, className }: PRListItemProps) { +export function PRListItem({ pr, projectId, mode = "developer", className }: PRListItemProps) { + const isSuggestionMode = mode === "standard"; const formatDate = (timestamp: string) => { const date = new Date(timestamp); const now = new Date(); @@ -56,13 +59,13 @@ export function PRListItem({ pr, projectId, className }: PRListItemProps) { case "merged": return ( - Merged + {isSuggestionMode ? "Accepted" : "Merged"} ); case "closed": return ( - Closed + {isSuggestionMode ? "Rejected" : "Closed"} ); default: @@ -116,7 +119,7 @@ export function PRListItem({ pr, projectId, className }: PRListItemProps) { {pr.status === "merged" && pr.merged_at - ? `merged ${formatDate(pr.merged_at)}` + ? `${isSuggestionMode ? "accepted" : "merged"} ${formatDate(pr.merged_at)}` : `opened ${formatDate(pr.created_at)}`} diff --git a/node_modules b/node_modules new file mode 120000 index 00000000..a5df6637 --- /dev/null +++ b/node_modules @@ -0,0 +1 @@ +/home/johnrdorazio/development/CatholicOS_org/ontokit/ontokit-web/node_modules \ No newline at end of file