Skip to content

fix(ui): paginate skipped-PR audit feed with offset append #7503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jsdevninja wants to merge 1 commit into from
Closed

fix(ui): paginate skipped-PR audit feed with offset append #7503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions apps/loopover-ui/public/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11061,11 +11061,21 @@
"remediation"
]
}
},
"offset": {
"type": "integer",
"minimum": 0
},
"total": {
"type": "integer",
"minimum": 0
}
},
"required": [
"generatedAt",
"limit",
"offset",
"total",
"hasMore",
"filters",
"items"
Expand Down Expand Up @@ -17892,10 +17902,20 @@
"example": "50"
},
"required": false,
"description": "Maximum rows to return, clamped from 1 to 100.",
"description": "Maximum rows to return per page, clamped from 1 to 100.",
"name": "limit",
"in": "query"
},
{
"schema": {
"type": "string",
"example": "0"
},
"required": false,
"description": "Zero-based row offset for pagination. Advance by `limit` (or the previous page's item count) and append; do not grow `limit` to page.",
"name": "offset",
"in": "query"
},
{
"schema": {
"type": "string",
Expand All @@ -17909,6 +17929,7 @@
{
"schema": {
"type": "string",
"example": "not_official_gittensor_miner",
"enum": [
"surface_off",
"missing_author",
Expand All @@ -17917,8 +17938,7 @@
"maintainer_author",
"miner_detection_unavailable",
"not_official_gittensor_miner"
],
"example": "not_official_gittensor_miner"
]
},
"required": false,
"description": "Optional PR skip reason filter.",
Expand Down
23 changes: 23 additions & 0 deletions apps/loopover-ui/src/components/site/audit-feed-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export type SkippedPrAuditItem = {
export type SkippedPrAuditExport = {
generatedAt: string;
limit: number;
offset: number;
total: number;
hasMore: boolean;
filters: {
repoFullName: string | null;
Expand All @@ -36,14 +38,21 @@ export const SKIP_REASON_OPTIONS: Array<{ value: "" | SkippedPrAuditReason; labe
{ value: "not_official_gittensor_miner", label: "Not official Gittensor miner" },
];

/** Page size for each skipped-PR audit request. Load more advances `offset` by this amount (#7438). */
export const AUDIT_FEED_PAGE_SIZE = 50;
/** Cap on how many rows the UI will accumulate client-side before asking the user to narrow filters. */
export const AUDIT_FEED_MAX_ITEMS = 100;

export function buildSkippedPrAuditPath(options: {
limit: number;
offset?: number;
repoFullName?: string;
reason?: SkippedPrAuditReason;
since?: string;
}): string {
const params = new URLSearchParams();
params.set("limit", String(options.limit));
params.set("offset", String(Math.max(0, options.offset ?? 0)));
if (options.repoFullName?.trim()) params.set("repoFullName", options.repoFullName.trim());
if (options.reason) params.set("reason", options.reason);
if (options.since?.trim()) params.set("since", options.since.trim());
Expand Down Expand Up @@ -83,6 +92,8 @@ export function normalizeSkippedPrAuditExport(data: unknown): SkippedPrAuditExpo
return {
generatedAt: raw.generatedAt,
limit: typeof raw.limit === "number" ? raw.limit : items.length,
offset: typeof raw.offset === "number" ? raw.offset : 0,
total: typeof raw.total === "number" ? raw.total : items.length,
hasMore: Boolean(raw.hasMore),
filters: {
repoFullName:
Expand All @@ -97,6 +108,18 @@ export function normalizeSkippedPrAuditExport(data: unknown): SkippedPrAuditExpo
};
}

/** Append a newly fetched page onto the already-rendered list without re-deriving prior rows (#7438). */
export function appendSkippedPrAuditPage(
current: SkippedPrAuditExport | null,
nextPage: SkippedPrAuditExport,
): SkippedPrAuditExport {
if (!current || nextPage.offset === 0) return nextPage;
return {
...nextPage,
items: [...current.items, ...nextPage.items],
};
}

export function formatSkipReason(reason: string): string {
const match = SKIP_REASON_OPTIONS.find((option) => option.value === reason);
if (match && match.value) return match.label;
Expand Down
95 changes: 82 additions & 13 deletions apps/loopover-ui/src/components/site/audit-feed.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ vi.mock("@/lib/api/request", () => ({ apiFetch: (...args: unknown[]) => apiFetch
vi.mock("@/lib/api/origin", () => ({ getApiOrigin: () => "https://api.test" }));

import {
appendSkippedPrAuditPage,
buildSkippedPrAuditPath,
formatSkipReason,
normalizeSinceInput,
Expand All @@ -17,6 +18,8 @@ import { AuditFeed } from "@/components/site/audit-feed";
const SAMPLE: {
generatedAt: string;
limit: number;
offset: number;
total: number;
hasMore: boolean;
filters: { repoFullName: null; reason: null; since: null };
items: Array<{
Expand All @@ -29,6 +32,8 @@ const SAMPLE: {
} = {
generatedAt: "2026-05-28T00:00:05.000Z",
limit: 50,
offset: 0,
total: 1,
hasMore: false,
filters: { repoFullName: null, reason: null, since: null },
items: [
Expand All @@ -43,17 +48,23 @@ const SAMPLE: {
};

describe("audit feed helpers", () => {
it("builds query paths for skipped PR audit filters", () => {
expect(buildSkippedPrAuditPath({ limit: 25 })).toBe("/v1/app/skipped-pr-audit?limit=25");
it("builds query paths for skipped PR audit filters including offset (#7438)", () => {
expect(buildSkippedPrAuditPath({ limit: 25 })).toBe(
"/v1/app/skipped-pr-audit?limit=25&offset=0",
);
expect(
buildSkippedPrAuditPath({
limit: 50,
offset: 50,
repoFullName: "repo-owner/owned-repo",
reason: "bot_author",
since: "2026-05-28T00:00:00.000Z",
}),
).toBe(
"/v1/app/skipped-pr-audit?limit=50&repoFullName=repo-owner%2Fowned-repo&reason=bot_author&since=2026-05-28T00%3A00%3A00.000Z",
"/v1/app/skipped-pr-audit?limit=50&offset=50&repoFullName=repo-owner%2Fowned-repo&reason=bot_author&since=2026-05-28T00%3A00%3A00.000Z",
);
expect(buildSkippedPrAuditPath({ limit: 10, offset: -3 })).toBe(
"/v1/app/skipped-pr-audit?limit=10&offset=0",
);
});

Expand All @@ -78,6 +89,13 @@ describe("audit feed helpers", () => {
expect(normalizeSkippedPrAuditExport({ ...SAMPLE, items: [] })).toMatchObject({ items: [] });
expect(normalizeSkippedPrAuditExport(null)).toBeNull();
expect(normalizeSkippedPrAuditExport({ generatedAt: "2026-05-28T00:00:05.000Z" })).toBeNull();
expect(
normalizeSkippedPrAuditExport({
generatedAt: SAMPLE.generatedAt,
hasMore: true,
items: SAMPLE.items,
}),
).toMatchObject({ offset: 0, total: 1, limit: 1 });
expect(
normalizeSkippedPrAuditExport({
...SAMPLE,
Expand All @@ -95,6 +113,35 @@ describe("audit feed helpers", () => {
}),
).toMatchObject({ items: [{ repoFullName: "x/y", pullNumber: 1 }] });
});

it("REGRESSION (#7438): appends the next offset page instead of replacing already-visible rows", () => {
const page1 = {
...SAMPLE,
hasMore: true,
total: 2,
items: [SAMPLE.items[0]!],
};
const page2 = {
...SAMPLE,
offset: 50,
hasMore: false,
total: 2,
items: [
{
repoFullName: "repo-owner/owned-repo",
pullNumber: 5,
reason: "bot_author",
timestamp: "2026-05-28T00:00:03.000Z",
remediation: "Ignore bot authors in repository settings.",
},
],
};
expect(appendSkippedPrAuditPage(null, page1)).toEqual(page1);
expect(appendSkippedPrAuditPage(page1, { ...page1, offset: 0, items: [] }).items).toEqual([]);
expect(appendSkippedPrAuditPage(page1, page2).items.map((item) => item.pullNumber)).toEqual([
6, 5,
]);
});
});

describe("AuditFeed", () => {
Expand All @@ -111,7 +158,7 @@ describe("AuditFeed", () => {
"https://github.com/repo-owner/owned-repo/pull/6",
);
expect(apiFetch).toHaveBeenCalledWith(
"https://api.test/v1/app/skipped-pr-audit?limit=50",
"https://api.test/v1/app/skipped-pr-audit?limit=50&offset=0",
expect.objectContaining({ credentials: "include" }),
);
});
Expand All @@ -131,7 +178,7 @@ describe("AuditFeed", () => {
});

it("shows an empty state when the audit export has no items", async () => {
apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, items: [] } });
apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, items: [], total: 0 } });
render(<AuditFeed />);
expect(await screen.findByText("No skipped PR events")).toBeTruthy();
});
Expand Down Expand Up @@ -160,6 +207,7 @@ describe("AuditFeed", () => {
expect.any(Object),
),
);
expect(apiFetch.mock.calls.some(([url]) => String(url).includes("offset=0"))).toBe(true);
});

it("ignores invalid since values when applying filters", async () => {
Expand Down Expand Up @@ -194,24 +242,45 @@ describe("AuditFeed", () => {
expect(apiFetch).not.toHaveBeenCalled();
});

it("loads more rows until the maximum page size", async () => {
apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, hasMore: true } });
it("REGRESSION (#7438): Load more advances offset and appends rows instead of growing limit", async () => {
const firstPage = {
...SAMPLE,
hasMore: true,
total: 2,
items: [SAMPLE.items[0]!],
};
const secondPage = {
...SAMPLE,
offset: 50,
hasMore: false,
total: 2,
items: [
{
repoFullName: "repo-owner/owned-repo",
pullNumber: 5,
reason: "bot_author",
timestamp: "2026-05-28T00:00:03.000Z",
remediation: "Ignore bot authors in repository settings.",
},
],
};
apiFetch.mockResolvedValueOnce({ ok: true, data: firstPage });
render(<AuditFeed />);
await screen.findByText("repo-owner/owned-repo");
expect(await screen.findByText("#6")).toBeTruthy();
apiFetch.mockClear();
apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, hasMore: true, limit: 100 } });
apiFetch.mockResolvedValueOnce({ ok: true, data: secondPage });

fireEvent.click(screen.getByRole("button", { name: /load more/i }));

await waitFor(() =>
expect(apiFetch).toHaveBeenCalledWith(
"https://api.test/v1/app/skipped-pr-audit?limit=100",
"https://api.test/v1/app/skipped-pr-audit?limit=50&offset=50",
expect.any(Object),
),
);

expect(screen.getByText(/maximum page size \(100\)/i)).toBeTruthy();
expect(screen.queryByRole("button", { name: /load more/i })).toBeNull();
expect(await screen.findByText("#5")).toBeTruthy();
expect(screen.getByText("#6")).toBeTruthy();
expect(apiFetch.mock.calls.some(([url]) => String(url).includes("limit=100"))).toBe(false);
});

it("shows an error state when the audit response is malformed", async () => {
Expand Down
Loading
Loading