Skip to content

Commit 6b9d1d7

Browse files
betegonclaude
betegon
and
claude
committed
fix(issue-list): address PR review feedback for project ID resolution
Remove fetchProjectId from shared resolveAllTargets/resolveOrgAndProject (Point 1), enrich missing projectId in issue list's auto-detect path so env-var and config-default targets also get numeric IDs (Point 2), fix toNumericId(0) returning undefined via explicit Number.isFinite check (Point 3), and update tests accordingly (Point 4). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 045f554 commit 6b9d1d7

File tree

4 files changed

+81
-41
lines changed

4 files changed

+81
-41
lines changed

‎src/commands/issue/list.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,19 @@ async function resolveTargetsFromParsedArg(
284284
cwd: string
285285
): Promise<TargetResolutionResult> {
286286
switch (parsed.type) {
287-
case "auto-detect":
287+
case "auto-detect": {
288288
// Use existing resolution logic (DSN detection, config defaults)
289-
return resolveAllTargets({ cwd, usageHint: USAGE_HINT });
289+
const result = await resolveAllTargets({ cwd, usageHint: USAGE_HINT });
290+
// Enrich targets missing projectId (env var / config default paths)
291+
await Promise.all(
292+
result.targets.map(async (target) => {
293+
if (!target.projectId) {
294+
target.projectId = await fetchProjectId(target.org, target.project);
295+
}
296+
})
297+
);
298+
return result;
299+
}
290300

291301
case "explicit": {
292302
// Single explicit target — fetch project ID for API query param

‎src/lib/resolve-target.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,15 @@ import {
4747
import { warning } from "./formatters/colors.js";
4848
import { isAllDigits } from "./utils.js";
4949

50-
/** Convert a string or numeric ID to a number, or `undefined` if falsy/NaN. */
50+
/** Convert a string or numeric ID to a positive number, or `undefined` if invalid. */
5151
export function toNumericId(
5252
id: string | number | undefined
5353
): number | undefined {
54-
if (id === null) {
54+
if (id === undefined || id === null) {
5555
return;
5656
}
57-
return Number(id) || undefined;
57+
const n = Number(id);
58+
return Number.isFinite(n) && n > 0 ? n : undefined;
5859
}
5960

6061
/**
@@ -600,13 +601,11 @@ export async function resolveAllTargets(
600601

601602
// 1. CLI flags take priority (both must be provided together)
602603
if (org && project) {
603-
const projectId = await fetchProjectId(org, project);
604604
return {
605605
targets: [
606606
{
607607
org,
608608
project,
609-
projectId,
610609
orgDisplay: org,
611610
projectDisplay: project,
612611
},
@@ -726,11 +725,9 @@ export async function resolveOrgAndProject(
726725

727726
// 1. CLI flags take priority (both must be provided together)
728727
if (org && project) {
729-
const projectId = await fetchProjectId(org, project);
730728
return {
731729
org,
732730
project,
733-
projectId,
734731
orgDisplay: org,
735732
projectDisplay: project,
736733
};

‎test/commands/issue/list.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ describe("issue list: error propagation", () => {
114114
// listIssues hits: /api/0/organizations/test-org/issues/?query=project:test-project
115115
globalThis.fetch = mockFetch(async (input, init) => {
116116
const req = new Request(input, init);
117+
// fetchProjectId enrichment for config-defaults path
118+
if (req.url.includes("/api/0/projects/test-org/test-project/")) {
119+
return Response.json({
120+
id: "789",
121+
slug: "test-project",
122+
name: "Test Project",
123+
});
124+
}
117125
if (req.url.includes("/issues/")) {
118126
return new Response(
119127
JSON.stringify({ detail: "Invalid query: unknown field" }),
@@ -140,6 +148,14 @@ describe("issue list: error propagation", () => {
140148
test("throws ApiError with 404 status when project not found", async () => {
141149
globalThis.fetch = mockFetch(async (input, init) => {
142150
const req = new Request(input, init);
151+
// fetchProjectId enrichment for config-defaults path
152+
if (req.url.includes("/api/0/projects/test-org/test-project/")) {
153+
return Response.json({
154+
id: "789",
155+
slug: "test-project",
156+
name: "Test Project",
157+
});
158+
}
143159
if (req.url.includes("/issues/")) {
144160
return new Response(JSON.stringify({ detail: "Project not found" }), {
145161
status: 404,
@@ -165,6 +181,14 @@ describe("issue list: error propagation", () => {
165181
test("throws ApiError with 429 status on rate limiting", async () => {
166182
globalThis.fetch = mockFetch(async (input, init) => {
167183
const req = new Request(input, init);
184+
// fetchProjectId enrichment for config-defaults path
185+
if (req.url.includes("/api/0/projects/test-org/test-project/")) {
186+
return Response.json({
187+
id: "789",
188+
slug: "test-project",
189+
name: "Test Project",
190+
});
191+
}
168192
if (req.url.includes("/issues/")) {
169193
return new Response(JSON.stringify({ detail: "Too many requests" }), {
170194
status: 429,
@@ -190,6 +214,14 @@ describe("issue list: error propagation", () => {
190214
test("preserves ApiError detail from original error", async () => {
191215
globalThis.fetch = mockFetch(async (input, init) => {
192216
const req = new Request(input, init);
217+
// fetchProjectId enrichment for config-defaults path
218+
if (req.url.includes("/api/0/projects/test-org/test-project/")) {
219+
return Response.json({
220+
id: "789",
221+
slug: "test-project",
222+
name: "Test Project",
223+
});
224+
}
193225
if (req.url.includes("/issues/")) {
194226
return new Response(
195227
JSON.stringify({ detail: "Invalid search query: bad syntax" }),
@@ -373,6 +405,14 @@ describe("issue list: partial failure handling", () => {
373405
test("JSON output wraps in {data, hasMore} object", async () => {
374406
globalThis.fetch = mockFetch(async (input, init) => {
375407
const req = new Request(input, init);
408+
// fetchProjectId enrichment for config-defaults path
409+
if (req.url.includes("/api/0/projects/test-org/test-project/")) {
410+
return Response.json({
411+
id: "789",
412+
slug: "test-project",
413+
name: "Test Project",
414+
});
415+
}
376416
if (req.url.includes("/issues/")) {
377417
return new Response(JSON.stringify([mockIssue()]), {
378418
status: 200,
@@ -844,6 +884,15 @@ describe("issue list: compound cursor resume", () => {
844884
const req = new Request(input, init);
845885
const url = req.url;
846886

887+
// fetchProjectId enrichment for explicit target
888+
if (url.includes("/api/0/projects/test-org/proj-a/")) {
889+
return Response.json({
890+
id: "100",
891+
slug: "proj-a",
892+
name: "Proj A",
893+
});
894+
}
895+
847896
// listIssues for proj-a: resumed from cursor
848897
if (url.includes("/organizations/test-org/issues/")) {
849898
const cursor = new URL(url).searchParams.get("cursor");

‎test/lib/resolve-target.test.ts‎

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,17 @@ describe("toNumericId", () => {
143143
test("returns undefined for non-numeric string", () => {
144144
expect(toNumericId("abc")).toBeUndefined();
145145
});
146+
147+
test("returns undefined for negative numbers", () => {
148+
expect(toNumericId(-1)).toBeUndefined();
149+
expect(toNumericId("-5")).toBeUndefined();
150+
});
151+
152+
test("returns undefined for Infinity", () => {
153+
expect(toNumericId(Number.POSITIVE_INFINITY)).toBeUndefined();
154+
expect(toNumericId(Number.NEGATIVE_INFINITY)).toBeUndefined();
155+
expect(toNumericId("Infinity")).toBeUndefined();
156+
});
146157
});
147158

148159
// ============================================================================
@@ -222,29 +233,15 @@ describe("Environment variable resolution (SENTRY_ORG / SENTRY_PROJECT)", () =>
222233
process.env.SENTRY_ORG = "env-org";
223234
process.env.SENTRY_PROJECT = "env-project";
224235

225-
await setAuthToken("test-token");
226-
await setOrgRegion("flag-org", DEFAULT_SENTRY_URL);
227-
globalThis.fetch = mockFetch(async (input, init) => {
228-
const req = new Request(input, init);
229-
if (req.url.includes("/api/0/projects/flag-org/flag-project/")) {
230-
return Response.json({
231-
id: "123",
232-
slug: "flag-project",
233-
name: "Flag Project",
234-
});
235-
}
236-
return new Response(JSON.stringify({ detail: "Not found" }), {
237-
status: 404,
238-
});
239-
});
240-
241236
const result = await resolveOrgAndProject({
242237
org: "flag-org",
243238
project: "flag-project",
244239
cwd: "/tmp",
245240
});
246241
expect(result?.org).toBe("flag-org");
247242
expect(result?.project).toBe("flag-project");
243+
// Explicit path no longer fetches projectId
244+
expect(result?.projectId).toBeUndefined();
248245
});
249246

250247
test("resolveOrgAndProject: ignores empty/whitespace-only values", async () => {
@@ -304,28 +301,15 @@ describe("Environment variable resolution (SENTRY_ORG / SENTRY_PROJECT)", () =>
304301
process.env.SENTRY_ORG = "env-org";
305302
process.env.SENTRY_PROJECT = "env-project";
306303

307-
await setAuthToken("test-token");
308-
await setOrgRegion("flag-org", DEFAULT_SENTRY_URL);
309-
globalThis.fetch = mockFetch(async (input, init) => {
310-
const req = new Request(input, init);
311-
if (req.url.includes("/api/0/projects/flag-org/flag-project/")) {
312-
return Response.json({
313-
id: "123",
314-
slug: "flag-project",
315-
name: "Flag Project",
316-
});
317-
}
318-
return new Response(JSON.stringify({ detail: "Not found" }), {
319-
status: 404,
320-
});
321-
});
322-
323304
const result = await resolveAllTargets({
324305
org: "flag-org",
325306
project: "flag-project",
326307
cwd: "/tmp",
327308
});
328309
expect(result.targets[0]?.org).toBe("flag-org");
310+
expect(result.targets[0]?.project).toBe("flag-project");
311+
// Explicit path no longer fetches projectId
312+
expect(result.targets[0]?.projectId).toBeUndefined();
329313
});
330314

331315
// --- resolveOrgsForListing ---

0 commit comments

Comments
 (0)