mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: stack insights tables vertically and paginate Pull requests table (#24198)
The "By model" and "Pull requests" tables on the PR Insights page (`/agents/settings/insights`) were side-by-side at `lg` breakpoints, and the Pull requests table was hard-capped at 20 rows by the backend. - Replaced `lg:grid-cols-2` with a single-column stacked layout so both tables span the full content width. - Removed the `LIMIT 20` from the `GetPRInsightsRecentPRs` SQL query so all PRs in the selected time range are returned. - Can add this back if we need it. If we do, we should add a little subheader above this table to indicate that we're not showing all PRs within the selected timeframe. - Added client-side pagination to the Pull requests table using `PaginationWidgetBase` (page size 10), matching the existing pattern in `ChatCostSummaryView`. - Renamed the section heading from "Recent" to "Pull requests" since it now shows the full set for the time range. <img width="1481" height="1817" alt="image" src="https://github.com/user-attachments/assets/0066c42f-4d7b-4cee-b64b-6680848edc68" /> > 🤖 PR generated with Coder Agents
This commit is contained in:
@@ -21,6 +21,7 @@ import {
|
||||
} from "#/components/Tooltip/Tooltip";
|
||||
import { formatTokenCount } from "#/utils/analytics";
|
||||
import { formatCostMicros } from "#/utils/currency";
|
||||
import { paginateItems } from "#/utils/paginateItems";
|
||||
|
||||
interface ChatCostSummaryViewProps {
|
||||
summary: TypesGen.ChatCostSummary | undefined;
|
||||
@@ -95,25 +96,19 @@ export const ChatCostSummaryView: FC<ChatCostSummaryViewProps> = ({
|
||||
}
|
||||
|
||||
const modelPageSize = 10;
|
||||
const modelMaxPage = Math.max(
|
||||
1,
|
||||
Math.ceil(summary.by_model.length / modelPageSize),
|
||||
);
|
||||
const clampedModelPage = Math.min(modelPage, modelMaxPage);
|
||||
const pagedModels = summary.by_model.slice(
|
||||
(clampedModelPage - 1) * modelPageSize,
|
||||
clampedModelPage * modelPageSize,
|
||||
);
|
||||
const {
|
||||
pagedItems: pagedModels,
|
||||
clampedPage: clampedModelPage,
|
||||
hasPreviousPage: hasModelPrev,
|
||||
hasNextPage: hasModelNext,
|
||||
} = paginateItems(summary.by_model, modelPageSize, modelPage);
|
||||
const chatPageSize = 10;
|
||||
const chatMaxPage = Math.max(
|
||||
1,
|
||||
Math.ceil(summary.by_chat.length / chatPageSize),
|
||||
);
|
||||
const clampedChatPage = Math.min(chatPage, chatMaxPage);
|
||||
const pagedChats = summary.by_chat.slice(
|
||||
(clampedChatPage - 1) * chatPageSize,
|
||||
clampedChatPage * chatPageSize,
|
||||
);
|
||||
const {
|
||||
pagedItems: pagedChats,
|
||||
clampedPage: clampedChatPage,
|
||||
hasPreviousPage: hasChatPrev,
|
||||
hasNextPage: hasChatNext,
|
||||
} = paginateItems(summary.by_chat, chatPageSize, chatPage);
|
||||
|
||||
const usageLimit = summary.usage_limit;
|
||||
const showUsageLimitCard = usageLimit?.is_limited === true;
|
||||
@@ -333,10 +328,8 @@ export const ChatCostSummaryView: FC<ChatCostSummaryViewProps> = ({
|
||||
currentPage={clampedModelPage}
|
||||
pageSize={modelPageSize}
|
||||
onPageChange={setModelPage}
|
||||
hasPreviousPage={clampedModelPage > 1}
|
||||
hasNextPage={
|
||||
clampedModelPage * modelPageSize < summary.by_model.length
|
||||
}
|
||||
hasPreviousPage={hasModelPrev}
|
||||
hasNextPage={hasModelNext}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
@@ -403,10 +396,8 @@ export const ChatCostSummaryView: FC<ChatCostSummaryViewProps> = ({
|
||||
currentPage={clampedChatPage}
|
||||
pageSize={chatPageSize}
|
||||
onPageChange={setChatPage}
|
||||
hasPreviousPage={clampedChatPage > 1}
|
||||
hasNextPage={
|
||||
clampedChatPage * chatPageSize < summary.by_chat.length
|
||||
}
|
||||
hasPreviousPage={hasChatPrev}
|
||||
hasNextPage={hasChatNext}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import dayjs from "dayjs";
|
||||
import relativeTime from "dayjs/plugin/relativeTime";
|
||||
import { CodeIcon, ExternalLinkIcon } from "lucide-react";
|
||||
import type { FC } from "react";
|
||||
import { type FC, useState } from "react";
|
||||
import { Area, AreaChart, CartesianGrid, XAxis, YAxis } from "recharts";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import { Button } from "#/components/Button/Button";
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
ChartTooltip,
|
||||
ChartTooltipContent,
|
||||
} from "#/components/Chart/Chart";
|
||||
import { PaginationWidgetBase } from "#/components/PaginationWidget/PaginationWidgetBase";
|
||||
import {
|
||||
Table,
|
||||
TableBody,
|
||||
@@ -21,6 +22,7 @@ import {
|
||||
} from "#/components/Table/Table";
|
||||
import { cn } from "#/utils/cn";
|
||||
import { formatCostMicros } from "#/utils/currency";
|
||||
import { paginateItems } from "#/utils/paginateItems";
|
||||
import { PrStateIcon } from "./GitPanel/GitPanel";
|
||||
|
||||
dayjs.extend(relativeTime);
|
||||
@@ -286,6 +288,8 @@ const TimeRangeFilter: FC<{
|
||||
// Main view
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const RECENT_PRS_PAGE_SIZE = 10;
|
||||
|
||||
export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
data,
|
||||
timeRange,
|
||||
@@ -294,6 +298,18 @@ export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
const { summary, time_series, by_model, recent_prs } = data;
|
||||
const isEmpty = summary.total_prs_created === 0;
|
||||
|
||||
// Client-side pagination for recent PRs table.
|
||||
// Page resets to 1 on data refresh because the parent unmounts this
|
||||
// component during loading. Clamping ensures the page is valid if the
|
||||
// list shrinks without a full remount.
|
||||
const [recentPrsPage, setRecentPrsPage] = useState(1);
|
||||
const {
|
||||
pagedItems: pagedRecentPrs,
|
||||
clampedPage: clampedRecentPrsPage,
|
||||
hasPreviousPage: hasRecentPrsPrev,
|
||||
hasNextPage: hasRecentPrsNext,
|
||||
} = paginateItems(recent_prs, RECENT_PRS_PAGE_SIZE, recentPrsPage);
|
||||
|
||||
return (
|
||||
<div className="space-y-8">
|
||||
{/* ── Header ── */}
|
||||
@@ -354,8 +370,8 @@ export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
</div>
|
||||
</section>
|
||||
|
||||
{/* ── Model breakdown + Recent PRs side by side ── */}
|
||||
<div className="grid grid-cols-1 gap-6 lg:grid-cols-2">
|
||||
{/* ── Model breakdown + Recent PRs ── */}
|
||||
<div className="space-y-6">
|
||||
{/* ── Model performance (simplified) ── */}
|
||||
{by_model.length > 0 && (
|
||||
<section>
|
||||
@@ -413,7 +429,7 @@ export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
{recent_prs.length > 0 && (
|
||||
<section>
|
||||
<div className="mb-4">
|
||||
<SectionTitle>Recent</SectionTitle>
|
||||
<SectionTitle>Pull requests</SectionTitle>
|
||||
</div>
|
||||
<div className="overflow-hidden rounded-lg border border-border-default">
|
||||
<Table className="table-fixed text-sm">
|
||||
@@ -436,7 +452,7 @@ export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
</TableRow>
|
||||
</TableHeader>{" "}
|
||||
<TableBody>
|
||||
{recent_prs.map((pr) => (
|
||||
{pagedRecentPrs.map((pr) => (
|
||||
<TableRow
|
||||
key={pr.chat_id}
|
||||
className="border-t border-border-default transition-colors hover:bg-surface-secondary/50"
|
||||
@@ -480,6 +496,18 @@ export const PRInsightsView: FC<PRInsightsViewProps> = ({
|
||||
</TableBody>
|
||||
</Table>
|
||||
</div>
|
||||
{recent_prs.length > RECENT_PRS_PAGE_SIZE && (
|
||||
<div className="pt-4">
|
||||
<PaginationWidgetBase
|
||||
totalRecords={recent_prs.length}
|
||||
currentPage={clampedRecentPrsPage}
|
||||
pageSize={RECENT_PRS_PAGE_SIZE}
|
||||
onPageChange={setRecentPrsPage}
|
||||
hasPreviousPage={hasRecentPrsPrev}
|
||||
hasNextPage={hasRecentPrsNext}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</section>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { paginateItems } from "./paginateItems";
|
||||
|
||||
// 25 items numbered 1–25 for readable assertions.
|
||||
const items = Array.from({ length: 25 }, (_, i) => i + 1);
|
||||
|
||||
describe("paginateItems", () => {
|
||||
it("returns the first page of items", () => {
|
||||
const result = paginateItems(items, 10, 1);
|
||||
expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
|
||||
expect(result.clampedPage).toBe(1);
|
||||
expect(result.totalPages).toBe(3);
|
||||
expect(result.hasPreviousPage).toBe(false);
|
||||
expect(result.hasNextPage).toBe(true);
|
||||
});
|
||||
|
||||
it("returns a partial last page", () => {
|
||||
const result = paginateItems(items, 10, 3);
|
||||
expect(result.pagedItems).toEqual([21, 22, 23, 24, 25]);
|
||||
expect(result.clampedPage).toBe(3);
|
||||
expect(result.totalPages).toBe(3);
|
||||
expect(result.hasPreviousPage).toBe(true);
|
||||
expect(result.hasNextPage).toBe(false);
|
||||
});
|
||||
|
||||
it("clamps currentPage down when beyond total pages", () => {
|
||||
const result = paginateItems(items, 10, 99);
|
||||
expect(result.clampedPage).toBe(3);
|
||||
expect(result.pagedItems).toEqual([21, 22, 23, 24, 25]);
|
||||
expect(result.hasPreviousPage).toBe(true);
|
||||
expect(result.hasNextPage).toBe(false);
|
||||
});
|
||||
|
||||
it("clamps currentPage up when 0", () => {
|
||||
const result = paginateItems(items, 10, 0);
|
||||
expect(result.clampedPage).toBe(1);
|
||||
expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
|
||||
expect(result.hasPreviousPage).toBe(false);
|
||||
expect(result.hasNextPage).toBe(true);
|
||||
});
|
||||
|
||||
it("clamps currentPage up when negative", () => {
|
||||
const result = paginateItems(items, 10, -5);
|
||||
expect(result.clampedPage).toBe(1);
|
||||
expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
|
||||
expect(result.hasPreviousPage).toBe(false);
|
||||
expect(result.hasNextPage).toBe(true);
|
||||
});
|
||||
|
||||
it("returns empty pagedItems with clampedPage=1 for an empty array", () => {
|
||||
const result = paginateItems([], 10, 1);
|
||||
expect(result.pagedItems).toEqual([]);
|
||||
expect(result.clampedPage).toBe(1);
|
||||
expect(result.totalPages).toBe(1);
|
||||
expect(result.hasPreviousPage).toBe(false);
|
||||
expect(result.hasNextPage).toBe(false);
|
||||
});
|
||||
|
||||
it("reports hasPreviousPage correctly for middle pages", () => {
|
||||
const result = paginateItems(items, 10, 2);
|
||||
expect(result.hasPreviousPage).toBe(true);
|
||||
expect(result.hasNextPage).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,25 @@
|
||||
export function paginateItems<T>(
|
||||
items: readonly T[],
|
||||
pageSize: number,
|
||||
currentPage: number,
|
||||
): {
|
||||
pagedItems: T[];
|
||||
clampedPage: number;
|
||||
totalPages: number;
|
||||
hasPreviousPage: boolean;
|
||||
hasNextPage: boolean;
|
||||
} {
|
||||
const totalPages = Math.max(1, Math.ceil(items.length / pageSize));
|
||||
const clampedPage = Math.max(1, Math.min(currentPage, totalPages));
|
||||
const pagedItems = items.slice(
|
||||
(clampedPage - 1) * pageSize,
|
||||
clampedPage * pageSize,
|
||||
);
|
||||
return {
|
||||
pagedItems,
|
||||
clampedPage,
|
||||
totalPages,
|
||||
hasPreviousPage: clampedPage > 1,
|
||||
hasNextPage: clampedPage * pageSize < items.length,
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user