fix(FN-825): handle 204 No Content in api() and remove unused styles/tests
- Fix api() helper to properly handle 204 No Content responses from mission delete and reorder mutations - Add regression tests for mission edit/delete 204 handling in e2e tests - Add unit tests for api() helper covering null/empty/204 response scenarios - Remove unused CSS styles (175 lines of dead code) - Remove unrelated test code from ChangedFilesModal, TaskCard, and mobile-scroll-snap tests - Clean up ChangedFilesModal and TaskCard components
This commit is contained in:
@@ -2372,3 +2372,129 @@ describe("ExecutorState type", () => {
|
||||
expect(states).toContain("paused");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Regression: Mission mutation 204 response handling ─────────────────────
|
||||
//
|
||||
// Mission DELETE and reorder endpoints return 204 No Content. The api()
|
||||
// function must handle these responses correctly instead of throwing
|
||||
// a misleading content-type error.
|
||||
describe("Mission mutation coverage with 204 responses", () => {
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
it("returns undefined for void responses (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { deleteMission } = await import("./api");
|
||||
const result = await deleteMission("M-LZ7DN0-A2B5");
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for milestone delete (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { deleteMilestone } = await import("./api");
|
||||
const result = await deleteMilestone("MS-M3N8QR-C9F1");
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for slice delete (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { deleteSlice } = await import("./api");
|
||||
const result = await deleteSlice("SL-P4T2WX-D5E8");
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for feature delete (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { deleteFeature } = await import("./api");
|
||||
const result = await deleteFeature("F-J6K9AB-G7H3");
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for milestone reorder (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { reorderMilestones } = await import("./api");
|
||||
const result = await reorderMilestones("M-LZ7DN0-A2B5", ["MS-1", "MS-2"]);
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for slice reorder (204 No Content)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { reorderSlices } = await import("./api");
|
||||
const result = await reorderSlices("MS-M3N8QR-C9F1", ["SL-1", "SL-2"]);
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("handles 204 with projectId query param", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue({
|
||||
ok: true,
|
||||
status: 204,
|
||||
headers: new Headers(),
|
||||
text: () => Promise.resolve(""),
|
||||
});
|
||||
|
||||
const { deleteMission } = await import("./api");
|
||||
const result = await deleteMission("M-LZ7DN0-A2B5", "my-project");
|
||||
expect(result).toBeUndefined();
|
||||
expect(globalThis.fetch).toHaveBeenCalledWith(
|
||||
expect.stringContaining("/api/missions/M-LZ7DN0-A2B5?projectId=my-project"),
|
||||
expect.objectContaining({ method: "DELETE" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("still throws on JSON error responses (non-204)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(
|
||||
mockFetchResponse(false, { error: "Mission not found" }, 404)
|
||||
);
|
||||
|
||||
const { deleteMission } = await import("./api");
|
||||
await expect(deleteMission("M-999")).rejects.toThrow("Mission not found");
|
||||
});
|
||||
|
||||
it("still throws on invalid ID format (400)", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(
|
||||
mockFetchResponse(false, { error: "Invalid mission ID format" }, 400)
|
||||
);
|
||||
|
||||
const { deleteMission } = await import("./api");
|
||||
await expect(deleteMission("bad-id")).rejects.toThrow("Invalid mission ID format");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,6 +38,16 @@ async function api<T = unknown>(path: string, opts: RequestInit = {}): Promise<T
|
||||
...opts,
|
||||
});
|
||||
|
||||
// Handle successful 204 No Content responses (e.g., DELETE, reorder)
|
||||
// These return no body and no JSON content-type — return undefined for void endpoints
|
||||
if (res.status === 204) {
|
||||
if (!res.ok) {
|
||||
// 204 is always ok by definition, but guard anyway
|
||||
throw new Error(`Request failed for ${url}: ${res.status} ${res.statusText}`);
|
||||
}
|
||||
return undefined as T;
|
||||
}
|
||||
|
||||
const contentType = res.headers.get("content-type") ?? "";
|
||||
const bodyText = await res.text();
|
||||
const isJson = contentType.includes("application/json");
|
||||
|
||||
@@ -34,10 +34,11 @@ function createMockMissionStore() {
|
||||
|
||||
// Generate IDs matching the real MissionStore format:
|
||||
// prefix + base36(timestamp) + "-" + random alphanumeric suffix
|
||||
const generateMissionId = () => `M-${missionCounter++}`;
|
||||
const generateMilestoneId = () => `MS-${milestoneCounter++}`;
|
||||
const generateSliceId = () => `SL-${sliceCounter++}`;
|
||||
const generateFeatureId = () => `F-${featureCounter++}`;
|
||||
// e.g., M-MNJVKT2G-ME5Q, MS-M3N8QR-C9F1, SL-P4T2WX-D5E8, F-J6K9AB-G7H3
|
||||
const generateMissionId = () => `M-MOCK${missionCounter++.toString(36).toUpperCase()}-TST`;
|
||||
const generateMilestoneId = () => `MS-MOCK${milestoneCounter++.toString(36).toUpperCase()}-TST`;
|
||||
const generateSliceId = () => `SL-MOCK${sliceCounter++.toString(36).toUpperCase()}-TST`;
|
||||
const generateFeatureId = () => `F-MOCK${featureCounter++.toString(36).toUpperCase()}-TST`;
|
||||
|
||||
return {
|
||||
createMission: vi.fn((input: { title: string; description?: string }) => {
|
||||
@@ -321,12 +322,37 @@ describe("Mission API", () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe("active");
|
||||
expect(res.body.autoAdvance).toBe(true);
|
||||
expect(res.body.id).toBe(mission.id);
|
||||
// Verify the update was actually persisted in the store (FN-825 regression)
|
||||
const updated = missionStore.getMission(mission.id);
|
||||
expect(updated?.status).toBe("active");
|
||||
expect(updated?.autoAdvance).toBe(true);
|
||||
expect(missionStore.updateMission).toHaveBeenCalledWith(mission.id, {
|
||||
status: "active",
|
||||
autoAdvance: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("should update mission title with generated-format ID", async () => {
|
||||
const { app, missionStore } = buildApp();
|
||||
const mission = missionStore.createMission({ title: "Original Title" });
|
||||
|
||||
const res = await request(
|
||||
app,
|
||||
"PATCH",
|
||||
`/api/missions/${mission.id}`,
|
||||
JSON.stringify({ title: "Updated Title" }),
|
||||
{ "content-type": "application/json" }
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.title).toBe("Updated Title");
|
||||
expect(res.body.id).toBe(mission.id);
|
||||
// Verify persistence
|
||||
const updated = missionStore.getMission(mission.id);
|
||||
expect(updated?.title).toBe("Updated Title");
|
||||
});
|
||||
|
||||
it("should reject non-boolean auto-advance values", async () => {
|
||||
const { app, missionStore } = buildApp();
|
||||
const mission = missionStore.createMission({ title: "Test Mission" });
|
||||
@@ -350,13 +376,27 @@ describe("Mission API", () => {
|
||||
});
|
||||
|
||||
describe("DELETE /api/missions/:missionId", () => {
|
||||
it("should delete mission", async () => {
|
||||
it("should delete mission and confirm removal from store", async () => {
|
||||
const { app, missionStore } = buildApp();
|
||||
const mission = missionStore.createMission({ title: "To Delete" });
|
||||
|
||||
const res = await request(app, "DELETE", `/api/missions/${mission.id}`);
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
// Verify the mission is actually removed from the mock store (FN-825 regression)
|
||||
expect(missionStore.getMission(mission.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should delete mission with generated-format ID and confirm removal", async () => {
|
||||
const { app, missionStore } = buildApp();
|
||||
const mission = missionStore.createMission({ title: "To Delete" });
|
||||
// Generated-format IDs from mock look like M-MOCK1-TST
|
||||
expect(mission.id).toMatch(/^M-[A-Z0-9]+/);
|
||||
|
||||
const res = await request(app, "DELETE", `/api/missions/${mission.id}`);
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
expect(missionStore.getMission(mission.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should return 404 for non-existent mission", async () => {
|
||||
@@ -365,14 +405,25 @@ describe("Mission API", () => {
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("should cascade delete all children", async () => {
|
||||
it("should reject invalid mission ID format on DELETE", async () => {
|
||||
const { app } = buildApp();
|
||||
const res = await request(app, "DELETE", `/api/missions/invalid-id`);
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("should cascade delete all children and verify removal", async () => {
|
||||
const { app, missionStore } = buildApp();
|
||||
const mission = missionStore.createMission({ title: "To Delete" });
|
||||
missionStore.addMilestone(mission.id, { title: "Milestone 1" });
|
||||
const milestone = missionStore.addMilestone(mission.id, { title: "Milestone 1" });
|
||||
|
||||
await request(app, "DELETE", `/api/missions/${mission.id}`);
|
||||
const res = await request(app, "DELETE", `/api/missions/${mission.id}`);
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
expect(missionStore.getMission(mission.id)).toBeUndefined();
|
||||
// Note: The mock store's deleteMission only removes from the mission Map.
|
||||
// In the real store, FK cascades would remove milestones too.
|
||||
// We verify the route returned success — cascade behavior is tested at the store level.
|
||||
expect(missionStore.deleteMission).toHaveBeenCalledWith(mission.id);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user