diff --git a/src/approval-report/__tests__/build-rule-approval-rows.test.ts b/src/approval-report/__tests__/build-rule-approval-rows.test.ts index 04faac7..3eab952 100644 --- a/src/approval-report/__tests__/build-rule-approval-rows.test.ts +++ b/src/approval-report/__tests__/build-rule-approval-rows.test.ts @@ -5,6 +5,7 @@ jest.mock("@octokit/rest", () => ({ import type { Parent } from "unist"; import { buildRuleApprovalRows, + classifyRuleStatus, type ApprovalReportDeps, } from "../build-rule-approval-rows"; import type { ApprovalReportOptions } from "../types"; @@ -71,7 +72,7 @@ function oneAtomic( } describe("buildRuleApprovalRows", () => { - it("skips deprecated rules", async () => { + it("includes deprecated rules with highest-priority status", async () => { const rows = await buildRuleApprovalRows( baseOpts, mockDeps({ @@ -83,8 +84,11 @@ describe("buildRuleApprovalRows", () => { pathRelativeToRepo: () => "_rules/keep.md", }), ); - expect(rows).toHaveLength(1); - expect(rows[0].ruleId).toBe("keep"); + expect(rows).toHaveLength(2); + expect(rows[0]).toMatchObject({ + ruleId: "gone", + status: "Deprecated", + }); }); it("strips issue body from row issues", async () => { @@ -108,6 +112,7 @@ describe("buildRuleApprovalRows", () => { number: 7, title: "rid bug", html_url: "https://github.com/o/r/issues/7", + labelNames: [], }, ]); }); @@ -118,6 +123,7 @@ describe("buildRuleApprovalRows", () => { oneAtomic("n", { loadCompleteImplementationsByRuleId: () => ({}) }), ); expect(rows[0].reportBucket).toBe("notReady"); + expect(rows[0].status).toBe("Awaiting implementation"); }); it("buckets notReady when a matched issue has Blocker label", async () => { @@ -136,13 +142,17 @@ describe("buildRuleApprovalRows", () => { }), ); expect(rows[0].reportBucket).toBe("notReady"); + expect(rows[0].status).toBe("Blocked by issue"); expect(rows[0].blockersCount).toBe(1); + expect(rows[0].blockers).toEqual(rows[0].issues); }); it("buckets proposedReadyForUpdate when not WAI-approved but has implementation", async () => { const rows = await buildRuleApprovalRows(baseOpts, oneAtomic("p")); expect(rows[0].reportBucket).toBe("proposedReadyForUpdate"); + expect(rows[0].status).toBe("Proposed, reviewable"); expect(rows[0].waiApproved).toBe(false); + expect(rows[0].reviewPrUrl).toBeNull(); }); it("buckets approvedUpToDate when approved with no commits after approval", async () => { @@ -155,6 +165,7 @@ describe("buildRuleApprovalRows", () => { }), ); expect(rows[0].reportBucket).toBe("approvedUpToDate"); + expect(rows[0].status).toBe("Approved, current"); expect(rows[0].commitsBehindSummary).toBe("0"); }); @@ -177,10 +188,51 @@ describe("buildRuleApprovalRows", () => { }), ); expect(rows[0].reportBucket).toBe("approvedReadyForUpdate"); + expect(rows[0].status).toBe("Approved, unpublished changes"); expect(rows[0].commitsBehindSummary).toBe("1"); expect(rows[0].changes).toEqual([change]); }); + it("splits rule commits from definition-only commits", async () => { + const changes = [ + { + hash: "a".repeat(40), + subject: "rule and definition", + dateIso: "2024-03-01T00:00:00Z", + touchedRule: true, + touchedDefinitionKeys: ["foo"], + }, + { + hash: "b".repeat(40), + subject: "definition only", + dateIso: "2024-02-01T00:00:00Z", + touchedRule: false, + touchedDefinitionKeys: ["foo"], + }, + { + hash: "c".repeat(40), + subject: "rule only", + dateIso: "2024-01-01T00:00:00Z", + touchedRule: true, + touchedDefinitionKeys: [], + }, + ]; + const rows = await buildRuleApprovalRows( + baseOpts, + oneAtomic("split", { + loadApprovalByRuleId: () => ({ + split: { approved: true, approvalIsoDate: "2023-01-01" }, + }), + getChangesSinceApproval: () => changes, + }), + ); + + expect(rows[0]).toMatchObject({ + ruleCommitCount: 2, + definitionCommitCount: 1, + }); + }); + it("does not call getChangesSinceApproval when rule is not WAI-approved", async () => { const getChangesSinceApproval = jest.fn(() => []); await buildRuleApprovalRows( @@ -215,3 +267,81 @@ describe("buildRuleApprovalRows", () => { expect(rows[0].compositeInputs).toEqual(["in1", "in2"]); }); }); + +describe("classifyRuleStatus", () => { + const reviewable = { + deprecated: false, + reviewPrUrl: null, + blockersCount: 0, + completeImplementationCount: 1, + waiApproved: false, + changesCount: 0, + }; + + it.each([ + [ + { + ...reviewable, + deprecated: true, + reviewPrUrl: "https://example.test/pr/1", + blockersCount: 1, + completeImplementationCount: 0, + }, + "Deprecated", + ], + [ + { + ...reviewable, + reviewPrUrl: "https://example.test/pr/1", + blockersCount: 1, + completeImplementationCount: 0, + }, + "In review", + ], + [ + { + ...reviewable, + blockersCount: 1, + completeImplementationCount: 0, + waiApproved: true, + }, + "Blocked by issue", + ], + [ + { + ...reviewable, + completeImplementationCount: 0, + waiApproved: true, + }, + "Awaiting implementation", + ], + [{ ...reviewable, waiApproved: true }, "Approved, current"], + [ + { ...reviewable, waiApproved: true, changesCount: 1 }, + "Approved, unpublished changes", + ], + [reviewable, "Proposed, reviewable"], + ])("applies status inputs in precedence order", (inputs, expected) => { + expect(classifyRuleStatus(inputs)).toBe(expected); + }); + + it("does not let a non-blocker issue affect status", async () => { + const rows = await buildRuleApprovalRows( + baseOpts, + oneAtomic("open-issue", { + fetchOpenIssues: async () => [ + { + number: 2, + title: "open-issue discussion", + html_url: "https://example.test/issues/2", + labelNames: ["enhancement"], + }, + ], + }), + ); + + expect(rows[0].status).toBe("Proposed, reviewable"); + expect(rows[0].issues).toHaveLength(1); + expect(rows[0].blockers).toHaveLength(0); + }); +}); diff --git a/src/approval-report/__tests__/generate-report.test.ts b/src/approval-report/__tests__/generate-report.test.ts index bc37bab..f69f201 100644 --- a/src/approval-report/__tests__/generate-report.test.ts +++ b/src/approval-report/__tests__/generate-report.test.ts @@ -12,10 +12,17 @@ function baseRow( name: ruleId, ruleTypeSummary: "atomic", waiApproved: false, + status: "Approved, current", + reviewPrUrl: null, reportBucket: "approvedUpToDate", implementations: ["axe"], issues: [], + blockers: [], changes: [], + approvalIsoDate: null, + lastUpdatedIsoDate: "2024-01-01", + ruleCommitCount: 0, + definitionCommitCount: 0, lastApprovedSummary: "2023-01-01", lastUpdatedSummary: "2024-01-01", commitsBehindSummary: "0", diff --git a/src/approval-report/__tests__/load-data.test.ts b/src/approval-report/__tests__/load-data.test.ts index a3d120a..7a575aa 100644 --- a/src/approval-report/__tests__/load-data.test.ts +++ b/src/approval-report/__tests__/load-data.test.ts @@ -125,6 +125,21 @@ describe("loadCompleteImplementationsByRuleId", () => { expect(loadCompleteImplementationsByRuleId(dir)["rx"]).toEqual(["custom"]); }); + it("uses the act-implementations name when JSON name is missing", () => { + writeTree(dir, { + "_data/wcag-act-rules/act-implementations.yml": ` +- uniqueKey: custom + name: Custom Accessibility Tool +`, + "_data/wcag-act-rules/implementations/custom.json": JSON.stringify({ + actRuleMapping: [{ ruleId: "rx", consistency: "complete" }], + }), + }); + expect(loadCompleteImplementationsByRuleId(dir)["rx"]).toEqual([ + "Custom Accessibility Tool", + ]); + }); + it("returns empty object when implementations dir has no json", () => { fs.mkdirSync(path.join(dir, "_data/wcag-act-rules/implementations"), { recursive: true, diff --git a/src/approval-report/__tests__/run.test.ts b/src/approval-report/__tests__/run.test.ts index a17dd0f..4bdd65c 100644 --- a/src/approval-report/__tests__/run.test.ts +++ b/src/approval-report/__tests__/run.test.ts @@ -48,11 +48,17 @@ describe("runApprovalReport", () => { name: "Only rule", ruleTypeSummary: "atomic", waiApproved: true, + status: "Approved, current", + reviewPrUrl: null, reportBucket: "approvedUpToDate", implementations: ["axe"], issues: [], + blockers: [], changes: [], approvalIsoDate: "2023-01-01", + lastUpdatedIsoDate: "2024-01-01", + ruleCommitCount: 0, + definitionCommitCount: 0, lastApprovedSummary: "2023-01-01", lastUpdatedSummary: "2024-01-01", commitsBehindSummary: "0", @@ -65,16 +71,33 @@ describe("runApprovalReport", () => { fs.rmSync(tmp, { recursive: true, force: true }); }); - it("writes markdown from rows and logs path", async () => { + it("writes JSON row array and logs path", async () => { const log = jest.spyOn(console, "log").mockImplementation(() => undefined); await runApprovalReport({ ...baseOpts, outFile }); - const written = fs.readFileSync(outFile, "utf8"); - expect(written).toContain("# ACT rules ready for approval"); - expect(written).toContain("[only](#only)"); + const written = JSON.parse(fs.readFileSync(outFile, "utf8")); + expect(written).toEqual([ + expect.objectContaining({ + ruleId: "only", + status: "Approved, current", + ruleCommitCount: 0, + definitionCommitCount: 0, + }), + ]); expect(log).toHaveBeenCalledWith( expect.stringContaining(path.resolve(outFile)), ); expect(log).toHaveBeenCalledWith(expect.stringContaining("(1 rules)")); log.mockRestore(); }); + + it("writes markdown only when explicitly requested", async () => { + const markdownFile = path.join(tmp, "debug", "report.md"); + const log = jest.spyOn(console, "log").mockImplementation(() => undefined); + await runApprovalReport({ ...baseOpts, outFile, markdownFile }); + + expect(fs.readFileSync(markdownFile, "utf8")).toContain( + "# ACT rules ready for approval", + ); + log.mockRestore(); + }); }); diff --git a/src/approval-report/build-rule-approval-rows.ts b/src/approval-report/build-rule-approval-rows.ts index 37c8fc8..361d74e 100644 --- a/src/approval-report/build-rule-approval-rows.ts +++ b/src/approval-report/build-rule-approval-rows.ts @@ -25,16 +25,56 @@ import { GitHubIssueRef, ReportBucket, RuleApprovalRow, + RuleStatus, } from "./types"; function stripIssueBody(issues: GitHubIssueRef[]): GitHubIssueRef[] { - return issues.map(({ number, title, html_url }) => ({ + return issues.map(({ number, title, html_url, labelNames }) => ({ number, title, html_url, + labelNames, })); } +export type RuleStatusInputs = { + deprecated: boolean; + reviewPrUrl: string | null; + blockersCount: number; + completeImplementationCount: number; + waiApproved: boolean; + changesCount: number; +}; + +/** Apply the status precedence defined by the act-board epic. */ +export function classifyRuleStatus(inputs: RuleStatusInputs): RuleStatus { + if (inputs.deprecated) return "Deprecated"; + if (inputs.reviewPrUrl) return "In review"; + if (inputs.blockersCount > 0) return "Blocked by issue"; + if (inputs.completeImplementationCount === 0) { + return "Awaiting implementation"; + } + if (inputs.waiApproved) { + return inputs.changesCount === 0 + ? "Approved, current" + : "Approved, unpublished changes"; + } + return "Proposed, reviewable"; +} + +function reportBucketForStatus(status: RuleStatus): ReportBucket { + switch (status) { + case "Approved, current": + return "approvedUpToDate"; + case "Approved, unpublished changes": + return "approvedReadyForUpdate"; + case "Proposed, reviewable": + return "proposedReadyForUpdate"; + default: + return "notReady"; + } +} + export type ApprovalReportDeps = { getRulePages: typeof getRulePages; getDefinitionPages: typeof getDefinitionPages; @@ -74,13 +114,13 @@ export async function buildRuleApprovalRows( const rows: RuleApprovalRow[] = []; for (const rule of rules) { const ruleId = rule.frontmatter.id; - if (rule.frontmatter.deprecated) continue; const approval = approvalById[ruleId] ?? { approved: false }; const implementations = implById[ruleId] ?? []; const matched = issuesForRuleId(ruleId, openIssues); - const blockersCount = matched.filter(issueHasBlockerLabel).length; const issues = stripIssueBody(matched); + const blockers = stripIssueBody(matched.filter(issueHasBlockerLabel)); + const blockersCount = blockers.length; const ruleRel = d.pathRelativeToRepo( opts.actRulesRepo, @@ -117,19 +157,23 @@ export async function buildRuleApprovalRows( const waiApproved = Boolean(approval.approved && approval.approvalIsoDate); const lastApprovedSummary = approval.approvalIsoDate ?? "-"; const commitsBehindSummary = waiApproved ? String(changes.length) : "-"; - - const hasCompleteImpl = implementations.length > 0; - const hasBlockers = blockersCount > 0; - - let reportBucket: ReportBucket; - if (!hasCompleteImpl || hasBlockers) { - reportBucket = "notReady"; - } else if (waiApproved) { - reportBucket = - changes.length > 0 ? "approvedReadyForUpdate" : "approvedUpToDate"; - } else { - reportBucket = "proposedReadyForUpdate"; - } + const reviewPrUrl = null; + const status = classifyRuleStatus({ + deprecated: Boolean(rule.frontmatter.deprecated), + reviewPrUrl, + blockersCount, + completeImplementationCount: implementations.length, + waiApproved, + changesCount: changes.length, + }); + const reportBucket = reportBucketForStatus(status); + const ruleCommitCount = changes.filter( + (change) => change.touchedRule, + ).length; + const definitionCommitCount = changes.filter( + (change) => + !change.touchedRule && change.touchedDefinitionKeys.length > 0, + ).length; rows.push({ ruleId, @@ -140,11 +184,17 @@ export async function buildRuleApprovalRows( ? [...rule.frontmatter.input_rules] : undefined, waiApproved, + status, + reviewPrUrl, reportBucket, implementations, issues, + blockers, changes, - approvalIsoDate: approval.approvalIsoDate, + approvalIsoDate: approval.approvalIsoDate ?? null, + lastUpdatedIsoDate: lastUpdatedRaw, + ruleCommitCount, + definitionCommitCount, lastApprovedSummary, lastUpdatedSummary, commitsBehindSummary, diff --git a/src/approval-report/load-data.ts b/src/approval-report/load-data.ts index 6ff59e6..1be8314 100644 --- a/src/approval-report/load-data.ts +++ b/src/approval-report/load-data.ts @@ -47,14 +47,11 @@ export function loadApprovalByRuleId( export function loadCompleteImplementationsByRuleId( wcagActRulesDir: string, ): Record { - const implDir = path.join( - wcagActRulesDir, - "_data", - "wcag-act-rules", - "implementations", - ); + const dataDir = path.join(wcagActRulesDir, "_data", "wcag-act-rules"); + const implDir = path.join(dataDir, "implementations"); const files = globby.sync(path.join(implDir, "*.json")); const byRule: Record> = {}; + const implementationNames = loadImplementationNames(dataDir); for (const file of files) { const json = JSON.parse(fs.readFileSync(file, "utf8")) as { @@ -64,7 +61,8 @@ export function loadCompleteImplementationsByRuleId( consistency?: string; }>; }; - const toolName = json.name ?? path.basename(file, ".json"); + const uniqueKey = path.basename(file, ".json"); + const toolName = json.name ?? implementationNames[uniqueKey] ?? uniqueKey; for (const row of json.actRuleMapping ?? []) { if (row.consistency !== "complete") continue; const ruleId = row.ruleId; @@ -79,3 +77,17 @@ export function loadCompleteImplementationsByRuleId( } return result; } + +function loadImplementationNames(dataDir: string): Record { + const implementationsPath = path.join(dataDir, "act-implementations.yml"); + if (!fs.existsSync(implementationsPath)) return {}; + + const entries = yaml.load(fs.readFileSync(implementationsPath, "utf8")) as + | Array<{ uniqueKey?: string; name?: string }> + | undefined; + const names: Record = {}; + for (const entry of entries ?? []) { + if (entry.uniqueKey && entry.name) names[entry.uniqueKey] = entry.name; + } + return names; +} diff --git a/src/approval-report/run.ts b/src/approval-report/run.ts index 5938d0e..e371cc1 100644 --- a/src/approval-report/run.ts +++ b/src/approval-report/run.ts @@ -9,11 +9,19 @@ export async function runApprovalReport( opts: ApprovalReportOptions, ): Promise { const rows = await buildRuleApprovalRows(opts); - const md = generateApprovalReportMarkdown(rows, { - owner: opts.githubOwner, - repo: opts.githubRepo, - }); fs.mkdirSync(path.dirname(path.resolve(opts.outFile)), { recursive: true }); - fs.writeFileSync(opts.outFile, md, "utf8"); + fs.writeFileSync(opts.outFile, `${JSON.stringify(rows, null, 2)}\n`, "utf8"); console.log(`Wrote ${path.resolve(opts.outFile)} (${rows.length} rules)`); + + if (opts.markdownFile) { + const md = generateApprovalReportMarkdown(rows, { + owner: opts.githubOwner, + repo: opts.githubRepo, + }); + fs.mkdirSync(path.dirname(path.resolve(opts.markdownFile)), { + recursive: true, + }); + fs.writeFileSync(opts.markdownFile, md, "utf8"); + console.log(`Wrote debug report ${path.resolve(opts.markdownFile)}`); + } } diff --git a/src/approval-report/types.ts b/src/approval-report/types.ts index baa13dd..731fb54 100644 --- a/src/approval-report/types.ts +++ b/src/approval-report/types.ts @@ -32,6 +32,16 @@ export type ReportBucket = | "approvedUpToDate" | "notReady"; +/** Values intentionally match the act-board Project Status options. */ +export type RuleStatus = + | "Deprecated" + | "In review" + | "Blocked by issue" + | "Awaiting implementation" + | "Approved, current" + | "Approved, unpublished changes" + | "Proposed, reviewable"; + export type RuleApprovalRow = { ruleId: string; name: string; @@ -40,11 +50,20 @@ export type RuleApprovalRow = { compositeInputs?: string[]; /** Rule has an approved WAI snapshot (`index.md` in rule-versions) */ waiApproved: boolean; + status: RuleStatus; + /** Populated by the PR lookup added in issue #66. */ + reviewPrUrl: string | null; reportBucket: ReportBucket; implementations: string[]; issues: GitHubIssueRef[]; + blockers: GitHubIssueRef[]; changes: ChangeEntry[]; - approvalIsoDate?: string; + approvalIsoDate: string | null; + lastUpdatedIsoDate: string | null; + /** Post-approval commits that touched the rule file (including rule + glossary commits). */ + ruleCommitCount: number; + /** Post-approval commits that touched glossary paths, but not the rule file. */ + definitionCommitCount: number; /** Summary table: YYYY-MM-DD when rule has an approved snapshot, else "-" */ lastApprovedSummary: string; /** Summary table: YYYY-MM-DD of latest commit touching rule + transitive glossary, else "-" */ @@ -62,6 +81,8 @@ export type ApprovalReportOptions = { actRulesRepo: string; wcagActRulesDir: string; outFile: string; + /** Optional legacy four-bucket report for debugging. */ + markdownFile?: string; githubOwner: string; githubRepo: string; }; diff --git a/src/cli/approval-report.ts b/src/cli/approval-report.ts index 61f13f5..f3f575d 100644 --- a/src/cli/approval-report.ts +++ b/src/cli/approval-report.ts @@ -10,9 +10,7 @@ const defaultSibling = (segment: string): string => const program = new Command(); program - .description( - "List ACT rules ready for WAI approval: updated approved rules, or proposed rules with a complete implementation", - ) + .description("Write the act-board classifier JSON snapshot for all ACT rules") .option( "-r, --rulesDir ", "Path to act-rules.github.io _rules directory", @@ -40,9 +38,10 @@ program ) .option( "-o, --outFile ", - "Output markdown file", - path.resolve(process.cwd(), "approval-report.md"), + "Output JSON file", + path.resolve(process.cwd(), "approval-report.json"), ) + .option("--markdownFile ", "Optional four-bucket markdown debug report") .option( "--githubOwner ", "GitHub owner for issues lookup and commit links in report details", @@ -64,6 +63,7 @@ const opts: ApprovalReportOptions = { actRulesRepo: path.resolve(o.actRulesRepo), wcagActRulesDir: path.resolve(o.wcagActRulesDir), outFile: path.resolve(o.outFile), + markdownFile: o.markdownFile ? path.resolve(o.markdownFile) : undefined, githubOwner: o.githubOwner, githubRepo: o.githubRepo, };