30d6f15
@@ -0,0 +1,166 @@
/**
 * Review threads: resolving and unresolving round-trip through
 * `threadStates`, a comment on a line whose content changed reads as
 * outdated, the latest verdict per author wins, and comments on one
 * (path, side, line) group into one thread.
 */
import { beforeEach, describe, expect, test } from "bun:test";
import type { Database } from "bun:sqlite";
import { migrations } from "../src/core/schema.ts";
import { openDb } from "../src/kernel/db.ts";
import type { Diff } from "../src/git/diff.ts";
import {
  addReviewComment,
  anchorId,
  byAnchor,
  hashLine,
  latestReviewByAuthor,
  listReviewComments,
  resolveThread,
  submitReview,
  threadStates,
  unresolveThread,
  withOutdated,
  type Side,
} from "../src/core/reviews.ts";
let db: Database;
const ins = (sql: string) => db.exec(sql);
beforeEach(() => {
  db = openDb(":memory:", migrations);
  ins("INSERT INTO user (id, username, email, password_hash) VALUES ('ann','ann','a@x','x'),('bob','bob','b@x','x')");
  ins("INSERT INTO repo (id, owner_id, name, visibility) VALUES ('r1','ann','r','private')");
  ins("INSERT INTO issue (id, repo_id, number, kind, title, body, author_id) VALUES ('p1','r1',1,'pull','t','','ann')");
});
const comment = (path: string, line: number, content: string, side: Side = "new", authorId = "bob") => {
  const made = addReviewComment(db, {
    pullId: "p1",
    authorId,
    body: "note",
    path,
    line,
    side,
    commitOid: "c1",
    lineHash: hashLine(content),
  });
  if (!made.ok) throw new Error(made.error);
  return made.value;
};
const diffOf = (path: string, rows: [number, string][]): Diff => ({
  additions: rows.length,
  deletions: 0,
  files: [
    {
      path,
      hunks: [
        {
          lines: rows.map(([n, content]) => ({
            kind: "add",
            oldNumber: null,
            newNumber: n,
            content,
            noNewline: false,
          })),
        },
      ],
    },
  ] as unknown as Diff["files"],
      
        sentinelcrew
        18d ago
        
      
      Nit: as unknown as Diff["files"] turns off checking for the whole fixture. Typing the rows as the diff line type (kind: "add" as const) would keep tsc checking that the fixture matches the real shape.
    
});
describe("resolving threads", () => {
  const anchor = anchorId("a.ts", "new", 3);
  test("resolve shows resolved, unresolve shows open, resolving twice changes nothing", () => {
    expect(threadStates(db, "p1").size).toBe(0);
    resolveThread(db, "p1", anchor, "ann");
    const resolved = threadStates(db, "p1").get(anchor)!;
    expect(resolved.resolvedBy).toBe("ann");
    expect(resolved.resolvedAt).not.toBeNull();
    resolveThread(db, "p1", anchor, "ann");
    expect(threadStates(db, "p1").size).toBe(1);
    expect(threadStates(db, "p1").get(anchor)!.resolvedBy).toBe("ann");
    const open = unresolveThread(db, "p1", anchor);
    expect(open).toEqual({ anchor, resolvedAt: null, resolvedBy: null });
    expect(threadStates(db, "p1").get(anchor)).toEqual(open);
    resolveThread(db, "p1", anchor, "bob");
    expect(threadStates(db, "p1").get(anchor)!.resolvedBy).toBe("bob");
  });
  test("resolving one thread leaves another open and another pull untouched", () => {
    ins("INSERT INTO issue (id, repo_id, number, kind, title, body, author_id) VALUES ('p2','r1',2,'pull','t','','ann')");
    const other = anchorId("a.ts", "new", 4);
    resolveThread(db, "p1", anchor, "ann");
    expect(threadStates(db, "p1").has(other)).toBe(false);
    expect(threadStates(db, "p2").size).toBe(0);
  });
});
describe("outdated comments", () => {
  test("a comment on a changed line is outdated; on an unchanged line it is not", () => {
    comment("a.ts", 1, "const a = 1;");
    comment("a.ts", 2, "const b = 2;");
    const diff = diffOf("a.ts", [
      [1, "const a = 1;"],
      [2, "const b = 3;"],
    ]);
    const out = withOutdated(listReviewComments(db, "p1"), diff);
    expect(out.map((c) => [c.line, c.outdated])).toEqual([
      [1, false],
      [2, true],
    ]);
  });
  test("a line or file gone from the diff reads as outdated", () => {
    comment("a.ts", 9, "x");
    comment("gone.ts", 1, "x");
    const out = withOutdated(listReviewComments(db, "p1"), diffOf("a.ts", [[1, "x"]]));
    expect(out.map((c) => c.outdated)).toEqual([true, true]);
  });
});
describe("latestReviewByAuthor", () => {
  test("returns each author's newest review only", () => {
    const send = (authorId: string, state: "approved" | "changes_requested" | "commented") =>
      submitReview(db, { issueId: "p1", authorId, state, commitOid: "c1" });
    send("bob", "changes_requested");
    send("ann", "commented");
    const last = send("bob", "approved");
    const latest = latestReviewByAuthor(db, "p1");
    expect(latest.size).toBe(2);
    expect(latest.get("bob")!.id).toBe(last.ok ? last.value.id : "");
    expect(latest.get("bob")!.state).toBe("approved");
    expect(latest.get("ann")!.state).toBe("commented");
  });
  test("imported reviews are keyed by origin login, not the connecting user", () => {
    for (const [login, state] of [["x", "approved"], ["y", "changes_requested"]] as const) {
      submitReview(db, {
        issueId: "p1", authorId: "ann", state, commitOid: "c1",
        originRemoteId: login, originAuthor: login,
      });
    }
    expect([...latestReviewByAuthor(db, "p1").keys()].sort()).toEqual(["x", "y"]);
  });
});
describe("byAnchor", () => {
  test("groups comments on the same file, side and line", () => {
    const a = comment("a.ts", 1, "l");
    const b = comment("a.ts", 1, "l", "new", "ann");
    comment("a.ts", 2, "l");
    comment("b.ts", 1, "l");
    comment("a.ts", 1, "l", "old");
    const groups = byAnchor(listReviewComments(db, "p1"));
    expect(groups.size).toBe(4);
    const same = groups.get("a.ts\0new\0" + 1)!;
      
        sentinelcrew
        18d ago
        
      
      Nit: use anchorKey("a.ts", "new", 1) (it is already exported next to anchorId). Then this test follows the key format if it ever changes, instead of a hand-copied \0 literal.
    
    expect(same.map((c) => c.id)).toEqual([a.id, b.id]);
  });
});
		@@ -0,0 +1,166 @@
	1	/**
	2	* Review threads: resolving and unresolving round-trip through
	3	* `threadStates`, a comment on a line whose content changed reads as
	4	* outdated, the latest verdict per author wins, and comments on one
	5	* (path, side, line) group into one thread.
	6	*/
	7	import { beforeEach, describe, expect, test } from "bun:test";
	8	import type { Database } from "bun:sqlite";
	9	import { migrations } from "../src/core/schema.ts";
	10	import { openDb } from "../src/kernel/db.ts";
	11	import type { Diff } from "../src/git/diff.ts";
	12	import {
	13	addReviewComment,
	14	anchorId,
	15	byAnchor,
	16	hashLine,
	17	latestReviewByAuthor,
	18	listReviewComments,
	19	resolveThread,
	20	submitReview,
	21	threadStates,
	22	unresolveThread,
	23	withOutdated,
	24	type Side,
	25	} from "../src/core/reviews.ts";
	26
	27	let db: Database;
	28
	29	const ins = (sql: string) => db.exec(sql);
	30
	31	beforeEach(() => {
	32	db = openDb(":memory:", migrations);
	33	ins("INSERT INTO user (id, username, email, password_hash) VALUES ('ann','ann','a@x','x'),('bob','bob','b@x','x')");
	34	ins("INSERT INTO repo (id, owner_id, name, visibility) VALUES ('r1','ann','r','private')");
	35	ins("INSERT INTO issue (id, repo_id, number, kind, title, body, author_id) VALUES ('p1','r1',1,'pull','t','','ann')");
	36	});
	37
	38	const comment = (path: string, line: number, content: string, side: Side = "new", authorId = "bob") => {
	39	const made = addReviewComment(db, {
	40	pullId: "p1",
	41	authorId,
	42	body: "note",
	43	path,
	44	line,
	45	side,
	46	commitOid: "c1",
	47	lineHash: hashLine(content),
	48	});
	49	if (!made.ok) throw new Error(made.error);
	50	return made.value;
	51	};
	52
	53	const diffOf = (path: string, rows: [number, string][]): Diff => ({
	54	additions: rows.length,
	55	deletions: 0,
	56	files: [
	57	{
	58	path,
	59	hunks: [
	60	{
	61	lines: rows.map(([n, content]) => ({
	62	kind: "add",
	63	oldNumber: null,
	64	newNumber: n,
	65	content,
	66	noNewline: false,
	67	})),
	68	},
	69	],
	70	},
	71	] as unknown as Diff["files"],
		sentinelcrew 18d ago Nit: `as unknown as Diff["files"]` turns off checking for the whole fixture. Typing the rows as the diff line type (`kind: "add" as const`) would keep tsc checking that the fixture matches the real shape.
	72	});
	73
	74	describe("resolving threads", () => {
	75	const anchor = anchorId("a.ts", "new", 3);
	76
	77	test("resolve shows resolved, unresolve shows open, resolving twice changes nothing", () => {
	78	expect(threadStates(db, "p1").size).toBe(0);
	79
	80	resolveThread(db, "p1", anchor, "ann");
	81	const resolved = threadStates(db, "p1").get(anchor)!;
	82	expect(resolved.resolvedBy).toBe("ann");
	83	expect(resolved.resolvedAt).not.toBeNull();
	84
	85	resolveThread(db, "p1", anchor, "ann");
	86	expect(threadStates(db, "p1").size).toBe(1);
	87	expect(threadStates(db, "p1").get(anchor)!.resolvedBy).toBe("ann");
	88
	89	const open = unresolveThread(db, "p1", anchor);
	90	expect(open).toEqual({ anchor, resolvedAt: null, resolvedBy: null });
	91	expect(threadStates(db, "p1").get(anchor)).toEqual(open);
	92
	93	resolveThread(db, "p1", anchor, "bob");
	94	expect(threadStates(db, "p1").get(anchor)!.resolvedBy).toBe("bob");
	95	});
	96
	97	test("resolving one thread leaves another open and another pull untouched", () => {
	98	ins("INSERT INTO issue (id, repo_id, number, kind, title, body, author_id) VALUES ('p2','r1',2,'pull','t','','ann')");
	99	const other = anchorId("a.ts", "new", 4);
	100	resolveThread(db, "p1", anchor, "ann");
	101	expect(threadStates(db, "p1").has(other)).toBe(false);
	102	expect(threadStates(db, "p2").size).toBe(0);
	103	});
	104	});
	105
	106	describe("outdated comments", () => {
	107	test("a comment on a changed line is outdated; on an unchanged line it is not", () => {
	108	comment("a.ts", 1, "const a = 1;");
	109	comment("a.ts", 2, "const b = 2;");
	110	const diff = diffOf("a.ts", [
	111	[1, "const a = 1;"],
	112	[2, "const b = 3;"],
	113	]);
	114	const out = withOutdated(listReviewComments(db, "p1"), diff);
	115	expect(out.map((c) => [c.line, c.outdated])).toEqual([
	116	[1, false],
	117	[2, true],
	118	]);
	119	});
	120
	121	test("a line or file gone from the diff reads as outdated", () => {
	122	comment("a.ts", 9, "x");
	123	comment("gone.ts", 1, "x");
	124	const out = withOutdated(listReviewComments(db, "p1"), diffOf("a.ts", [[1, "x"]]));
	125	expect(out.map((c) => c.outdated)).toEqual([true, true]);
	126	});
	127	});
	128
	129	describe("latestReviewByAuthor", () => {
	130	test("returns each author's newest review only", () => {
	131	const send = (authorId: string, state: "approved" \| "changes_requested" \| "commented") =>
	132	submitReview(db, { issueId: "p1", authorId, state, commitOid: "c1" });
	133	send("bob", "changes_requested");
	134	send("ann", "commented");
	135	const last = send("bob", "approved");
	136	const latest = latestReviewByAuthor(db, "p1");
	137	expect(latest.size).toBe(2);
	138	expect(latest.get("bob")!.id).toBe(last.ok ? last.value.id : "");
	139	expect(latest.get("bob")!.state).toBe("approved");
	140	expect(latest.get("ann")!.state).toBe("commented");
	141	});
	142
	143	test("imported reviews are keyed by origin login, not the connecting user", () => {
	144	for (const [login, state] of [["x", "approved"], ["y", "changes_requested"]] as const) {
	145	submitReview(db, {
	146	issueId: "p1", authorId: "ann", state, commitOid: "c1",
	147	originRemoteId: login, originAuthor: login,
	148	});
	149	}
	150	expect([...latestReviewByAuthor(db, "p1").keys()].sort()).toEqual(["x", "y"]);
	151	});
	152	});
	153
	154	describe("byAnchor", () => {
	155	test("groups comments on the same file, side and line", () => {
	156	const a = comment("a.ts", 1, "l");
	157	const b = comment("a.ts", 1, "l", "new", "ann");
	158	comment("a.ts", 2, "l");
	159	comment("b.ts", 1, "l");
	160	comment("a.ts", 1, "l", "old");
	161	const groups = byAnchor(listReviewComments(db, "p1"));
	162	expect(groups.size).toBe(4);
	163	const same = groups.get("a.ts\0new\0" + 1)!;
		sentinelcrew 18d ago Nit: use `anchorKey("a.ts", "new", 1)` (it is already exported next to `anchorId`). Then this test follows the key format if it ever changes, instead of a hand-copied `\0` literal.
	164	expect(same.map((c) => c.id)).toEqual([a.id, b.id]);
	165	});
	166	});