Skip to content

Commit f901a00

Browse files
committed
refactor(api): dedups PR enrichment field copy into a shared helper
Extracts pickEnrichmentFields so mergeEnrichment, fallbackToPreviousEnrichment, and the DashboardPage fine-grained merge share one field list, removing the drift risk that a future enrichment field is added to some copy sites but not others. Adds unit tests covering fallbackToPreviousEnrichment (empty-previous, already-enriched passthrough, missing/unenriched prev, carry-forward, closed-PR drop) and pickEnrichmentFields field selection.
1 parent 99996f9 commit f901a00

3 files changed

Lines changed: 170 additions & 31 deletions

File tree

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import DependenciesTab from "./DependenciesTab";
1414
import { isDependencyPr, expandBotLogins, needsBodyFallback, parseRenovateBody, type VersionInfo } from "../../lib/dependency-detection";
1515
import { isRepoExcludedFromDependencies } from "../../lib/dependency-exclusion";
1616
import { findDashboardIssues, parseAbandonedSection, resetAbandonedPatternCache, type AbandonedDependency } from "../../lib/dependency-dashboard";
17-
import { fetchDashboardIssueBodies, fetchDepPRBodies, fallbackToPreviousEnrichment } from "../../services/api";
17+
import { fetchDashboardIssueBodies, fetchDepPRBodies, fallbackToPreviousEnrichment, pickEnrichmentFields } from "../../services/api";
1818
import type { SortOption } from "../shared/SortDropdown";
1919
import type { Issue, PullRequest, WorkflowRun } from "../../services/api";
2020
import { fetchOrgs } from "../../services/api";
@@ -290,16 +290,7 @@ async function pollFetch(): Promise<DashboardData> {
290290
// enriched from a prior cycle.
291291
const regressing = e.enriched === false && pr.enriched !== false;
292292
if (!regressing) {
293-
pr.headSha = e.headSha;
294-
pr.assigneeLogins = e.assigneeLogins;
295-
pr.reviewerLogins = e.reviewerLogins;
296-
pr.checkStatus = e.checkStatus;
297-
pr.additions = e.additions;
298-
pr.deletions = e.deletions;
299-
pr.changedFiles = e.changedFiles;
300-
pr.comments = e.comments;
301-
pr.reviewThreads = e.reviewThreads;
302-
pr.totalReviewCount = e.totalReviewCount;
293+
Object.assign(pr, pickEnrichmentFields(e));
303294
pr.enriched = e.enriched;
304295
}
305296
pr.nodeId = e.nodeId;

src/app/services/api.ts

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,42 @@ export async function fetchDepPRBodies(
13291329
);
13301330
}
13311331

1332+
/**
1333+
* The heavy PR fields populated by phase-2 enrichment. Centralized so the sites
1334+
* that copy enrichment onto a PR — mergeEnrichment, fallbackToPreviousEnrichment,
1335+
* and the fine-grained store merge in DashboardPage — stay in sync when a field
1336+
* is added or removed.
1337+
*/
1338+
type EnrichmentFields = Pick<
1339+
PullRequest,
1340+
| "headSha"
1341+
| "assigneeLogins"
1342+
| "reviewerLogins"
1343+
| "checkStatus"
1344+
| "additions"
1345+
| "deletions"
1346+
| "changedFiles"
1347+
| "comments"
1348+
| "reviewThreads"
1349+
| "totalReviewCount"
1350+
>;
1351+
1352+
/** Extracts just the heavy enrichment fields from any PR-shaped source. */
1353+
export function pickEnrichmentFields(source: EnrichmentFields): EnrichmentFields {
1354+
return {
1355+
headSha: source.headSha,
1356+
assigneeLogins: source.assigneeLogins,
1357+
reviewerLogins: source.reviewerLogins,
1358+
checkStatus: source.checkStatus,
1359+
additions: source.additions,
1360+
deletions: source.deletions,
1361+
changedFiles: source.changedFiles,
1362+
comments: source.comments,
1363+
reviewThreads: source.reviewThreads,
1364+
totalReviewCount: source.totalReviewCount,
1365+
};
1366+
}
1367+
13321368
/**
13331369
* Merges phase 2 enrichment data into light PRs. Returns enriched PR array.
13341370
* Also detects fork PRs for the statusCheckRollup fallback.
@@ -1348,16 +1384,7 @@ function mergeEnrichment(
13481384

13491385
return {
13501386
...pr,
1351-
headSha: e.headSha,
1352-
assigneeLogins: e.assigneeLogins,
1353-
reviewerLogins: e.reviewerLogins,
1354-
checkStatus: e.checkStatus,
1355-
additions: e.additions,
1356-
deletions: e.deletions,
1357-
changedFiles: e.changedFiles,
1358-
comments: e.comments,
1359-
reviewThreads: e.reviewThreads,
1360-
totalReviewCount: e.totalReviewCount,
1387+
...pickEnrichmentFields(e),
13611388
enriched: true,
13621389
};
13631390
});
@@ -1382,16 +1409,7 @@ export function fallbackToPreviousEnrichment(
13821409
if (!prev || prev.enriched === false) return pr;
13831410
return {
13841411
...pr,
1385-
headSha: prev.headSha,
1386-
assigneeLogins: prev.assigneeLogins,
1387-
reviewerLogins: prev.reviewerLogins,
1388-
checkStatus: prev.checkStatus,
1389-
additions: prev.additions,
1390-
deletions: prev.deletions,
1391-
changedFiles: prev.changedFiles,
1392-
comments: prev.comments,
1393-
reviewThreads: prev.reviewThreads,
1394-
totalReviewCount: prev.totalReviewCount,
1412+
...pickEnrichmentFields(prev),
13951413
enriched: true,
13961414
};
13971415
});

tests/services/api-optimization.test.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import {
44
fetchIssuesAndPullRequests,
55
fetchWorkflowRuns,
66
fetchPREnrichment,
7+
fallbackToPreviousEnrichment,
8+
pickEnrichmentFields,
79
type RepoRef,
810
} from "../../src/app/services/api";
911
import { clearCache } from "../../src/app/stores/cache";
12+
import { makePullRequest } from "../helpers/factories";
1013

1114
vi.mock("../../src/app/lib/errors", () => ({
1215
pushNotification: vi.fn(),
@@ -597,3 +600,130 @@ describe("fetchPREnrichment mergeStateStatus UNSTABLE override", () => {
597600
expect(enrichments.get(100)!.checkStatus).toBe("failure");
598601
});
599602
});
603+
604+
// ── Enrichment carry-forward on backfill failure ──────────────────────────────
605+
606+
describe("fallbackToPreviousEnrichment", () => {
607+
it("returns next unchanged when previous is empty", () => {
608+
const next = [makePullRequest({ id: 1, enriched: false })];
609+
const result = fallbackToPreviousEnrichment([], next);
610+
expect(result).toBe(next);
611+
});
612+
613+
it("passes through a PR that is already enriched in next (no carry-forward)", () => {
614+
const previous = [makePullRequest({ id: 1, enriched: true, checkStatus: "failure" })];
615+
const next = [makePullRequest({ id: 1, enriched: true, checkStatus: "success" })];
616+
const result = fallbackToPreviousEnrichment(previous, next);
617+
// Fresh enriched data wins — prior "failure" must not overwrite fresh "success".
618+
expect(result[0].checkStatus).toBe("success");
619+
expect(result[0]).toBe(next[0]);
620+
});
621+
622+
it("passes through an unenriched next PR with no matching previous entry", () => {
623+
const previous = [makePullRequest({ id: 99, enriched: true })];
624+
const next = [makePullRequest({ id: 1, enriched: false })];
625+
const result = fallbackToPreviousEnrichment(previous, next);
626+
expect(result[0]).toBe(next[0]);
627+
expect(result[0].enriched).toBe(false);
628+
});
629+
630+
it("passes through an unenriched next PR whose previous entry was also unenriched", () => {
631+
const previous = [makePullRequest({ id: 1, enriched: false })];
632+
const next = [makePullRequest({ id: 1, enriched: false })];
633+
const result = fallbackToPreviousEnrichment(previous, next);
634+
expect(result[0]).toBe(next[0]);
635+
expect(result[0].enriched).toBe(false);
636+
});
637+
638+
it("carries forward prior enrichment for an unenriched next PR that was enriched before", () => {
639+
const previous = [
640+
makePullRequest({
641+
id: 1,
642+
enriched: true,
643+
checkStatus: "success",
644+
additions: 42,
645+
deletions: 7,
646+
changedFiles: 3,
647+
comments: 5,
648+
reviewThreads: 2,
649+
totalReviewCount: 4,
650+
reviewerLogins: ["reviewer1"],
651+
assigneeLogins: ["assignee1"],
652+
headSha: "prevsha",
653+
}),
654+
];
655+
const next = [
656+
makePullRequest({
657+
id: 1,
658+
enriched: false,
659+
state: "OPEN",
660+
title: "Fresh title",
661+
checkStatus: null,
662+
additions: 0,
663+
deletions: 0,
664+
changedFiles: 0,
665+
comments: 0,
666+
reviewThreads: 0,
667+
totalReviewCount: 0,
668+
reviewerLogins: [],
669+
assigneeLogins: [],
670+
headSha: "",
671+
}),
672+
];
673+
const result = fallbackToPreviousEnrichment(previous, next);
674+
// Heavy fields restored from the prior cycle...
675+
expect(result[0].enriched).toBe(true);
676+
expect(result[0].checkStatus).toBe("success");
677+
expect(result[0].additions).toBe(42);
678+
expect(result[0].deletions).toBe(7);
679+
expect(result[0].changedFiles).toBe(3);
680+
expect(result[0].comments).toBe(5);
681+
expect(result[0].reviewThreads).toBe(2);
682+
expect(result[0].totalReviewCount).toBe(4);
683+
expect(result[0].reviewerLogins).toEqual(["reviewer1"]);
684+
expect(result[0].assigneeLogins).toEqual(["assignee1"]);
685+
expect(result[0].headSha).toBe("prevsha");
686+
// ...but fresh light fields survive.
687+
expect(result[0].title).toBe("Fresh title");
688+
});
689+
690+
it("drops a PR that closed (present in previous, absent from next)", () => {
691+
const previous = [
692+
makePullRequest({ id: 1, enriched: true }),
693+
makePullRequest({ id: 2, enriched: true }),
694+
];
695+
const next = [makePullRequest({ id: 1, enriched: false })];
696+
const result = fallbackToPreviousEnrichment(previous, next);
697+
// Only the PR still in `next` remains — the closed PR (id 2) is not resurrected.
698+
expect(result.map((pr) => pr.id)).toEqual([1]);
699+
});
700+
});
701+
702+
describe("pickEnrichmentFields", () => {
703+
it("returns only the heavy enrichment fields, not light fields", () => {
704+
const pr = makePullRequest({
705+
id: 1,
706+
title: "light title",
707+
state: "OPEN",
708+
checkStatus: "success",
709+
additions: 10,
710+
reviewerLogins: ["r1"],
711+
});
712+
const fields = pickEnrichmentFields(pr);
713+
expect(fields).toEqual({
714+
headSha: pr.headSha,
715+
assigneeLogins: pr.assigneeLogins,
716+
reviewerLogins: pr.reviewerLogins,
717+
checkStatus: pr.checkStatus,
718+
additions: pr.additions,
719+
deletions: pr.deletions,
720+
changedFiles: pr.changedFiles,
721+
comments: pr.comments,
722+
reviewThreads: pr.reviewThreads,
723+
totalReviewCount: pr.totalReviewCount,
724+
});
725+
expect(fields).not.toHaveProperty("title");
726+
expect(fields).not.toHaveProperty("state");
727+
expect(fields).not.toHaveProperty("enriched");
728+
});
729+
});

0 commit comments

Comments
 (0)