add webhook id to test webhook #7884
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdded optional webhook_id to TestWebhookPayload in the insight webhooks API and included this field when invoking testWebhook from the useTestWebhook hook. No changes to request path, response handling, or control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
nischitpra
added
the
merge-queue
Graphite App
Merge activity
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7884 +/- ##
=======================================
Coverage 56.54% 56.54%
=======================================
Files 904 904
Lines 58592 58592
Branches 4143 4143
=======================================
Hits 33131 33131
Misses 25355 25355
Partials 106 106
🚀 New features to boost your workflow:
|
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.ts (1)
1-1: Mark this hooks module as a Client ModuleThis file uses React hooks (
useState) and toasts; it should start with the client directive to satisfy Next’s client boundary rules.Apply:
+"use client"; import { useState } from "react";
🧹 Nitpick comments (3)
apps/dashboard/src/@/api/insight/webhooks.ts (1)
60-64: Model the payload as an exclusive union (id XOR url) to prevent ambiguous requestsIf the API expects exactly one of
webhook_idorwebhook_url, enforce that at the type level. This also matches the guidelines to prefer type aliases for shapes like this.If the backend allows both fields simultaneously, ignore this. Otherwise, consider:
-interface TestWebhookPayload { - webhook_id?: string; - webhook_url: string; - type?: "event" | "transaction"; -} +type TestWebhookPayload = + { type?: "event" | "transaction" } & ( + | { webhook_id: string; webhook_url?: never } + | { webhook_url: string; webhook_id?: never } + );Optionally document precedence (if both are sent) in a short JSDoc so callers don’t guess behavior.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.ts (2)
44-46: Avoid sending both webhook_id and webhook_url; build the payload conditionallyPrevents ambiguous server behavior and avoids sending empty IDs. Also trims whitespace-only IDs.
If the API truly supports both fields at once, feel free to skip. Otherwise:
- const result = await testWebhook( - { type, webhook_url: webhookUrl, webhook_id: id }, - clientId, - ); + const payload = + typeof id === "string" && id.trim().length > 0 + ? { type, webhook_id: id.trim() } + : { type, webhook_url: webhookUrl }; + const result = await testWebhook(payload, clientId);If we adopt the exclusive-union type proposed in webhooks.ts, this also gives compile-time safety here.
71-75: Surface server error details in the toast to aid debuggingIf the API returns an error string, include it for better UX.
- toast.error("Failed to send test event", { - description: - "The server reported a failure in sending the test event.", - }); + toast.error("Failed to send test event", { + description: + result.error ?? "The server reported a failure in sending the test event.", + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/dashboard/src/@/api/insight/webhooks.ts(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.tsapps/dashboard/src/@/api/insight/webhooks.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.tsapps/dashboard/src/@/api/insight/webhooks.ts
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/webhooks/hooks/useTestWebhook.tsapps/dashboard/src/@/api/insight/webhooks.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/dashboard/src/@/api/insight/webhooks.ts (1)
60-64: LGTM: optional webhook_id on TestWebhookPayload is non‑breakingAdding
webhook_id?: stringis backward-compatible and aligns with the new usage. No runtime or control-flow risk here.
<!-- start pr-codex -->
## PR-Codex overview
This PR introduces a new optional property `webhook_id` to the `TestWebhookPayload` interface and updates the call to `testWebhook` to include this new property.
### Detailed summary
- Added an optional property `webhook_id` to the `TestWebhookPayload` interface in `webhooks.ts`.
- Updated the `testWebhook` function call in `useTestWebhook.ts` to include `webhook_id` when invoking the function.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Test Webhook requests now include the specific webhook ID when available, ensuring the test targets the correct webhook and improving reliability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
np/test_webhook
branch
from
71854ca to
b9e63ff
Compare
graphite-app
bot
removed
the
merge-queue
PR-Codex overview
This PR introduces an optional
webhook_idproperty to theTestWebhookPayloadinterface and updates thetestWebhookfunction call to include this new property.Detailed summary
webhook_idproperty to theTestWebhookPayloadinterface inwebhooks.ts.testWebhookfunction call inuseTestWebhook.tsto includewebhook_idwhen calling the function.Summary by CodeRabbit