-
Notifications
You must be signed in to change notification settings - Fork 1
Webhook APIs and supporting documentation #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a41ee44
5558f74
ed640c2
4eed3ab
8761430
0c7f12d
f318bfa
2fcfced
e74f341
1938a55
776bc4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { Router } from "express"; | ||
| import type { Request, Response } from "express"; | ||
| import { getWebhookAttemptsByOrganizationIdAndWebhookId } from "../../../controllers/WebhookAttemptController"; | ||
|
|
||
| const router = Router({ mergeParams: true }); | ||
|
|
||
| router.get( | ||
| "/", | ||
| async ( | ||
| req: Request< | ||
| { organizationId: string; webhookId: string } | ||
| >, | ||
| res: Response | ||
| ) => { | ||
| const webhookAttempts = await getWebhookAttemptsByOrganizationIdAndWebhookId({ | ||
| organizationId: req.organization._id.toString(), | ||
| webhookId: req.params.webhookId, | ||
| }); | ||
|
|
||
| return res.json(webhookAttempts); | ||
| } | ||
| ); | ||
|
|
||
| export default router; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { Router } from "express"; | ||
| import type { Request, Response } from "express"; | ||
| import { body } from "express-validator"; | ||
| import { expressValidatorMiddleware } from "../../../middlewares/expressValidatorMiddleware"; | ||
| import { | ||
| createWebhook, | ||
| deleteWebhookByOrganizationIdAndId, | ||
| getWebhookById, | ||
| getWebhookByOrganizationIdAndId, | ||
| getWebhooksByOrganizationId, | ||
| } from "../../../controllers/WebhookController"; | ||
| import { WebhookEvents } from "../../../models/Webhook"; | ||
| import { sendWebhookEvent } from "../../../controllers/WebhookAttemptController"; | ||
| import attemptsRouter from "./attempts"; | ||
|
|
||
| const router = Router({ mergeParams: true }); | ||
|
|
||
| router.get( | ||
| "/", | ||
| async (req: Request<{ organizationId: string }, {}, {}>, res: Response) => { | ||
| const webhooks = await getWebhooksByOrganizationId({ | ||
| organizationId: req.organization._id.toString(), | ||
| }); | ||
| return res.json(webhooks); | ||
| } | ||
| ); | ||
|
|
||
| router.post( | ||
| "/", | ||
| body("url").isString().trim(), | ||
| body("events").isArray().notEmpty(), | ||
| expressValidatorMiddleware, | ||
| async ( | ||
| req: Request< | ||
| { organizationId: string }, | ||
| {}, | ||
| { url: string; events: (typeof WebhookEvents)[number][] } | ||
| >, | ||
| res: Response | ||
| ) => { | ||
| const webhook = await createWebhook({ | ||
| organizationId: req.organization._id.toString(), | ||
| url: req.body.url, | ||
| events: req.body.events, | ||
| }); | ||
| return res.json(webhook); | ||
| } | ||
| ); | ||
|
Comment on lines
+28
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tighten test route targeting and creation semantics for webhooks A few points in this router worth adjusting:
Items (2) and (3) are nice-to-have refactors; item (1) is the important behavioral fix. Also applies to: 49-88, 90-109 |
||
|
|
||
| router.get( | ||
| "/:webhookId", | ||
| async ( | ||
| req: Request<{ organizationId: string; webhookId: string }, {}, {}>, | ||
| res: Response | ||
| ) => { | ||
| const webhook = await getWebhookByOrganizationIdAndId({ | ||
| organizationId: req.organization._id.toString(), | ||
| id: req.params.webhookId, | ||
| }); | ||
| if (!webhook) { | ||
| return res.status(404).json({ error: "Webhook not found" }); | ||
| } | ||
| return res.json(webhook); | ||
| } | ||
| ); | ||
|
|
||
| router.post( | ||
| "/:webhookId/test", | ||
| async ( | ||
| req: Request<{ organizationId: string; webhookId: string }, {}, {}>, | ||
| res: Response | ||
| ) => { | ||
| const webhook = await getWebhookByOrganizationIdAndId({ | ||
| organizationId: req.organization._id.toString(), | ||
| id: req.params.webhookId, | ||
| }); | ||
| if (!webhook) { | ||
| return res.status(404).json({ error: "Webhook not found" }); | ||
| } | ||
| await sendWebhookEvent({ | ||
| organizationId: req.organization._id.toString(), | ||
| event: "message.received", | ||
| payload: { | ||
| test: "test", | ||
| } | ||
| }); | ||
| return res.json({ success: true }); | ||
| } | ||
| ); | ||
|
|
||
| router.delete( | ||
| "/:webhookId", | ||
| async ( | ||
| req: Request<{ organizationId: string; webhookId: string }, {}, {}>, | ||
| res: Response | ||
| ) => { | ||
| const webhook = await getWebhookByOrganizationIdAndId({ | ||
| organizationId: req.organization._id.toString(), | ||
| id: req.params.webhookId, | ||
| }); | ||
| if (!webhook) { | ||
| return res.status(404).json({ error: "Webhook not found" }); | ||
| } | ||
| await deleteWebhookByOrganizationIdAndId({ | ||
| organizationId: req.organization._id.toString(), | ||
| id: req.params.webhookId, | ||
| }); | ||
| return res.json(webhook); | ||
| } | ||
| ); | ||
|
|
||
| router.use("/:webhookId/attempts", attemptsRouter); | ||
|
|
||
| export default router; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling and validation for the route handler.
The async handler lacks error handling, which could result in unhandled promise rejections. Additionally,
req.organizationis accessed without type checking, andwebhookIdis not validated before querying the database.Apply this diff to add proper error handling:
router.get( "/", async ( req: Request< { organizationId: string; webhookId: string } >, res: Response ) => { + try { + if (!req.organization?._id) { + return res.status(401).json({ error: 'Unauthorized' }); + } + + if (!req.params.webhookId) { + return res.status(400).json({ error: 'Webhook ID is required' }); + } + const webhookAttempts = await getWebhookAttemptsByOrganizationIdAndWebhookId({ organizationId: req.organization._id.toString(), webhookId: req.params.webhookId, }); return res.json(webhookAttempts); + } catch (error) { + console.error('Failed to fetch webhook attempts:', error); + return res.status(500).json({ + error: 'Failed to fetch webhook attempts', + message: error instanceof Error ? error.message : 'Unknown error' + }); + } } );📝 Committable suggestion
🤖 Prompt for AI Agents