feat(api): implement GET /bounties/:id bounty detail#92
feat(api): implement GET /bounties/:id bounty detail#92automaton365-sys wants to merge 1 commit intodevasignhq:mainfrom
Conversation
🟡 AI Code Review ResultsStatus: Review Recommended 🟡 Merge Score: 80/100🟡 Recommendation: The PR correctly implements the feature with good tests. However, it contains a notable performance inefficiency in how it counts applications. A suggested optimization using a database 💡 Code Suggestions (1)🟡 Medium Priority (1)
💭 Reasoning: Using a dedicated SQL Suggested Code: const [[{ count: applicationCount }], creator, assignee] = await Promise.all([
db.select({ count: sql<number>`cast(count(*) as int)` }).from(applications).where(eq(applications.bountyId, bounty.id)),
db.query.users.findFirst({
where: eq(users.id, bounty.creatorId),
columns: { username: true, avatarUrl: true },
}),
bounty.assigneeId
? db.query.users.findFirst({
where: eq(users.id, bounty.assigneeId),
columns: { username: true, avatarUrl: true },
})
: Promise.resolve(null),
]);
return c.json({
...bounty,
application_count: applicationCount,
creator: creator
? {
username: creator.username,
avatar_url: creator.avatarUrl,
}
: null,
assignee: assignee
? {
username: assignee.username,
avatar_url: assignee.avatarUrl,
}
: null,
});📊 Review Metadata
|
Implements bounty detail endpoint enhancements requested in #21.\n\n### What changed\n- Updated to return:\n - full bounty details\n - info (, )\n - \n - info if assigned (, )\n- Added tests in for:\n - successful response shape\n - 404 when bounty is missing\n\n### Notes\n- Followed existing route/test style and DB query conventions.\n\nFixes #21\n/claim #21