forked from farhoodlabs/paperclip
fix(issues): default list endpoint to include blockedBy graph (GRO-2096)
GET /api/companies/{companyId}/issues now returns the blockedBy graph by
default, matching GET /api/issues/{id}. The hygiene scanner was producing
false-unblock cascades on GRO-1178 because the blocked filter on the list
endpoint returned no blockedBy data, so the scanner never saw that GRO-1472
was still blocking the task.
Opt out with ?includeBlockedBy=false (or 0) for perf-sensitive callers that
do not need the dependency graph. The service layer default remains opt-in
to keep services/issues.test.ts:1419 stable.
Cross-repo PR: farhoodlabs/paperclip#<pending>
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -0,0 +1,167 @@
|
|||||||
|
import express from "express";
|
||||||
|
import request from "supertest";
|
||||||
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
|
const mockList = vi.hoisted(() => vi.fn());
|
||||||
|
const mockIssueService = vi.hoisted(() => ({
|
||||||
|
list: mockList,
|
||||||
|
getById: vi.fn(),
|
||||||
|
getByIdentifier: vi.fn(async () => null),
|
||||||
|
getComment: vi.fn(),
|
||||||
|
getCommentCursor: vi.fn(async () => ({
|
||||||
|
totalComments: 0,
|
||||||
|
latestCommentId: null,
|
||||||
|
latestCommentAt: null,
|
||||||
|
})),
|
||||||
|
getRelationSummaries: vi.fn(),
|
||||||
|
update: vi.fn(),
|
||||||
|
getAncestors: vi.fn(async () => []),
|
||||||
|
listWakeableBlockedDependents: vi.fn(async () => []),
|
||||||
|
getWakeableParentAfterChildCompletion: vi.fn(async () => null),
|
||||||
|
findMentionedAgents: vi.fn(async () => []),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../services/index.js", async () => {
|
||||||
|
const actual = await vi.importActual<typeof import("../services/index.js")>(
|
||||||
|
"../services/index.js",
|
||||||
|
);
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
companyService: () => ({
|
||||||
|
getById: vi.fn(async () => ({ id: "company-1", attachmentMaxBytes: 10 * 1024 * 1024 })),
|
||||||
|
}),
|
||||||
|
accessService: () => ({
|
||||||
|
canUser: vi.fn(),
|
||||||
|
hasPermission: vi.fn(),
|
||||||
|
}),
|
||||||
|
agentService: () => ({
|
||||||
|
getById: vi.fn(),
|
||||||
|
}),
|
||||||
|
documentAnnotationService: () => ({ remapOpenThreadsForDocument: async () => [] }),
|
||||||
|
documentService: () => ({
|
||||||
|
getIssueDocumentPayload: vi.fn(async () => ({})),
|
||||||
|
}),
|
||||||
|
executionWorkspaceService: () => ({
|
||||||
|
getById: vi.fn(),
|
||||||
|
}),
|
||||||
|
feedbackService: () => ({}),
|
||||||
|
goalService: () => ({
|
||||||
|
getById: vi.fn(),
|
||||||
|
getDefaultCompanyGoal: vi.fn(),
|
||||||
|
}),
|
||||||
|
heartbeatService: () => ({
|
||||||
|
wakeup: vi.fn(async () => undefined),
|
||||||
|
reportRunActivity: vi.fn(async () => undefined),
|
||||||
|
}),
|
||||||
|
instanceSettingsService: () => ({
|
||||||
|
get: vi.fn(),
|
||||||
|
listCompanyIds: vi.fn(),
|
||||||
|
}),
|
||||||
|
issueApprovalService: () => ({}),
|
||||||
|
issueReferenceService: () => ({
|
||||||
|
deleteDocumentSource: async () => undefined,
|
||||||
|
diffIssueReferenceSummary: () => ({
|
||||||
|
addedReferencedIssues: [],
|
||||||
|
removedReferencedIssues: [],
|
||||||
|
currentReferencedIssues: [],
|
||||||
|
}),
|
||||||
|
emptySummary: () => ({ outbound: [], inbound: [] }),
|
||||||
|
listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }),
|
||||||
|
syncComment: async () => undefined,
|
||||||
|
syncDocument: async () => undefined,
|
||||||
|
syncIssue: async () => undefined,
|
||||||
|
}),
|
||||||
|
issueRecoveryActionService: () => ({
|
||||||
|
getActiveForIssue: vi.fn(async () => null),
|
||||||
|
listActiveForIssues: vi.fn(async () => new Map()),
|
||||||
|
}),
|
||||||
|
issueThreadInteractionService: () => ({
|
||||||
|
listForIssue: vi.fn(async () => []),
|
||||||
|
expireRequestConfirmationsSupersededByComment: vi.fn(async () => []),
|
||||||
|
expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []),
|
||||||
|
}),
|
||||||
|
issueService: () => mockIssueService,
|
||||||
|
projectService: () => ({
|
||||||
|
getById: vi.fn(),
|
||||||
|
listByIds: vi.fn(async () => []),
|
||||||
|
}),
|
||||||
|
routineService: () => ({
|
||||||
|
syncRunStatusForIssue: vi.fn(async () => undefined),
|
||||||
|
}),
|
||||||
|
workProductService: () => ({
|
||||||
|
listForIssue: vi.fn(async () => []),
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
async function createApp() {
|
||||||
|
const [{ issueRoutes }, { errorHandler }] = await Promise.all([
|
||||||
|
vi.importActual<typeof import("../routes/issues.js")>("../routes/issues.js"),
|
||||||
|
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
|
||||||
|
]);
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use((req, _res, next) => {
|
||||||
|
(req as any).actor = {
|
||||||
|
type: "board",
|
||||||
|
userId: "local-board",
|
||||||
|
companyIds: ["company-1"],
|
||||||
|
memberships: [{ companyId: "company-1", membershipRole: "owner", status: "active" }],
|
||||||
|
source: "local_implicit",
|
||||||
|
isInstanceAdmin: false,
|
||||||
|
};
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
app.use("/api", issueRoutes({} as any, {} as any));
|
||||||
|
app.use(errorHandler);
|
||||||
|
return app;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("GET /companies/:companyId/issues includeBlockedBy default", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetModules();
|
||||||
|
vi.doUnmock("../routes/issues.js");
|
||||||
|
vi.doUnmock("../routes/authz.js");
|
||||||
|
vi.doUnmock("../middleware/index.js");
|
||||||
|
vi.clearAllMocks();
|
||||||
|
mockList.mockResolvedValue([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("defaults includeBlockedBy to true so list responses are consistent with GET /api/issues/:id", async () => {
|
||||||
|
const res = await request(await createApp()).get("/api/companies/company-1/issues");
|
||||||
|
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||||
|
expect(mockList).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockList.mock.calls[0]?.[1] ?? {};
|
||||||
|
expect(callArgs).toMatchObject({ includeBlockedBy: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("defaults includeBlockedBy to true when the status filter is blocked (GRO-2096 regression guard)", async () => {
|
||||||
|
const res = await request(await createApp())
|
||||||
|
.get("/api/companies/company-1/issues")
|
||||||
|
.query({ status: "blocked" });
|
||||||
|
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||||
|
expect(mockList).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockList.mock.calls[0]?.[1] ?? {};
|
||||||
|
expect(callArgs).toMatchObject({ status: "blocked", includeBlockedBy: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("opts out of includeBlockedBy when the caller passes ?includeBlockedBy=false", async () => {
|
||||||
|
const res = await request(await createApp())
|
||||||
|
.get("/api/companies/company-1/issues")
|
||||||
|
.query({ includeBlockedBy: "false" });
|
||||||
|
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||||
|
expect(mockList).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockList.mock.calls[0]?.[1] ?? {};
|
||||||
|
expect(callArgs).toMatchObject({ includeBlockedBy: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("opts out of includeBlockedBy when the caller passes ?includeBlockedBy=0", async () => {
|
||||||
|
const res = await request(await createApp())
|
||||||
|
.get("/api/companies/company-1/issues")
|
||||||
|
.query({ includeBlockedBy: "0" });
|
||||||
|
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||||
|
expect(mockList).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockList.mock.calls[0]?.[1] ?? {};
|
||||||
|
expect(callArgs).toMatchObject({ includeBlockedBy: false });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1944,7 +1944,9 @@ export function issueRoutes(
|
|||||||
req.query.excludeRoutineExecutions === "true" || req.query.excludeRoutineExecutions === "1",
|
req.query.excludeRoutineExecutions === "true" || req.query.excludeRoutineExecutions === "1",
|
||||||
includePluginOperations:
|
includePluginOperations:
|
||||||
req.query.includePluginOperations === "true" || req.query.includePluginOperations === "1",
|
req.query.includePluginOperations === "true" || req.query.includePluginOperations === "1",
|
||||||
includeBlockedBy: req.query.includeBlockedBy === "true" || req.query.includeBlockedBy === "1",
|
// Default to including blockedBy so list responses are consistent with GET /api/issues/:id.
|
||||||
|
// Opt out with ?includeBlockedBy=false (or 0) for perf-sensitive callers that don't need the graph.
|
||||||
|
includeBlockedBy: req.query.includeBlockedBy !== "false" && req.query.includeBlockedBy !== "0",
|
||||||
includeBlockedInboxAttention:
|
includeBlockedInboxAttention:
|
||||||
req.query.includeBlockedInboxAttention === "true" || req.query.includeBlockedInboxAttention === "1",
|
req.query.includeBlockedInboxAttention === "true" || req.query.includeBlockedInboxAttention === "1",
|
||||||
q: req.query.q as string | undefined,
|
q: req.query.q as string | undefined,
|
||||||
|
|||||||
Reference in New Issue
Block a user