mirror of
https://github.com/block/buzz.git
synced 2026-11-13 12:51:34 +00:00
feat(desktop): add commit-scoped PR review decisions (#2161)
This commit is contained in:
@@ -14,8 +14,14 @@ export type ProjectPullRequestComment = {
|
||||
content: string;
|
||||
author: string;
|
||||
createdAt: number;
|
||||
commit: string | null;
|
||||
isApproval: boolean;
|
||||
isChangeRequest: boolean;
|
||||
isReviewRequest: boolean;
|
||||
isTrustedReviewDecision: boolean;
|
||||
isTrustedReviewRequest: boolean;
|
||||
reviewDecision: "approved" | "changes-requested" | null;
|
||||
reviewDecisionStatus: "current" | "historical" | null;
|
||||
reviewerPubkeys: string[];
|
||||
};
|
||||
|
||||
@@ -23,10 +29,21 @@ export type ProjectPullRequestApproval = {
|
||||
id: string;
|
||||
author: string;
|
||||
createdAt: number;
|
||||
commit: string;
|
||||
reviewDecision: "approved";
|
||||
};
|
||||
|
||||
export type ProjectPullRequestChangeRequest = {
|
||||
id: string;
|
||||
author: string;
|
||||
createdAt: number;
|
||||
commit: string;
|
||||
reviewDecision: "changes-requested";
|
||||
};
|
||||
|
||||
export const PR_REVIEW_REQUEST_LABEL: string;
|
||||
export const PR_APPROVAL_LABEL: string;
|
||||
export const PR_CHANGES_REQUESTED_LABEL: string;
|
||||
|
||||
export type ProjectPullRequest = {
|
||||
id: string;
|
||||
@@ -39,8 +56,10 @@ export type ProjectPullRequest = {
|
||||
recipients: string[];
|
||||
/** Requested reviewers (root `p` tags + trusted review-request comments). */
|
||||
reviewers: string[];
|
||||
/** Latest approval per reviewer, oldest first. */
|
||||
/** Latest current-commit approval per reviewer, oldest first. */
|
||||
approvals: ProjectPullRequestApproval[];
|
||||
/** Latest current-commit change request per reviewer, oldest first. */
|
||||
changeRequests: ProjectPullRequestChangeRequest[];
|
||||
status: "Open" | "Merged" | "Closed" | "Draft";
|
||||
statusEventId: string | null;
|
||||
statusCreatedAt: number | null;
|
||||
@@ -65,6 +84,35 @@ export function nextProjectPullRequestStatusCreatedAt(
|
||||
pullRequest: Pick<ProjectPullRequest, "statusCreatedAt">,
|
||||
now: number,
|
||||
): number;
|
||||
export function nextProjectPullRequestReviewCreatedAt(
|
||||
pullRequest: Pick<ProjectPullRequest, "approvals" | "changeRequests">,
|
||||
now: number,
|
||||
): number;
|
||||
export function projectPullRequestCommentTimelineKind(
|
||||
comment: Pick<
|
||||
ProjectPullRequestComment,
|
||||
| "isTrustedReviewDecision"
|
||||
| "isTrustedReviewRequest"
|
||||
| "reviewDecision"
|
||||
| "reviewDecisionStatus"
|
||||
>,
|
||||
): "approved" | "changes-requested" | "review-request" | null;
|
||||
export function projectPullRequestReviewSummary(
|
||||
pullRequest: Pick<
|
||||
ProjectPullRequest,
|
||||
"approvals" | "changeRequests" | "reviewers" | "status"
|
||||
>,
|
||||
): {
|
||||
approvalCount: number;
|
||||
changeRequestCount: number;
|
||||
detail: string | null;
|
||||
showState: boolean;
|
||||
state: string;
|
||||
};
|
||||
export function projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest: Pick<ProjectPullRequest, "approvals" | "changeRequests">,
|
||||
comment: Pick<ProjectPullRequestComment, "id">,
|
||||
): "approved" | "changes-requested" | null;
|
||||
export function projectPullRequestEventsToPullRequests(
|
||||
pullRequestEvents: RelayEvent[],
|
||||
updateEvents?: RelayEvent[],
|
||||
|
||||
@@ -67,6 +67,70 @@ export function nextProjectPullRequestStatusCreatedAt(pullRequest, now) {
|
||||
return Math.max(now, (pullRequest.statusCreatedAt ?? 0) + 1);
|
||||
}
|
||||
|
||||
/** Keep consecutive decisions ordered across whole-second Nostr timestamps. */
|
||||
export function nextProjectPullRequestReviewCreatedAt(pullRequest, now) {
|
||||
const latestDecisionCreatedAt = [
|
||||
...pullRequest.approvals,
|
||||
...pullRequest.changeRequests,
|
||||
].reduce((latest, decision) => Math.max(latest, decision.createdAt), 0);
|
||||
return Math.max(now, latestDecisionCreatedAt + 1);
|
||||
}
|
||||
|
||||
/** Trusted presentation kind for a compact review timeline row. */
|
||||
export function projectPullRequestCommentTimelineKind(comment) {
|
||||
if (comment.isTrustedReviewRequest) return "review-request";
|
||||
if (
|
||||
!comment.isTrustedReviewDecision ||
|
||||
!comment.reviewDecisionStatus ||
|
||||
!comment.reviewDecision
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return comment.reviewDecision;
|
||||
}
|
||||
|
||||
/** Effective review summary shown above the PR review actions. */
|
||||
export function projectPullRequestReviewSummary(pullRequest) {
|
||||
const approvalCount = pullRequest.approvals.length;
|
||||
const changeRequestCount = pullRequest.changeRequests.length;
|
||||
const isDraft = pullRequest.status === "Draft";
|
||||
const state = isDraft
|
||||
? "This pull request is still a work in progress."
|
||||
: changeRequestCount > 0
|
||||
? `${changeRequestCount} reviewer${changeRequestCount === 1 ? "" : "s"} requested changes.`
|
||||
: pullRequest.reviewers.length > 0
|
||||
? "Review requested — no approvals yet."
|
||||
: "No reviews yet.";
|
||||
|
||||
return {
|
||||
approvalCount,
|
||||
changeRequestCount,
|
||||
detail: isDraft
|
||||
? "Draft pull requests cannot be merged."
|
||||
: approvalCount === 0 && changeRequestCount === 0
|
||||
? "Approvals from reviewers will show up here."
|
||||
: null,
|
||||
showState: approvalCount === 0 || changeRequestCount > 0,
|
||||
state,
|
||||
};
|
||||
}
|
||||
|
||||
/** Effective trusted, current-commit review decision represented by a comment. */
|
||||
export function projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
comment,
|
||||
) {
|
||||
if (pullRequest.approvals.some((decision) => decision.id === comment.id)) {
|
||||
return "approved";
|
||||
}
|
||||
if (
|
||||
pullRequest.changeRequests.some((decision) => decision.id === comment.id)
|
||||
) {
|
||||
return "changes-requested";
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function eventToPullRequestUpdate(event) {
|
||||
return {
|
||||
id: event.id,
|
||||
@@ -83,17 +147,28 @@ function eventToPullRequestUpdate(event) {
|
||||
// for any client (including `buzz` CLI users) that treats them as comments.
|
||||
export const PR_REVIEW_REQUEST_LABEL = "review-request";
|
||||
export const PR_APPROVAL_LABEL = "approval";
|
||||
export const PR_CHANGES_REQUESTED_LABEL = "changes-requested";
|
||||
|
||||
function eventToPullRequestComment(event) {
|
||||
const labels = getAllTags(event, "t").map((label) => label.toLowerCase());
|
||||
const isReviewRequest = labels.includes(PR_REVIEW_REQUEST_LABEL);
|
||||
const isApproval = labels.includes(PR_APPROVAL_LABEL);
|
||||
const isChangeRequest = labels.includes(PR_CHANGES_REQUESTED_LABEL);
|
||||
return {
|
||||
id: event.id,
|
||||
content: event.content,
|
||||
author: event.pubkey,
|
||||
createdAt: event.created_at,
|
||||
isApproval: labels.includes(PR_APPROVAL_LABEL),
|
||||
commit: getTag(event, "c") ?? null,
|
||||
isApproval,
|
||||
isChangeRequest,
|
||||
isReviewRequest,
|
||||
reviewDecision:
|
||||
isApproval === isChangeRequest
|
||||
? null
|
||||
: isApproval
|
||||
? "approved"
|
||||
: "changes-requested",
|
||||
// For review requests the `p` tags are the requested reviewers.
|
||||
reviewerPubkeys: isReviewRequest
|
||||
? getAllTags(event, "p").map((pubkey) => pubkey.toLowerCase())
|
||||
@@ -125,27 +200,62 @@ function reviewersForPullRequest(pullRequest, comments) {
|
||||
return [...reviewers];
|
||||
}
|
||||
|
||||
/** Latest trusted approval per author, oldest first. */
|
||||
function approvalsForPullRequest(pullRequest, comments, reviewers) {
|
||||
const author = pullRequest.pubkey.toLowerCase();
|
||||
const trustedApprovers = new Set(reviewers);
|
||||
for (const actor of allowedActorsForRoot(pullRequest)) {
|
||||
if (actor !== author) trustedApprovers.add(actor);
|
||||
}
|
||||
function reviewDecisionCommit(comment, initialCommit) {
|
||||
return comment.commit ?? initialCommit;
|
||||
}
|
||||
|
||||
function trustedReviewActors(pullRequest, reviewers) {
|
||||
const author = pullRequest.pubkey.toLowerCase();
|
||||
const trustedActors = new Set(reviewers);
|
||||
for (const actor of allowedActorsForRoot(pullRequest)) {
|
||||
if (actor !== author) trustedActors.add(actor);
|
||||
}
|
||||
return trustedActors;
|
||||
}
|
||||
|
||||
/** Latest trusted, current-commit review decision per author. */
|
||||
function reviewDecisionsForPullRequest(
|
||||
comments,
|
||||
trustedActors,
|
||||
initialCommit,
|
||||
currentCommit,
|
||||
) {
|
||||
const byAuthor = new Map();
|
||||
for (const comment of comments) {
|
||||
if (!comment.isApproval) continue;
|
||||
if (!comment.reviewDecision || !currentCommit) continue;
|
||||
const key = comment.author.toLowerCase();
|
||||
if (!trustedApprovers.has(key)) continue;
|
||||
if (!trustedActors.has(key)) continue;
|
||||
const commit = reviewDecisionCommit(comment, initialCommit);
|
||||
if (commit !== currentCommit) continue;
|
||||
const existing = byAuthor.get(key);
|
||||
if (!existing || comment.createdAt > existing.createdAt) {
|
||||
byAuthor.set(key, comment);
|
||||
if (
|
||||
!existing ||
|
||||
comment.createdAt > existing.createdAt ||
|
||||
(comment.createdAt === existing.createdAt && comment.id > existing.id)
|
||||
) {
|
||||
byAuthor.set(key, { ...comment, commit });
|
||||
}
|
||||
}
|
||||
return [...byAuthor.values()]
|
||||
.map(({ id, author, createdAt }) => ({ id, author, createdAt }))
|
||||
.sort((left, right) => left.createdAt - right.createdAt);
|
||||
const decisions = [...byAuthor.values()]
|
||||
.map(({ id, author: reviewer, createdAt, commit, reviewDecision }) => ({
|
||||
id,
|
||||
author: reviewer,
|
||||
createdAt,
|
||||
commit,
|
||||
reviewDecision,
|
||||
}))
|
||||
.sort(
|
||||
(left, right) =>
|
||||
left.createdAt - right.createdAt || left.id.localeCompare(right.id),
|
||||
);
|
||||
return {
|
||||
approvals: decisions.filter(
|
||||
(decision) => decision.reviewDecision === "approved",
|
||||
),
|
||||
changeRequests: decisions.filter(
|
||||
(decision) => decision.reviewDecision === "changes-requested",
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
export function eventToProjectPullRequest(
|
||||
@@ -160,17 +270,41 @@ export function eventToProjectPullRequest(
|
||||
pullRequest.id,
|
||||
trustedUpdatesForPullRequest(pullRequest, updateEvents),
|
||||
).map(eventToPullRequestUpdate);
|
||||
const comments = eventsForPullRequest(pullRequest.id, commentEvents).map(
|
||||
eventToPullRequestComment,
|
||||
);
|
||||
const reviewers = reviewersForPullRequest(pullRequest, comments);
|
||||
const approvals = approvalsForPullRequest(pullRequest, comments, reviewers);
|
||||
const parsedComments = eventsForPullRequest(
|
||||
pullRequest.id,
|
||||
commentEvents,
|
||||
).map(eventToPullRequestComment);
|
||||
const reviewers = reviewersForPullRequest(pullRequest, parsedComments);
|
||||
const trustedActors = trustedReviewActors(pullRequest, reviewers);
|
||||
const trustedReviewRequestActors = allowedActorsForRoot(pullRequest);
|
||||
const latestCommit = getTag(latestUpdate ?? pullRequest, "c") ?? null;
|
||||
const initialCommit = getTag(pullRequest, "c") ?? null;
|
||||
const comments = parsedComments.map((comment) => ({
|
||||
...comment,
|
||||
isTrustedReviewDecision:
|
||||
Boolean(comment.reviewDecision) &&
|
||||
trustedActors.has(comment.author.toLowerCase()),
|
||||
reviewDecisionStatus:
|
||||
comment.reviewDecision && trustedActors.has(comment.author.toLowerCase())
|
||||
? latestCommit &&
|
||||
reviewDecisionCommit(comment, initialCommit) === latestCommit
|
||||
? "current"
|
||||
: "historical"
|
||||
: null,
|
||||
isTrustedReviewRequest:
|
||||
comment.isReviewRequest &&
|
||||
trustedReviewRequestActors.has(comment.author.toLowerCase()),
|
||||
}));
|
||||
const title =
|
||||
getTag(pullRequest, "subject") ||
|
||||
pullRequest.content.split("\n")[0] ||
|
||||
"Untitled pull request";
|
||||
const latestCommit = getTag(latestUpdate ?? pullRequest, "c") ?? null;
|
||||
const initialCommit = getTag(pullRequest, "c") ?? null;
|
||||
const reviewDecisions = reviewDecisionsForPullRequest(
|
||||
comments,
|
||||
trustedActors,
|
||||
initialCommit,
|
||||
latestCommit,
|
||||
);
|
||||
|
||||
return {
|
||||
id: pullRequest.id,
|
||||
@@ -182,7 +316,8 @@ export function eventToProjectPullRequest(
|
||||
labels: getAllTags(pullRequest, "t"),
|
||||
recipients: getAllTags(pullRequest, "p"),
|
||||
reviewers,
|
||||
approvals,
|
||||
approvals: reviewDecisions.approvals,
|
||||
changeRequests: reviewDecisions.changeRequests,
|
||||
status: statusFromEvent(pullRequest, latestStatus),
|
||||
statusEventId: latestStatus?.id ?? null,
|
||||
statusCreatedAt: latestStatus?.created_at ?? null,
|
||||
|
||||
@@ -3,7 +3,11 @@ import test from "node:test";
|
||||
|
||||
import {
|
||||
eventToProjectPullRequest,
|
||||
nextProjectPullRequestReviewCreatedAt,
|
||||
nextProjectPullRequestStatusCreatedAt,
|
||||
projectPullRequestCommentTimelineKind,
|
||||
projectPullRequestEffectiveReviewDecision,
|
||||
projectPullRequestReviewSummary,
|
||||
} from "./projectPullRequests.mjs";
|
||||
|
||||
const OWNER = "a".repeat(64);
|
||||
@@ -168,14 +172,16 @@ test("honors status events from the PR author and repo owner", () => {
|
||||
});
|
||||
|
||||
function commentEvent({
|
||||
id,
|
||||
pubkey,
|
||||
createdAt,
|
||||
content = "",
|
||||
labels = [],
|
||||
reviewers = [],
|
||||
commit,
|
||||
}) {
|
||||
return {
|
||||
id: `comment-${pubkey.slice(0, 8)}-${createdAt}`,
|
||||
id: id ?? `comment-${pubkey.slice(0, 8)}-${createdAt}`,
|
||||
kind: 1,
|
||||
pubkey,
|
||||
created_at: createdAt,
|
||||
@@ -185,6 +191,7 @@ function commentEvent({
|
||||
["a", REPO_ADDRESS],
|
||||
...reviewers.map((reviewer) => ["p", reviewer]),
|
||||
...labels.map((label) => ["t", label]),
|
||||
...(commit ? [["c", commit]] : []),
|
||||
],
|
||||
};
|
||||
}
|
||||
@@ -252,6 +259,16 @@ test("reviewers come from root p tags plus trusted review requests", () => {
|
||||
|
||||
// The author never reviews their own PR; untrusted requests are ignored.
|
||||
assert.deepEqual(pullRequest.reviewers.sort(), [reviewer, requested].sort());
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === AUTHOR)
|
||||
?.isTrustedReviewRequest,
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === ATTACKER)
|
||||
?.isTrustedReviewRequest,
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("approvals keep the latest per author and flag comments", () => {
|
||||
@@ -295,6 +312,292 @@ test("approvals keep the latest per author and flag comments", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("review decisions only apply to the current pull request commit", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const initialCommit = "1111111111111111111111111111111111111111";
|
||||
const currentCommit = "2222222222222222222222222222222222222222";
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const update = updateEvent({
|
||||
pubkey: AUTHOR,
|
||||
createdAt: 300,
|
||||
commit: currentCommit,
|
||||
});
|
||||
const staleApproval = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 200,
|
||||
labels: ["approval"],
|
||||
commit: initialCommit,
|
||||
});
|
||||
const currentChangeRequest = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 400,
|
||||
labels: ["changes-requested"],
|
||||
commit: currentCommit,
|
||||
});
|
||||
|
||||
const pullRequest = eventToProjectPullRequest(
|
||||
event,
|
||||
[update],
|
||||
[staleApproval, currentChangeRequest],
|
||||
);
|
||||
|
||||
assert.deepEqual(pullRequest.approvals, []);
|
||||
assert.equal(pullRequest.changeRequests.length, 1);
|
||||
assert.equal(pullRequest.changeRequests[0].author, reviewer);
|
||||
assert.equal(pullRequest.changeRequests[0].commit, currentCommit);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.id === staleApproval.id)
|
||||
?.reviewDecisionStatus,
|
||||
"historical",
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find(
|
||||
(comment) => comment.id === currentChangeRequest.id,
|
||||
)?.reviewDecisionStatus,
|
||||
"current",
|
||||
);
|
||||
});
|
||||
|
||||
test("the latest current-commit review decision replaces the previous state", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const commit = "1111111111111111111111111111111111111111";
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const changeRequest = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 200,
|
||||
labels: ["changes-requested"],
|
||||
commit,
|
||||
});
|
||||
const approval = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 300,
|
||||
labels: ["approval"],
|
||||
commit,
|
||||
});
|
||||
|
||||
const pullRequest = eventToProjectPullRequest(
|
||||
event,
|
||||
[],
|
||||
[changeRequest, approval],
|
||||
);
|
||||
|
||||
assert.equal(pullRequest.approvals.length, 1);
|
||||
assert.equal(pullRequest.approvals[0].author, reviewer);
|
||||
assert.deepEqual(pullRequest.changeRequests, []);
|
||||
assert.equal(
|
||||
pullRequest.comments.filter((comment) => comment.isChangeRequest).length,
|
||||
1,
|
||||
);
|
||||
assert.equal(nextProjectPullRequestReviewCreatedAt(pullRequest, 300), 301);
|
||||
assert.equal(nextProjectPullRequestReviewCreatedAt(pullRequest, 400), 400);
|
||||
});
|
||||
|
||||
test("effective review decisions exclude superseded and untrusted labels", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const supersededApproval = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 200,
|
||||
labels: ["approval"],
|
||||
});
|
||||
const effectiveChangeRequest = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 300,
|
||||
labels: ["changes-requested"],
|
||||
});
|
||||
const effectiveApproval = commentEvent({
|
||||
pubkey: OWNER,
|
||||
createdAt: 350,
|
||||
labels: ["approval"],
|
||||
});
|
||||
const untrustedApproval = commentEvent({
|
||||
pubkey: ATTACKER,
|
||||
createdAt: 400,
|
||||
labels: ["approval"],
|
||||
});
|
||||
const pullRequest = eventToProjectPullRequest(
|
||||
event,
|
||||
[],
|
||||
[
|
||||
supersededApproval,
|
||||
effectiveChangeRequest,
|
||||
effectiveApproval,
|
||||
untrustedApproval,
|
||||
],
|
||||
);
|
||||
const commentById = new Map(
|
||||
pullRequest.comments.map((comment) => [comment.id, comment]),
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
commentById.get(effectiveChangeRequest.id),
|
||||
),
|
||||
"changes-requested",
|
||||
);
|
||||
assert.equal(
|
||||
projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
commentById.get(effectiveApproval.id),
|
||||
),
|
||||
"approved",
|
||||
);
|
||||
assert.equal(
|
||||
projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
commentById.get(supersededApproval.id),
|
||||
),
|
||||
null,
|
||||
);
|
||||
assert.equal(
|
||||
projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
commentById.get(untrustedApproval.id),
|
||||
),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("mixed effective decisions keep requested changes visible", () => {
|
||||
const reviewerOne = "d".repeat(64);
|
||||
const reviewerTwo = "e".repeat(64);
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewerOne], ["p", reviewerTwo]);
|
||||
const pullRequest = eventToProjectPullRequest(
|
||||
event,
|
||||
[],
|
||||
[
|
||||
commentEvent({
|
||||
pubkey: reviewerOne,
|
||||
createdAt: 200,
|
||||
labels: ["approval"],
|
||||
}),
|
||||
commentEvent({
|
||||
pubkey: reviewerTwo,
|
||||
createdAt: 300,
|
||||
labels: ["changes-requested"],
|
||||
}),
|
||||
],
|
||||
);
|
||||
|
||||
const summary = projectPullRequestReviewSummary(pullRequest);
|
||||
|
||||
assert.equal(summary.approvalCount, 1);
|
||||
assert.equal(summary.changeRequestCount, 1);
|
||||
assert.equal(summary.showState, true);
|
||||
assert.equal(summary.state, "1 reviewer requested changes.");
|
||||
});
|
||||
|
||||
test("trusted review requests cannot borrow decision timeline visuals", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const request = commentEvent({
|
||||
pubkey: AUTHOR,
|
||||
createdAt: 200,
|
||||
labels: ["review-request", "approval"],
|
||||
reviewers: [reviewer],
|
||||
});
|
||||
const pullRequest = eventToProjectPullRequest(
|
||||
pullRequestEvent(),
|
||||
[],
|
||||
[request],
|
||||
);
|
||||
const comment = pullRequest.comments[0];
|
||||
|
||||
assert.equal(comment.isTrustedReviewRequest, true);
|
||||
assert.equal(comment.isTrustedReviewDecision, false);
|
||||
assert.equal(
|
||||
projectPullRequestCommentTimelineKind(comment),
|
||||
"review-request",
|
||||
);
|
||||
});
|
||||
|
||||
test("equal-timestamp decisions use event ID ordering in both input orders", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const commit = "1111111111111111111111111111111111111111";
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const approval = commentEvent({
|
||||
id: "a".repeat(64),
|
||||
pubkey: reviewer,
|
||||
createdAt: 300,
|
||||
labels: ["approval"],
|
||||
commit,
|
||||
});
|
||||
const changeRequest = commentEvent({
|
||||
id: "b".repeat(64),
|
||||
pubkey: reviewer,
|
||||
createdAt: 300,
|
||||
labels: ["changes-requested"],
|
||||
commit,
|
||||
});
|
||||
|
||||
for (const comments of [
|
||||
[approval, changeRequest],
|
||||
[changeRequest, approval],
|
||||
]) {
|
||||
const pullRequest = eventToProjectPullRequest(event, [], comments);
|
||||
|
||||
assert.deepEqual(pullRequest.approvals, []);
|
||||
assert.equal(pullRequest.changeRequests.length, 1);
|
||||
assert.equal(pullRequest.changeRequests[0].id, changeRequest.id);
|
||||
}
|
||||
});
|
||||
|
||||
test("legacy unscoped review decisions are invalidated by later PR updates", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const approval = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 200,
|
||||
labels: ["approval"],
|
||||
});
|
||||
const update = updateEvent({
|
||||
pubkey: AUTHOR,
|
||||
createdAt: 300,
|
||||
commit: "2222222222222222222222222222222222222222",
|
||||
});
|
||||
|
||||
const pullRequest = eventToProjectPullRequest(event, [update], [approval]);
|
||||
|
||||
assert.deepEqual(pullRequest.approvals, []);
|
||||
assert.deepEqual(pullRequest.changeRequests, []);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.id === approval.id)
|
||||
?.reviewDecisionStatus,
|
||||
"historical",
|
||||
);
|
||||
});
|
||||
|
||||
test("legacy decisions stay bound to the initial commit after a backdated update", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const event = pullRequestEvent();
|
||||
event.tags.push(["p", reviewer]);
|
||||
const update = updateEvent({
|
||||
pubkey: AUTHOR,
|
||||
createdAt: 200,
|
||||
commit: "2222222222222222222222222222222222222222",
|
||||
});
|
||||
const approval = commentEvent({
|
||||
pubkey: reviewer,
|
||||
createdAt: 300,
|
||||
labels: ["approval"],
|
||||
});
|
||||
|
||||
const pullRequest = eventToProjectPullRequest(event, [update], [approval]);
|
||||
|
||||
assert.deepEqual(pullRequest.approvals, []);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.id === approval.id)
|
||||
?.reviewDecisionStatus,
|
||||
"historical",
|
||||
);
|
||||
});
|
||||
|
||||
test("approvals only count requested reviewers and the repository owner", () => {
|
||||
const reviewer = "d".repeat(64);
|
||||
const event = pullRequestEvent();
|
||||
@@ -328,6 +631,26 @@ test("approvals only count requested reviewers and the repository owner", () =>
|
||||
pullRequest.approvals.map((approval) => approval.author),
|
||||
[reviewer, OWNER],
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === reviewer)
|
||||
?.isTrustedReviewDecision,
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === reviewer)
|
||||
?.reviewDecisionStatus,
|
||||
"current",
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === ATTACKER)
|
||||
?.isTrustedReviewDecision,
|
||||
false,
|
||||
);
|
||||
assert.equal(
|
||||
pullRequest.comments.find((comment) => comment.author === ATTACKER)
|
||||
?.reviewDecisionStatus,
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("survives malformed value-less tags", () => {
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
} from "./projectPullRequests.mjs";
|
||||
import {
|
||||
PR_APPROVAL_LABEL,
|
||||
PR_CHANGES_REQUESTED_LABEL,
|
||||
PR_REVIEW_REQUEST_LABEL,
|
||||
} from "./projectPullRequests.mjs";
|
||||
|
||||
@@ -122,32 +123,67 @@ async function requestProjectPullRequestReview({
|
||||
);
|
||||
}
|
||||
|
||||
async function approveProjectPullRequest({
|
||||
type ProjectPullRequestReviewDecision = "approve" | "request-changes";
|
||||
|
||||
const REVIEW_DECISION_DETAILS: Record<
|
||||
ProjectPullRequestReviewDecision,
|
||||
{
|
||||
content: string;
|
||||
errorMessage: string;
|
||||
label: string;
|
||||
timeoutMessage: string;
|
||||
}
|
||||
> = {
|
||||
approve: {
|
||||
content: "Approved these changes",
|
||||
errorMessage: "Failed to approve pull request.",
|
||||
label: PR_APPROVAL_LABEL,
|
||||
timeoutMessage: "Timed out approving pull request.",
|
||||
},
|
||||
"request-changes": {
|
||||
content: "Requested changes",
|
||||
errorMessage: "Failed to request changes.",
|
||||
label: PR_CHANGES_REQUESTED_LABEL,
|
||||
timeoutMessage: "Timed out requesting changes.",
|
||||
},
|
||||
};
|
||||
|
||||
async function submitProjectPullRequestReview({
|
||||
createdAt,
|
||||
decision,
|
||||
project,
|
||||
pullRequest,
|
||||
}: {
|
||||
createdAt: number;
|
||||
decision: ProjectPullRequestReviewDecision;
|
||||
project: Project;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}): Promise<void> {
|
||||
if (!pullRequest.commit) {
|
||||
throw new Error("The pull request has no commit to review.");
|
||||
}
|
||||
const details = REVIEW_DECISION_DETAILS[decision];
|
||||
const recipients = new Set([
|
||||
project.owner.toLowerCase(),
|
||||
pullRequest.author.toLowerCase(),
|
||||
]);
|
||||
const event = await signRelayEvent({
|
||||
kind: KIND_TEXT_NOTE,
|
||||
content: "Approved these changes",
|
||||
content: details.content,
|
||||
createdAt,
|
||||
tags: [
|
||||
["e", pullRequest.id, "", "root"],
|
||||
["a", project.repoAddress],
|
||||
...[...recipients].map((recipient) => ["p", recipient]),
|
||||
["t", PR_APPROVAL_LABEL],
|
||||
["t", details.label],
|
||||
["c", pullRequest.commit],
|
||||
],
|
||||
});
|
||||
|
||||
await relayClient.publishEvent(
|
||||
event,
|
||||
"Timed out approving pull request.",
|
||||
"Failed to approve pull request.",
|
||||
details.timeoutMessage,
|
||||
details.errorMessage,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -218,16 +254,43 @@ export function useRequestProjectPullRequestReviewMutation(
|
||||
});
|
||||
}
|
||||
|
||||
export function useApproveProjectPullRequestMutation(
|
||||
function useProjectPullRequestReviewDecisionMutation(
|
||||
project: Project | null | undefined,
|
||||
decision: ProjectPullRequestReviewDecision,
|
||||
) {
|
||||
const invalidate = useProjectPullRequestWriteInvalidation(project);
|
||||
|
||||
return useMutation({
|
||||
mutationFn: ({ pullRequest }: { pullRequest: ProjectPullRequest }) => {
|
||||
mutationFn: ({
|
||||
createdAt,
|
||||
pullRequest,
|
||||
}: {
|
||||
createdAt: number;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}) => {
|
||||
if (!project) throw new Error("No project selected.");
|
||||
return approveProjectPullRequest({ project, pullRequest });
|
||||
return submitProjectPullRequestReview({
|
||||
createdAt,
|
||||
decision,
|
||||
project,
|
||||
pullRequest,
|
||||
});
|
||||
},
|
||||
onSuccess: invalidate,
|
||||
});
|
||||
}
|
||||
|
||||
export function useApproveProjectPullRequestMutation(
|
||||
project: Project | null | undefined,
|
||||
) {
|
||||
return useProjectPullRequestReviewDecisionMutation(project, "approve");
|
||||
}
|
||||
|
||||
export function useRequestProjectPullRequestChangesMutation(
|
||||
project: Project | null | undefined,
|
||||
) {
|
||||
return useProjectPullRequestReviewDecisionMutation(
|
||||
project,
|
||||
"request-changes",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
GitPullRequest,
|
||||
MessageSquare,
|
||||
UserPlus,
|
||||
X,
|
||||
} from "lucide-react";
|
||||
import type { ComponentType } from "react";
|
||||
|
||||
@@ -18,6 +19,7 @@ export type ProjectEventKind =
|
||||
| "issue"
|
||||
| "comment"
|
||||
| "approval"
|
||||
| "changes-requested"
|
||||
| "review-request";
|
||||
|
||||
export const PROJECT_EVENT_VISUALS: Record<
|
||||
@@ -70,6 +72,12 @@ export const PROJECT_EVENT_VISUALS: Record<
|
||||
detailClassName:
|
||||
"border-green-600/30 text-green-700 dark:border-green-500/30 dark:text-green-400",
|
||||
},
|
||||
"changes-requested": {
|
||||
icon: X,
|
||||
iconClassName: "text-destructive",
|
||||
badgeClassName: "bg-destructive/10 text-destructive",
|
||||
detailClassName: "border-destructive/40 text-destructive",
|
||||
},
|
||||
"review-request": {
|
||||
icon: UserPlus,
|
||||
iconClassName: "text-blue-600 dark:text-blue-400",
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
GitCommitHorizontal,
|
||||
GitMerge,
|
||||
GitPullRequest,
|
||||
GitPullRequestDraft,
|
||||
MessageSquare,
|
||||
UserPlus,
|
||||
X,
|
||||
@@ -19,16 +18,12 @@ import {
|
||||
type ProjectPullRequest,
|
||||
useCreateProjectPullRequestCommentMutation,
|
||||
} from "@/features/projects/hooks";
|
||||
import {
|
||||
useApproveProjectPullRequestMutation,
|
||||
useUpdateProjectPullRequestStatusMutation,
|
||||
} from "@/features/projects/pullRequestReviews";
|
||||
import { projectPullRequestCommentTimelineKind } from "@/features/projects/projectPullRequests.mjs";
|
||||
import { relativeTime } from "@/features/projects/lib/projectsViewHelpers";
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import { useIdentityQuery } from "@/shared/api/hooks";
|
||||
import type { ChannelMember } from "@/shared/api/types";
|
||||
import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { Markdown } from "@/shared/ui/markdown";
|
||||
import {
|
||||
ProjectFeedRow,
|
||||
@@ -41,8 +36,8 @@ import {
|
||||
ProfileAuthorName,
|
||||
ProfileIdentityButton,
|
||||
} from "./ProjectProfileIdentity";
|
||||
import { MergePullRequestButton } from "./MergePullRequestButton";
|
||||
import { PullRequestReviewersRow } from "./PullRequestReviewersRow";
|
||||
import { PullRequestReviewCard } from "./PullRequestReviewCard";
|
||||
|
||||
function compactDate(createdAt: number) {
|
||||
return new Date(createdAt * 1_000).toLocaleDateString(undefined, {
|
||||
@@ -328,174 +323,6 @@ function PullRequestRow({
|
||||
|
||||
export type PullRequestPanelMode = "conversation" | "commits" | "checks";
|
||||
|
||||
/** GitHub-style review state and actions rendered in the conversation flow. */
|
||||
function PullRequestReviewCard({
|
||||
project,
|
||||
pullRequest,
|
||||
}: {
|
||||
project: Project;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}) {
|
||||
const identityQuery = useIdentityQuery();
|
||||
const statusMutation = useUpdateProjectPullRequestStatusMutation(project);
|
||||
const approveMutation = useApproveProjectPullRequestMutation(project);
|
||||
|
||||
const viewerPubkey = identityQuery.data?.pubkey ?? null;
|
||||
const viewer = viewerPubkey ? normalizePubkey(viewerPubkey) : null;
|
||||
const isAuthor = viewer === normalizePubkey(pullRequest.author);
|
||||
const isOwner = viewer === normalizePubkey(project.owner);
|
||||
const isManagedAgentOwner = useIsManagedAgent(project.owner) === true;
|
||||
const canChangeStatus = Boolean(viewer) && (isAuthor || isOwner);
|
||||
const hasApproved = Boolean(
|
||||
viewer &&
|
||||
pullRequest.approvals.some(
|
||||
(approval) => normalizePubkey(approval.author) === viewer,
|
||||
),
|
||||
);
|
||||
const canApprove =
|
||||
Boolean(viewer) &&
|
||||
!isAuthor &&
|
||||
!hasApproved &&
|
||||
(pullRequest.status === "Open" || pullRequest.status === "Draft");
|
||||
const canMerge =
|
||||
(isOwner || isManagedAgentOwner) &&
|
||||
pullRequest.status === "Open" &&
|
||||
Boolean(pullRequest.branchName && pullRequest.commit);
|
||||
|
||||
const handleStatusChange = React.useCallback(
|
||||
async (status: "open" | "draft") => {
|
||||
try {
|
||||
await statusMutation.mutateAsync({ pullRequest, status });
|
||||
toast.success(
|
||||
status === "draft"
|
||||
? "Converted to draft."
|
||||
: "Marked as ready for review.",
|
||||
);
|
||||
} catch (error) {
|
||||
toast.error(
|
||||
error instanceof Error ? error.message : "Failed to update status.",
|
||||
);
|
||||
}
|
||||
},
|
||||
[pullRequest, statusMutation],
|
||||
);
|
||||
|
||||
const handleApprove = React.useCallback(async () => {
|
||||
try {
|
||||
await approveMutation.mutateAsync({ pullRequest });
|
||||
toast.success("Pull request approved.");
|
||||
} catch (error) {
|
||||
toast.error(
|
||||
error instanceof Error ? error.message : "Failed to approve.",
|
||||
);
|
||||
}
|
||||
}, [approveMutation, pullRequest]);
|
||||
|
||||
const approvalCount = pullRequest.approvals.length;
|
||||
const isDraft = pullRequest.status === "Draft";
|
||||
const reviewState = isDraft
|
||||
? "This pull request is still a work in progress."
|
||||
: pullRequest.reviewers.length > 0
|
||||
? "Review requested — no approvals yet."
|
||||
: "No reviews yet.";
|
||||
const reviewStateDetail = isDraft
|
||||
? "Draft pull requests cannot be merged."
|
||||
: approvalCount === 0
|
||||
? "Approvals from reviewers will show up here."
|
||||
: null;
|
||||
const showActions =
|
||||
hasApproved || canApprove || canMerge || (canChangeStatus && isDraft);
|
||||
const showDraftControl = canChangeStatus && pullRequest.status === "Open";
|
||||
|
||||
if (approvalCount > 0 && !showActions && !showDraftControl) return null;
|
||||
|
||||
return (
|
||||
<div className="space-y-2.5 pt-3">
|
||||
<div className="min-w-0 space-y-2.5 rounded-xl bg-muted/40 px-3 py-2.5">
|
||||
{approvalCount === 0 ? (
|
||||
<div className="flex min-w-0 items-start gap-2">
|
||||
{isDraft ? (
|
||||
<GitPullRequestDraft className="mt-0.5 h-4 w-4 shrink-0 text-muted-foreground" />
|
||||
) : (
|
||||
<GitPullRequest className="mt-0.5 h-4 w-4 shrink-0 text-muted-foreground" />
|
||||
)}
|
||||
<div className="min-w-0 flex-1">
|
||||
<p className="text-sm font-medium text-foreground">
|
||||
{reviewState}
|
||||
</p>
|
||||
{reviewStateDetail ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{reviewStateDetail}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
) : null}
|
||||
{showActions ? (
|
||||
<div className="flex min-w-0 flex-wrap items-center gap-2">
|
||||
{hasApproved ? (
|
||||
<span className="inline-flex h-8 items-center justify-center gap-1.5 rounded-lg border border-green-600/40 px-3.5 text-xs font-medium text-green-600 dark:text-green-500">
|
||||
<Check className="h-3.5 w-3.5" />
|
||||
Approved
|
||||
</span>
|
||||
) : null}
|
||||
{canApprove ? (
|
||||
<Button
|
||||
className="h-8 gap-1.5 bg-green-600 px-3.5 text-white shadow-sm hover:bg-green-700"
|
||||
disabled={approveMutation.isPending}
|
||||
onClick={() => {
|
||||
void handleApprove();
|
||||
}}
|
||||
size="xs"
|
||||
type="button"
|
||||
>
|
||||
<Check className="h-3.5 w-3.5" />
|
||||
Approve
|
||||
</Button>
|
||||
) : null}
|
||||
{canMerge ? (
|
||||
<MergePullRequestButton
|
||||
project={project}
|
||||
pullRequest={pullRequest}
|
||||
/>
|
||||
) : null}
|
||||
{canChangeStatus && isDraft ? (
|
||||
<Button
|
||||
className="h-7 gap-1.5 px-3"
|
||||
disabled={statusMutation.isPending}
|
||||
onClick={() => {
|
||||
void handleStatusChange("open");
|
||||
}}
|
||||
size="xs"
|
||||
type="button"
|
||||
variant="secondary"
|
||||
>
|
||||
<GitPullRequest className="h-3.5 w-3.5" />
|
||||
Ready for review
|
||||
</Button>
|
||||
) : null}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
{showDraftControl ? (
|
||||
<p className="px-1 text-xs text-muted-foreground">
|
||||
Still in progress?{" "}
|
||||
<button
|
||||
className="font-medium underline-offset-2 hover:text-foreground hover:underline disabled:opacity-50"
|
||||
disabled={statusMutation.isPending}
|
||||
onClick={() => {
|
||||
void handleStatusChange("draft");
|
||||
}}
|
||||
type="button"
|
||||
>
|
||||
Convert to draft
|
||||
</button>
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
/** GitHub-style PR title line, rendered as the top section of the PR detail
|
||||
* card. Status, branches, and dates live in the right-hand meta rail. */
|
||||
export function PullRequestDetailHeader({
|
||||
@@ -749,16 +576,33 @@ function PullRequestDetail({
|
||||
{pullRequest.comments.length > 0 ? (
|
||||
<div className="-mt-4">
|
||||
{pullRequest.comments.map((item) => {
|
||||
// Approvals and review requests render as compact timeline
|
||||
// Review decisions and requests render as compact timeline
|
||||
// rows (GitHub-style) rather than full comment cards.
|
||||
if (item.isApproval || item.isReviewRequest) {
|
||||
const timelineKind = projectPullRequestCommentTimelineKind(item);
|
||||
if (timelineKind) {
|
||||
const isHistoricalDecision =
|
||||
item.reviewDecisionStatus === "historical";
|
||||
return (
|
||||
<div
|
||||
className="-mx-4 flex min-h-10 min-w-0 items-center gap-2 border-b border-border/50 px-4 text-sm text-muted-foreground"
|
||||
key={item.id}
|
||||
>
|
||||
{item.isApproval ? (
|
||||
<Check className="h-3.5 w-3.5 shrink-0 text-green-600 dark:text-green-500" />
|
||||
{timelineKind === "approved" ? (
|
||||
<Check
|
||||
className={`h-3.5 w-3.5 shrink-0 ${
|
||||
isHistoricalDecision
|
||||
? "text-muted-foreground"
|
||||
: "text-green-600 dark:text-green-500"
|
||||
}`}
|
||||
/>
|
||||
) : timelineKind === "changes-requested" ? (
|
||||
<X
|
||||
className={`h-3.5 w-3.5 shrink-0 ${
|
||||
isHistoricalDecision
|
||||
? "text-muted-foreground"
|
||||
: "text-destructive"
|
||||
}`}
|
||||
/>
|
||||
) : (
|
||||
<UserPlus className="h-3.5 w-3.5 shrink-0" />
|
||||
)}
|
||||
@@ -767,9 +611,15 @@ function PullRequestDetail({
|
||||
{labelForPubkey(item.author, profiles)}
|
||||
</span>
|
||||
<span className="min-w-0 truncate">
|
||||
{item.isApproval
|
||||
? "approved these changes"
|
||||
: item.content.trim() || "requested a review"}
|
||||
{timelineKind === "approved"
|
||||
? isHistoricalDecision
|
||||
? "approved an earlier commit"
|
||||
: "approved these changes"
|
||||
: timelineKind === "changes-requested"
|
||||
? isHistoricalDecision
|
||||
? "requested changes on an earlier commit"
|
||||
: "requested changes"
|
||||
: item.content.trim() || "requested a review"}
|
||||
</span>
|
||||
</span>
|
||||
<span className="w-20 shrink-0 text-right text-xs text-muted-foreground/70">
|
||||
|
||||
@@ -16,6 +16,10 @@ import {
|
||||
markdownToPlainText,
|
||||
relativeTime,
|
||||
} from "@/features/projects/lib/projectsViewHelpers";
|
||||
import {
|
||||
projectPullRequestCommentTimelineKind,
|
||||
projectPullRequestEffectiveReviewDecision,
|
||||
} from "@/features/projects/projectPullRequests.mjs";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip";
|
||||
@@ -154,25 +158,41 @@ function buildActivityItems({
|
||||
});
|
||||
}
|
||||
for (const comment of pullRequest.comments) {
|
||||
const kind = comment.isApproval
|
||||
? "approval"
|
||||
: comment.isReviewRequest
|
||||
? "review-request"
|
||||
: "comment";
|
||||
const reviewDecision = projectPullRequestEffectiveReviewDecision(
|
||||
pullRequest,
|
||||
comment,
|
||||
);
|
||||
const timelineKind = projectPullRequestCommentTimelineKind(comment);
|
||||
const kind =
|
||||
reviewDecision === "approved"
|
||||
? "approval"
|
||||
: reviewDecision === "changes-requested"
|
||||
? "changes-requested"
|
||||
: timelineKind === "review-request"
|
||||
? "review-request"
|
||||
: "comment";
|
||||
items.push({
|
||||
id: `pr-comment:${comment.id}`,
|
||||
kind,
|
||||
createdAt: comment.createdAt,
|
||||
actorPubkey: comment.author,
|
||||
actorName: null,
|
||||
action: comment.isApproval
|
||||
? "approved a pull request in"
|
||||
: comment.isReviewRequest
|
||||
? "requested review in"
|
||||
: "commented on a pull request in",
|
||||
action:
|
||||
kind === "approval"
|
||||
? "approved a pull request in"
|
||||
: kind === "changes-requested"
|
||||
? "requested changes to a pull request in"
|
||||
: kind === "review-request"
|
||||
? "requested review in"
|
||||
: "commented on a pull request in",
|
||||
title: pullRequest.title,
|
||||
body: contentPreview(comment.content),
|
||||
detail: null,
|
||||
detail:
|
||||
kind === "approval"
|
||||
? "Approved"
|
||||
: kind === "changes-requested"
|
||||
? "Changes requested"
|
||||
: null,
|
||||
target,
|
||||
});
|
||||
}
|
||||
|
||||
278
desktop/src/features/projects/ui/PullRequestReviewCard.tsx
Normal file
278
desktop/src/features/projects/ui/PullRequestReviewCard.tsx
Normal file
@@ -0,0 +1,278 @@
|
||||
import { Check, GitPullRequest, GitPullRequestDraft, X } from "lucide-react";
|
||||
import * as React from "react";
|
||||
import { toast } from "sonner";
|
||||
|
||||
import { useIsManagedAgent } from "@/features/agent-memory/hooks";
|
||||
import type { Project, ProjectPullRequest } from "@/features/projects/hooks";
|
||||
import {
|
||||
nextProjectPullRequestReviewCreatedAt,
|
||||
projectPullRequestReviewSummary,
|
||||
} from "@/features/projects/projectPullRequests.mjs";
|
||||
import {
|
||||
useApproveProjectPullRequestMutation,
|
||||
useRequestProjectPullRequestChangesMutation,
|
||||
useUpdateProjectPullRequestStatusMutation,
|
||||
} from "@/features/projects/pullRequestReviews";
|
||||
import { useIdentityQuery } from "@/shared/api/hooks";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { MergePullRequestButton } from "./MergePullRequestButton";
|
||||
|
||||
/** GitHub-style review state and actions rendered in the conversation flow. */
|
||||
export function PullRequestReviewCard({
|
||||
project,
|
||||
pullRequest,
|
||||
}: {
|
||||
project: Project;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}) {
|
||||
const identityQuery = useIdentityQuery();
|
||||
const { isPending: isUpdatingStatus, mutateAsync: updatePullRequestStatus } =
|
||||
useUpdateProjectPullRequestStatusMutation(project);
|
||||
const { isPending: isApproving, mutateAsync: approvePullRequest } =
|
||||
useApproveProjectPullRequestMutation(project);
|
||||
const {
|
||||
isPending: isRequestingChanges,
|
||||
mutateAsync: requestPullRequestChanges,
|
||||
} = useRequestProjectPullRequestChangesMutation(project);
|
||||
const reviewDecisionInFlightRef = React.useRef(false);
|
||||
const lastReviewDecisionCreatedAtRef = React.useRef(0);
|
||||
|
||||
const viewerPubkey = identityQuery.data?.pubkey ?? null;
|
||||
const viewer = viewerPubkey ? normalizePubkey(viewerPubkey) : null;
|
||||
const isAuthor = viewer === normalizePubkey(pullRequest.author);
|
||||
const isOwner = viewer === normalizePubkey(project.owner);
|
||||
const isRequestedReviewer = Boolean(
|
||||
viewer &&
|
||||
pullRequest.reviewers.some(
|
||||
(reviewer) => normalizePubkey(reviewer) === viewer,
|
||||
),
|
||||
);
|
||||
const isManagedAgentOwner = useIsManagedAgent(project.owner) === true;
|
||||
const canChangeStatus = Boolean(viewer) && (isAuthor || isOwner);
|
||||
const hasApproved = Boolean(
|
||||
viewer &&
|
||||
pullRequest.approvals.some(
|
||||
(approval) => normalizePubkey(approval.author) === viewer,
|
||||
),
|
||||
);
|
||||
const hasRequestedChanges = Boolean(
|
||||
viewer &&
|
||||
pullRequest.changeRequests.some(
|
||||
(request) => normalizePubkey(request.author) === viewer,
|
||||
),
|
||||
);
|
||||
const canReview =
|
||||
Boolean(viewer) &&
|
||||
!isAuthor &&
|
||||
(isOwner || isRequestedReviewer) &&
|
||||
Boolean(pullRequest.commit) &&
|
||||
(pullRequest.status === "Open" || pullRequest.status === "Draft");
|
||||
const canApprove = canReview && !hasApproved;
|
||||
const canRequestChanges = canReview && !hasRequestedChanges;
|
||||
const canMerge =
|
||||
(isOwner || isManagedAgentOwner) &&
|
||||
pullRequest.status === "Open" &&
|
||||
Boolean(pullRequest.branchName && pullRequest.commit);
|
||||
|
||||
const handleStatusChange = React.useCallback(
|
||||
async (status: "open" | "draft") => {
|
||||
try {
|
||||
await updatePullRequestStatus({ pullRequest, status });
|
||||
toast.success(
|
||||
status === "draft"
|
||||
? "Converted to draft."
|
||||
: "Marked as ready for review.",
|
||||
);
|
||||
} catch (error) {
|
||||
toast.error(
|
||||
error instanceof Error ? error.message : "Failed to update status.",
|
||||
);
|
||||
}
|
||||
},
|
||||
[pullRequest, updatePullRequestStatus],
|
||||
);
|
||||
|
||||
const runReviewDecision = React.useCallback(
|
||||
async (
|
||||
mutate: (input: {
|
||||
createdAt: number;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}) => Promise<unknown>,
|
||||
successMessage: string,
|
||||
fallbackErrorMessage: string,
|
||||
) => {
|
||||
if (reviewDecisionInFlightRef.current) return;
|
||||
reviewDecisionInFlightRef.current = true;
|
||||
const createdAt = Math.max(
|
||||
nextProjectPullRequestReviewCreatedAt(
|
||||
pullRequest,
|
||||
Math.floor(Date.now() / 1_000),
|
||||
),
|
||||
lastReviewDecisionCreatedAtRef.current + 1,
|
||||
);
|
||||
lastReviewDecisionCreatedAtRef.current = createdAt;
|
||||
|
||||
try {
|
||||
await mutate({ createdAt, pullRequest });
|
||||
toast.success(successMessage);
|
||||
} catch (error) {
|
||||
toast.error(
|
||||
error instanceof Error ? error.message : fallbackErrorMessage,
|
||||
);
|
||||
} finally {
|
||||
reviewDecisionInFlightRef.current = false;
|
||||
}
|
||||
},
|
||||
[pullRequest],
|
||||
);
|
||||
|
||||
const handleApprove = React.useCallback(async () => {
|
||||
await runReviewDecision(
|
||||
approvePullRequest,
|
||||
"Pull request approved.",
|
||||
"Failed to approve.",
|
||||
);
|
||||
}, [approvePullRequest, runReviewDecision]);
|
||||
|
||||
const handleRequestChanges = React.useCallback(async () => {
|
||||
await runReviewDecision(
|
||||
requestPullRequestChanges,
|
||||
"Changes requested.",
|
||||
"Failed to request changes.",
|
||||
);
|
||||
}, [requestPullRequestChanges, runReviewDecision]);
|
||||
|
||||
const {
|
||||
approvalCount,
|
||||
changeRequestCount,
|
||||
detail: reviewStateDetail,
|
||||
showState: showReviewState,
|
||||
state: reviewState,
|
||||
} = projectPullRequestReviewSummary(pullRequest);
|
||||
const reviewDecisionPending = isApproving || isRequestingChanges;
|
||||
const isDraft = pullRequest.status === "Draft";
|
||||
const showActions =
|
||||
hasApproved ||
|
||||
hasRequestedChanges ||
|
||||
canApprove ||
|
||||
canRequestChanges ||
|
||||
canMerge ||
|
||||
(canChangeStatus && isDraft);
|
||||
const showDraftControl = canChangeStatus && pullRequest.status === "Open";
|
||||
|
||||
if (
|
||||
approvalCount + changeRequestCount > 0 &&
|
||||
!showActions &&
|
||||
!showDraftControl
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="space-y-2.5 pt-3">
|
||||
<div className="min-w-0 space-y-2.5 rounded-xl bg-muted/40 px-3 py-2.5">
|
||||
{showReviewState ? (
|
||||
<div className="flex min-w-0 items-start gap-2">
|
||||
{isDraft ? (
|
||||
<GitPullRequestDraft className="mt-0.5 h-4 w-4 shrink-0 text-muted-foreground" />
|
||||
) : (
|
||||
<GitPullRequest className="mt-0.5 h-4 w-4 shrink-0 text-muted-foreground" />
|
||||
)}
|
||||
<div className="min-w-0 flex-1">
|
||||
<p className="text-sm font-medium text-foreground">
|
||||
{reviewState}
|
||||
</p>
|
||||
{reviewStateDetail ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{reviewStateDetail}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
) : null}
|
||||
{showActions ? (
|
||||
<div className="flex min-w-0 flex-wrap items-center gap-2">
|
||||
{hasApproved ? (
|
||||
<span className="inline-flex h-8 items-center justify-center gap-1.5 rounded-lg border border-green-600/40 px-3.5 text-xs font-medium text-green-600 dark:text-green-500">
|
||||
<Check className="h-3.5 w-3.5" />
|
||||
Approved
|
||||
</span>
|
||||
) : null}
|
||||
{hasRequestedChanges ? (
|
||||
<span className="inline-flex h-8 items-center justify-center gap-1.5 rounded-lg border border-destructive/40 px-3.5 text-xs font-medium text-destructive">
|
||||
<X className="h-3.5 w-3.5" />
|
||||
Changes requested
|
||||
</span>
|
||||
) : null}
|
||||
{canApprove ? (
|
||||
<Button
|
||||
className="h-8 gap-1.5 bg-green-600 px-3.5 text-white shadow-sm hover:bg-green-700"
|
||||
disabled={reviewDecisionPending}
|
||||
onClick={() => {
|
||||
void handleApprove();
|
||||
}}
|
||||
size="xs"
|
||||
type="button"
|
||||
>
|
||||
<Check className="h-3.5 w-3.5" />
|
||||
Approve
|
||||
</Button>
|
||||
) : null}
|
||||
{canRequestChanges ? (
|
||||
<Button
|
||||
className="h-8 gap-1.5 px-3.5 text-destructive hover:text-destructive"
|
||||
disabled={reviewDecisionPending}
|
||||
onClick={() => {
|
||||
void handleRequestChanges();
|
||||
}}
|
||||
size="xs"
|
||||
type="button"
|
||||
variant="outline"
|
||||
>
|
||||
<X className="h-3.5 w-3.5" />
|
||||
Request changes
|
||||
</Button>
|
||||
) : null}
|
||||
{canMerge ? (
|
||||
<MergePullRequestButton
|
||||
project={project}
|
||||
pullRequest={pullRequest}
|
||||
/>
|
||||
) : null}
|
||||
{canChangeStatus && isDraft ? (
|
||||
<Button
|
||||
className="h-7 gap-1.5 px-3"
|
||||
disabled={isUpdatingStatus}
|
||||
onClick={() => {
|
||||
void handleStatusChange("open");
|
||||
}}
|
||||
size="xs"
|
||||
type="button"
|
||||
variant="secondary"
|
||||
>
|
||||
<GitPullRequest className="h-3.5 w-3.5" />
|
||||
Ready for review
|
||||
</Button>
|
||||
) : null}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
{showDraftControl ? (
|
||||
<p className="px-1 text-xs text-muted-foreground">
|
||||
Still in progress?{" "}
|
||||
<button
|
||||
className="font-medium underline-offset-2 hover:text-foreground hover:underline disabled:opacity-50"
|
||||
disabled={isUpdatingStatus}
|
||||
onClick={() => {
|
||||
void handleStatusChange("draft");
|
||||
}}
|
||||
type="button"
|
||||
>
|
||||
Convert to draft
|
||||
</button>
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Check, Search, UserPlus } from "lucide-react";
|
||||
import { Check, Search, UserPlus, X } from "lucide-react";
|
||||
import * as React from "react";
|
||||
import { toast } from "sonner";
|
||||
|
||||
@@ -87,6 +87,11 @@ export function PullRequestReviewersRow({
|
||||
const approvedBy = new Set(
|
||||
pullRequest.approvals.map((approval) => normalizePubkey(approval.author)),
|
||||
);
|
||||
const changesRequestedBy = new Set(
|
||||
pullRequest.changeRequests.map((request) =>
|
||||
normalizePubkey(request.author),
|
||||
),
|
||||
);
|
||||
|
||||
const handleRequest = React.useCallback(
|
||||
async (pubkey: string, reviewerLabel: string) => {
|
||||
@@ -127,6 +132,9 @@ export function PullRequestReviewersRow({
|
||||
const profile = profileForPubkey(pubkey, profiles);
|
||||
const label = labelForPubkey(pubkey, profiles);
|
||||
const hasApproved = approvedBy.has(normalizePubkey(pubkey));
|
||||
const hasRequestedChanges = changesRequestedBy.has(
|
||||
normalizePubkey(pubkey),
|
||||
);
|
||||
return (
|
||||
<Tooltip key={pubkey}>
|
||||
<TooltipTrigger asChild>
|
||||
@@ -141,12 +149,20 @@ export function PullRequestReviewersRow({
|
||||
<span className="-right-1 -bottom-1 absolute flex h-3.5 w-3.5 items-center justify-center rounded-full bg-green-600 text-white ring-2 ring-background">
|
||||
<Check className="h-2.5 w-2.5" />
|
||||
</span>
|
||||
) : hasRequestedChanges ? (
|
||||
<span className="-right-1 -bottom-1 absolute flex h-3.5 w-3.5 items-center justify-center rounded-full bg-destructive text-destructive-foreground ring-2 ring-background">
|
||||
<X className="h-2.5 w-2.5" />
|
||||
</span>
|
||||
) : null}
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
{label}
|
||||
{hasApproved ? " — approved" : " — review requested"}
|
||||
{hasApproved
|
||||
? " — approved"
|
||||
: hasRequestedChanges
|
||||
? " — requested changes"
|
||||
: " — review requested"}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
|
||||
@@ -934,6 +934,7 @@ declare global {
|
||||
) => RawFeedItem;
|
||||
__BUZZ_E2E_SIGNED_EVENTS__?: Array<{
|
||||
content: string;
|
||||
createdAt?: number;
|
||||
kind: number;
|
||||
tags: string[][];
|
||||
}>;
|
||||
@@ -10138,6 +10139,7 @@ export function maybeInstallE2eTauriMocks() {
|
||||
case "sign_event":
|
||||
window.__BUZZ_E2E_SIGNED_EVENTS__?.push({
|
||||
content: (payload as { content: string }).content,
|
||||
createdAt: (payload as { createdAt?: number }).createdAt,
|
||||
kind: (payload as { kind: number }).kind,
|
||||
tags: (payload as { tags: string[][] }).tags,
|
||||
});
|
||||
|
||||
@@ -54,13 +54,18 @@ test("PR creator/owner can toggle draft, request reviews, and approve", async ({
|
||||
const header = page.getByRole("heading", { level: 3 });
|
||||
await expect(header.first()).toBeVisible();
|
||||
|
||||
// Owner viewing an open PR: draft toggle + approve are both offered.
|
||||
// Owner viewing an open PR: draft toggle and both review decisions are offered.
|
||||
const convertToDraft = page.getByRole("button", {
|
||||
name: "Convert to draft",
|
||||
});
|
||||
const approve = page.getByRole("button", { name: "Approve", exact: true });
|
||||
const requestChanges = page.getByRole("button", {
|
||||
name: "Request changes",
|
||||
exact: true,
|
||||
});
|
||||
await expect(convertToDraft).toBeVisible();
|
||||
await expect(approve).toBeVisible();
|
||||
await expect(requestChanges).toBeVisible();
|
||||
|
||||
// Request a review from bob via the reviewers dropdown.
|
||||
await page.getByRole("button", { name: "Request", exact: true }).click();
|
||||
@@ -97,8 +102,50 @@ test("PR creator/owner can toggle draft, request reviews, and approve", async ({
|
||||
path: `${SHOTS}/01-review-requested.png`,
|
||||
});
|
||||
|
||||
// Approve the PR: header flips to the approved chip and the discussion
|
||||
// gains a compact approval timeline row.
|
||||
// Fire opposite decisions in the same event turn. The first choice wins;
|
||||
// the shared synchronous guard must prevent the approval from publishing.
|
||||
await requestChanges.evaluate((requestChangesButton) => {
|
||||
const approveButton = [...document.querySelectorAll("button")].find(
|
||||
(button) => button.textContent?.trim() === "Approve",
|
||||
);
|
||||
requestChangesButton.click();
|
||||
approveButton?.click();
|
||||
});
|
||||
await expect(page.getByText("Changes requested.")).toBeVisible();
|
||||
await expect(
|
||||
page.getByText("requested changes", { exact: true }),
|
||||
).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(requestChanges).toHaveCount(0);
|
||||
const changeRequestEvent = await page.evaluate(() =>
|
||||
window.__BUZZ_E2E_SIGNED_EVENTS__
|
||||
?.filter(
|
||||
(event) =>
|
||||
event.kind === 1 &&
|
||||
event.tags.some(
|
||||
(tag) => tag[0] === "t" && tag[1] === "changes-requested",
|
||||
),
|
||||
)
|
||||
.at(-1),
|
||||
);
|
||||
expect(changeRequestEvent?.tags).toContainEqual(["c", expect.any(String)]);
|
||||
const rapidDecisionEvents = await page.evaluate(
|
||||
() =>
|
||||
window.__BUZZ_E2E_SIGNED_EVENTS__?.filter(
|
||||
(event) =>
|
||||
event.kind === 1 &&
|
||||
event.tags.some(
|
||||
(tag) =>
|
||||
tag[0] === "t" &&
|
||||
(tag[1] === "approval" || tag[1] === "changes-requested"),
|
||||
),
|
||||
) ?? [],
|
||||
);
|
||||
expect(rapidDecisionEvents).toHaveLength(1);
|
||||
|
||||
// Replace the completed change request with an approval. Both decisions
|
||||
// remain tied to the current commit and their timestamps preserve order.
|
||||
await approve.click();
|
||||
await expect(page.getByText("Pull request approved.")).toBeVisible();
|
||||
await expect(page.getByText("approved these changes")).toBeVisible({
|
||||
@@ -107,6 +154,19 @@ test("PR creator/owner can toggle draft, request reviews, and approve", async ({
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Approve", exact: true }),
|
||||
).toHaveCount(0);
|
||||
const approvalEvent = await page.evaluate(() =>
|
||||
window.__BUZZ_E2E_SIGNED_EVENTS__
|
||||
?.filter(
|
||||
(event) =>
|
||||
event.kind === 1 &&
|
||||
event.tags.some((tag) => tag[0] === "t" && tag[1] === "approval"),
|
||||
)
|
||||
.at(-1),
|
||||
);
|
||||
expect(approvalEvent?.tags).toContainEqual(["c", expect.any(String)]);
|
||||
expect(approvalEvent?.createdAt).toBeGreaterThan(
|
||||
changeRequestEvent?.createdAt ?? 0,
|
||||
);
|
||||
|
||||
await waitForAnimations(page);
|
||||
await page.screenshot({
|
||||
|
||||
Reference in New Issue
Block a user