Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions app/api/admins/coding-agent/slack-tags/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { type NextRequest, NextResponse } from "next/server";
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
import { getSlackTagsHandler } from "@/lib/admins/coding-agent/getSlackTagsHandler";

/**
* GET /api/admins/coding-agent/slack-tags
*
* Returns Slack tagging analytics for the Recoup Coding Agent bot.
* Pulls directly from the Slack API as the source of truth.
* Supports period filtering: all (default), daily, weekly, monthly.
* Requires admin authentication.
*/
export async function GET(request: NextRequest): Promise<NextResponse> {
return getSlackTagsHandler(request);
}

export async function OPTIONS(): Promise<NextResponse> {
return new NextResponse(null, { status: 204, headers: getCorsHeaders() });
Comment on lines +13 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Please add endpoint coverage before merging.

I don't see tests in this change set for the success path, invalid period, auth rejection, Slack upstream failure, or the OPTIONS preflight. This endpoint is mostly orchestration, so a small route/handler test matrix will catch regressions quickly.

Based on learnings: "Write tests for new API endpoints covering all success and error paths."

If helpful, I can sketch the minimal test matrix for the route/handler split.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/admins/coding-agent/slack-tags/route.ts` around lines 13 - 18, The PR
lacks automated tests for the new Slack tags endpoint in route.ts (GET ->
getSlackTagsHandler and OPTIONS preflight); add a small test matrix that covers:
successful GET (mock auth to return a valid user and mock the Slack upstream to
return tags, assert JSON and status 200), invalid period parameter (call GET
with bad period, assert 400), auth rejection (mock auth to fail, assert 401/403
as implemented), Slack upstream failure (mock the upstream call used by
getSlackTagsHandler to return an error/timeout and assert the handler returns
the expected error/status), and the OPTIONS preflight (call OPTIONS and assert
status 204 and CORS headers via getCorsHeaders()). Use unit tests on
getSlackTagsHandler for orchestration logic and a lightweight route-level test
for OPTIONS; mock external deps (auth, Slack client, getCorsHeaders) and keep
tests minimal and focused.

Comment on lines +17 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add JSDoc for OPTIONS.

Line 17 is an exported route handler and currently has no docblock.

Based on learnings: "All API routes should have JSDoc comments."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/admins/coding-agent/slack-tags/route.ts` around lines 17 - 18, Add a
JSDoc block for the exported route handler OPTIONS explaining its purpose
(responds to preflight CORS requests), mention it returns a 204 No Content with
CORS headers, and annotate the return type (Promise<NextResponse>); place the
comment immediately above the export async function OPTIONS() declaration and
reference getCorsHeaders() in the description to clarify where headers come
from.

}
112 changes: 112 additions & 0 deletions lib/admins/coding-agent/__tests__/getSlackTagsHandler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { NextRequest, NextResponse } from "next/server";
import { getSlackTagsHandler } from "../getSlackTagsHandler";
import { validateGetSlackTagsQuery } from "../validateGetSlackTagsQuery";
import { fetchSlackMentions } from "../fetchSlackMentions";

vi.mock("@/lib/networking/getCorsHeaders", () => ({
getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })),
}));

vi.mock("../validateGetSlackTagsQuery", () => ({
validateGetSlackTagsQuery: vi.fn(),
}));

vi.mock("../fetchSlackMentions", () => ({
fetchSlackMentions: vi.fn(),
}));

const mockTags = [
{
user_id: "U012AB3CD",
user_name: "Jane Smith",
user_avatar: "https://avatars.slack-edge.com/jane.jpg",
prompt: "add dark mode support",
timestamp: "2024-01-15T10:30:00.000Z",
channel_id: "C012AB3CD",
channel_name: "dev-team",
},
{
user_id: "U098ZY7WX",
user_name: "Bob Lee",
user_avatar: null,
prompt: "fix login bug",
timestamp: "2024-01-14T08:00:00.000Z",
channel_id: "C012AB3CD",
channel_name: "dev-team",
},
];

function makeRequest(period = "all") {
return new NextRequest(
`https://example.com/api/admins/coding-agent/slack-tags?period=${period}`,
);
}

describe("getSlackTagsHandler", () => {
beforeEach(() => {
vi.clearAllMocks();
});

describe("successful cases", () => {
it("returns 200 with tags and total", async () => {
vi.mocked(validateGetSlackTagsQuery).mockResolvedValue({ period: "all" });
vi.mocked(fetchSlackMentions).mockResolvedValue(mockTags);

const response = await getSlackTagsHandler(makeRequest());
const body = await response.json();

expect(response.status).toBe(200);
expect(body.status).toBe("success");
expect(body.total).toBe(2);
expect(body.tags).toEqual(mockTags);
});

it("returns 200 with empty tags when no mentions found", async () => {
vi.mocked(validateGetSlackTagsQuery).mockResolvedValue({ period: "daily" });
vi.mocked(fetchSlackMentions).mockResolvedValue([]);

const response = await getSlackTagsHandler(makeRequest("daily"));
const body = await response.json();

expect(response.status).toBe(200);
expect(body.status).toBe("success");
expect(body.total).toBe(0);
expect(body.tags).toEqual([]);
});

it("passes the period to fetchSlackMentions", async () => {
vi.mocked(validateGetSlackTagsQuery).mockResolvedValue({ period: "weekly" });
vi.mocked(fetchSlackMentions).mockResolvedValue([]);

await getSlackTagsHandler(makeRequest("weekly"));

expect(fetchSlackMentions).toHaveBeenCalledWith("weekly");
});
});

describe("error cases", () => {
it("returns auth error when validateGetSlackTagsQuery returns NextResponse", async () => {
const errorResponse = NextResponse.json(
{ status: "error", message: "Forbidden" },
{ status: 403 },
);
vi.mocked(validateGetSlackTagsQuery).mockResolvedValue(errorResponse);

const response = await getSlackTagsHandler(makeRequest());

expect(response.status).toBe(403);
});

it("returns 500 when fetchSlackMentions throws", async () => {
vi.mocked(validateGetSlackTagsQuery).mockResolvedValue({ period: "all" });
vi.mocked(fetchSlackMentions).mockRejectedValue(new Error("Slack API error"));

const response = await getSlackTagsHandler(makeRequest());
const body = await response.json();

expect(response.status).toBe(500);
expect(body.status).toBe("error");
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { NextRequest, NextResponse } from "next/server";
import { validateGetSlackTagsQuery } from "../validateGetSlackTagsQuery";
import { validateAdminAuth } from "@/lib/admins/validateAdminAuth";

vi.mock("@/lib/networking/getCorsHeaders", () => ({
getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })),
}));

vi.mock("@/lib/admins/validateAdminAuth", () => ({
validateAdminAuth: vi.fn(),
}));

const mockAuth = { accountId: "test-account", orgId: null, authToken: "token" };

function makeRequest(period?: string) {
const url = period
? `https://example.com/api/admins/coding-agent/slack-tags?period=${period}`
: "https://example.com/api/admins/coding-agent/slack-tags";
return new NextRequest(url);
}

describe("validateGetSlackTagsQuery", () => {
beforeEach(() => {
vi.clearAllMocks();
});

describe("successful cases", () => {
it("returns all period by default", async () => {
vi.mocked(validateAdminAuth).mockResolvedValue(mockAuth);

const result = await validateGetSlackTagsQuery(makeRequest());

expect(result).toEqual({ period: "all" });
});

it("returns daily when period=daily", async () => {
vi.mocked(validateAdminAuth).mockResolvedValue(mockAuth);

const result = await validateGetSlackTagsQuery(makeRequest("daily"));

expect(result).toEqual({ period: "daily" });
});

it("returns weekly when period=weekly", async () => {
vi.mocked(validateAdminAuth).mockResolvedValue(mockAuth);

const result = await validateGetSlackTagsQuery(makeRequest("weekly"));

expect(result).toEqual({ period: "weekly" });
});

it("returns monthly when period=monthly", async () => {
vi.mocked(validateAdminAuth).mockResolvedValue(mockAuth);

const result = await validateGetSlackTagsQuery(makeRequest("monthly"));

expect(result).toEqual({ period: "monthly" });
});
});

describe("error cases", () => {
it("returns auth error when validateAdminAuth returns NextResponse", async () => {
const errorResponse = NextResponse.json(
{ status: "error", message: "Unauthorized" },
{ status: 401 },
);
vi.mocked(validateAdminAuth).mockResolvedValue(errorResponse);

const result = await validateGetSlackTagsQuery(makeRequest());

expect(result).toBeInstanceOf(NextResponse);
const body = await (result as NextResponse).json();
expect(body.status).toBe("error");
});

it("returns 400 for invalid period value", async () => {
vi.mocked(validateAdminAuth).mockResolvedValue(mockAuth);

const result = await validateGetSlackTagsQuery(makeRequest("yearly"));

expect(result).toBeInstanceOf(NextResponse);
const body = await (result as NextResponse).json();
expect(body.status).toBe("error");
});
});
});
Loading
Loading