Default comment reopen to checked

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
dotta
2026-03-20 15:46:01 -05:00
parent cff06c9a54
commit be911754c5
4 changed files with 169 additions and 18 deletions

View File

@@ -65,6 +65,7 @@ export type CreateIssueLabel = z.infer<typeof createIssueLabelSchema>;
export const updateIssueSchema = createIssueSchema.partial().extend({
comment: z.string().min(1).optional(),
reopen: z.boolean().optional(),
hiddenAt: z.string().datetime().nullable().optional(),
});

View File

@@ -0,0 +1,143 @@
import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { issueRoutes } from "../routes/issues.js";
import { errorHandler } from "../middleware/index.js";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
findMentionedAgents: vi.fn(),
}));
const mockAccessService = vi.hoisted(() => ({
canUser: vi.fn(),
hasPermission: vi.fn(),
}));
const mockHeartbeatService = vi.hoisted(() => ({
wakeup: vi.fn(async () => undefined),
reportRunActivity: vi.fn(async () => undefined),
}));
const mockAgentService = vi.hoisted(() => ({
getById: vi.fn(),
}));
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
vi.mock("../services/index.js", () => ({
accessService: () => mockAccessService,
agentService: () => mockAgentService,
documentService: () => ({}),
executionWorkspaceService: () => ({}),
goalService: () => ({}),
heartbeatService: () => mockHeartbeatService,
issueApprovalService: () => ({}),
issueService: () => mockIssueService,
logActivity: mockLogActivity,
projectService: () => ({}),
workProductService: () => ({}),
}));
function createApp() {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
};
next();
});
app.use("/api", issueRoutes({} as any, {} as any));
app.use(errorHandler);
return app;
}
function makeIssue(status: "todo" | "done") {
return {
id: "11111111-1111-4111-8111-111111111111",
companyId: "company-1",
status,
assigneeAgentId: "22222222-2222-4222-8222-222222222222",
assigneeUserId: null,
createdByUserId: "local-board",
identifier: "PAP-580",
title: "Comment reopen default",
};
}
describe("issue comment reopen routes", () => {
beforeEach(() => {
vi.clearAllMocks();
mockIssueService.addComment.mockResolvedValue({
id: "comment-1",
issueId: "11111111-1111-4111-8111-111111111111",
companyId: "company-1",
body: "hello",
createdAt: new Date(),
updatedAt: new Date(),
authorAgentId: null,
authorUserId: "local-board",
});
mockIssueService.findMentionedAgents.mockResolvedValue([]);
});
it("treats reopen=true as a no-op when the issue is already open", async () => {
mockIssueService.getById.mockResolvedValue(makeIssue("todo"));
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
...makeIssue("todo"),
...patch,
}));
const res = await request(createApp())
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" });
expect(res.status).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111", {
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
});
expect(mockLogActivity).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
action: "issue.updated",
details: expect.not.objectContaining({ reopened: true }),
}),
);
});
it("reopens closed issues via the PATCH comment path", async () => {
mockIssueService.getById.mockResolvedValue(makeIssue("done"));
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
...makeIssue("done"),
...patch,
}));
const res = await request(createApp())
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" });
expect(res.status).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111", {
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
status: "todo",
});
expect(mockLogActivity).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
action: "issue.updated",
details: expect.objectContaining({
reopened: true,
reopenedFrom: "done",
status: "todo",
}),
}),
);
});
});

View File

@@ -824,10 +824,14 @@ export function issueRoutes(db: Db, storage: StorageService) {
if (!(await assertAgentRunCheckoutOwnership(req, res, existing))) return;
const actor = getActorInfo(req);
const { comment: commentBody, hiddenAt: hiddenAtRaw, ...updateFields } = req.body;
const isClosed = existing.status === "done" || existing.status === "cancelled";
const { comment: commentBody, reopen: reopenRequested, hiddenAt: hiddenAtRaw, ...updateFields } = req.body;
if (hiddenAtRaw !== undefined) {
updateFields.hiddenAt = hiddenAtRaw ? new Date(hiddenAtRaw) : null;
}
if (commentBody && reopenRequested === true && isClosed && updateFields.status === undefined) {
updateFields.status = "todo";
}
let issue;
try {
issue = await svc.update(id, updateFields);
@@ -875,6 +879,13 @@ export function issueRoutes(db: Db, storage: StorageService) {
}
const hasFieldChanges = Object.keys(previous).length > 0;
const reopened =
commentBody &&
reopenRequested === true &&
isClosed &&
previous.status !== undefined &&
issue.status === "todo";
const reopenFromStatus = reopened ? existing.status : null;
await logActivity(db, {
companyId: issue.companyId,
actorType: actor.actorType,
@@ -888,6 +899,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
...updateFields,
identifier: issue.identifier,
...(commentBody ? { source: "comment" } : {}),
...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus } : {}),
_previous: hasFieldChanges ? previous : undefined,
},
});
@@ -913,6 +925,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
bodySnippet: comment.body.slice(0, 120),
identifier: issue.identifier,
issueTitle: issue.title,
...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus, source: "comment" } : {}),
...(hasFieldChanges ? { updated: true } : {}),
},
});

View File

@@ -50,7 +50,6 @@ interface CommentThreadProps {
mentions?: MentionOption[];
}
const CLOSED_STATUSES = new Set(["done", "cancelled"]);
const DRAFT_DEBOUNCE_MS = 800;
function loadDraft(draftKey: string): string {
@@ -261,7 +260,6 @@ export function CommentThread({
companyId,
projectId,
onAdd,
issueStatus,
agentMap,
imageUploadHandler,
onAttachImage,
@@ -286,8 +284,6 @@ export function CommentThread({
const location = useLocation();
const hasScrolledRef = useRef(false);
const isClosed = issueStatus ? CLOSED_STATUSES.has(issueStatus) : false;
const timeline = useMemo<TimelineItem[]>(() => {
const commentItems: TimelineItem[] = comments.map((comment) => ({
kind: "comment",
@@ -369,10 +365,10 @@ export function CommentThread({
setSubmitting(true);
try {
await onAdd(trimmed, isClosed && reopen ? true : undefined, reassignment ?? undefined);
await onAdd(trimmed, reopen ? true : undefined, reassignment ?? undefined);
setBody("");
if (draftKey) clearDraft(draftKey);
setReopen(false);
setReopen(true);
setReassignTarget(effectiveSuggestedAssigneeValue);
} finally {
setSubmitting(false);
@@ -439,17 +435,15 @@ export function CommentThread({
</Button>
</div>
)}
{isClosed && (
<label className="flex items-center gap-1.5 text-xs text-muted-foreground cursor-pointer select-none">
<input
type="checkbox"
checked={reopen}
onChange={(e) => setReopen(e.target.checked)}
className="rounded border-border"
/>
Re-open
</label>
)}
<label className="flex items-center gap-1.5 text-xs text-muted-foreground cursor-pointer select-none">
<input
type="checkbox"
checked={reopen}
onChange={(e) => setReopen(e.target.checked)}
className="rounded border-border"
/>
Re-open
</label>
{enableReassign && reassignOptions.length > 0 && (
<InlineEntitySelector
value={reassignTarget}