-
Notifications
You must be signed in to change notification settings - Fork 535
Add webhooks feature to Insight dashboard #6929
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6929 +/- ##
=======================================
Coverage 55.81% 55.81%
=======================================
Files 900 900
Lines 57848 57848
Branches 4067 4067
=======================================
Hits 32288 32288
Misses 25454 25454
Partials 106 106
🚀 New features to boost your workflow:
|
size-limit report 📦
|
8887def
to
4b39e26
Compare
...p/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/create-webhook-modal.tsx
Outdated
Show resolved
Hide resolved
...p/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/create-webhook-modal.tsx
Outdated
Show resolved
Hide resolved
4b39e26
to
b083bcc
Compare
b083bcc
to
36fcc86
Compare
.refine((val) => val.split(",").every(isValidAddress), { | ||
message: "Enter a valid address", | ||
}), |
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.
The current address validation has a potential issue with empty inputs. The .refine()
method will pass validation for strings containing only commas or spaces (like ",,,") because it's checking if every address is valid, but not verifying that valid addresses actually exist.
Consider enhancing the validation to ensure there's at least one valid address after splitting and filtering:
.refine(
(val) => {
const addresses = val.split(',').map(addr => addr.trim()).filter(Boolean);
return addresses.length > 0 && addresses.every(isValidAddress);
},
{ message: 'Enter at least one valid address' }
)
This approach first trims and filters out empty entries, then verifies both that there's at least one address and that all addresses are valid.
.refine((val) => val.split(",").every(isValidAddress), { | |
message: "Enter a valid address", | |
}), | |
.refine( | |
(val) => { | |
const addresses = val.split(",").map(addr => addr.trim()).filter(Boolean); | |
return addresses.length > 0 && addresses.every(isValidAddress); | |
}, | |
{ message: "Enter at least one valid address" } | |
), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
@@ -165,7 +165,9 @@ export const MultiSelect = forwardRef<HTMLButtonElement, MultiSelectProps>( | |||
{props.customTrigger || ( | |||
<Button | |||
ref={ref} | |||
{...props} | |||
{...(() => { |
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.
Why is this added?
|
||
{/* Navigation Buttons */} | ||
<div className="flex justify-between pt-4"> | ||
{currentStep !== WebhookFormSteps.BasicInfo ? ( |
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.
The navigation buttons should also go inside the step component so this is easier to maintain and update later
render={({ field }) => ( | ||
<FormItem className="flex flex-col"> | ||
<div | ||
className={cn( |
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.
Remove cn
here - not required, please also remove this from everywhere else as well.
only use it when conditional styles need to be added or when the className string is too large and you want to break it down to multiple lines
})); | ||
}, [pairs, isOpen, addresses, chainIds, type, thirdwebClient]); | ||
|
||
const results = useQueries({ queries }); |
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.
Avoid using useQueries
- If the number of queries can be large here it will crash the page due to a large amount of memory used. If possible, please do all request in a single useQuery
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.
hm this does not make sense to me, one useQuery would be LESS efficient in this case no?
* @param address Address to validate | ||
* @returns Boolean indicating if address is valid | ||
*/ | ||
export const isValidAddress = (address: string): boolean => { |
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.
We have a isAddress
utility exported from thirdweb/utils
fro this already
|
||
if (!response.ok) { | ||
const errorText = await response.text(); | ||
throw new Error(`Failed to delete webhook: ${errorText}`); |
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.
Don't throw errors in server actions - You won't be able to see them in client component (you will only be able to see errors in local dev server but not in vercel preview or production environments)
If you want to return error message, you have to return the error message manually
Example:
return {
errorMessage: await response.text()
}
6e90e96
to
5d068e0
Compare
toAddresses: z | ||
.string() | ||
.nonempty({ message: "To address is required" }) | ||
.refine((val) => val.split(",").every(isValidAddress), { | ||
message: "Enter a valid address", | ||
}), |
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.
The validation schema for toAddresses
currently requires this field unconditionally, but it should only be required when filterType
is set to 'transaction'. For event webhooks, this field isn't applicable. Consider implementing conditional validation using Zod's .superRefine()
or a dynamic schema based on the selected filter type to prevent validation errors when users are creating event webhooks. This would improve the form experience by only validating fields relevant to the current webhook type.
toAddresses: z | |
.string() | |
.nonempty({ message: "To address is required" }) | |
.refine((val) => val.split(",").every(isValidAddress), { | |
message: "Enter a valid address", | |
}), | |
toAddresses: z | |
.string() | |
.optional() | |
.superRefine((val, ctx) => { | |
// Only validate if filterType is 'transaction' | |
if (ctx.parent.filterType === 'transaction') { | |
if (!val || val.trim() === '') { | |
ctx.addIssue({ | |
code: z.ZodIssueCode.custom, | |
message: "To address is required for transaction webhooks", | |
}); | |
return; | |
} | |
if (!val.split(",").every(isValidAddress)) { | |
ctx.addIssue({ | |
code: z.ZodIssueCode.custom, | |
message: "Enter a valid address", | |
}); | |
} | |
} | |
}), |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
WalkthroughThese changes introduce a comprehensive webhook management feature to the dashboard application. New modules, components, hooks, and utilities enable users to create, review, test, and delete webhooks within a team/project context. The implementation includes multi-step modal forms, ABI processing, data validation, UI tables, and integration with the Thirdweb Insight API for backend operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebhooksPage
participant API_Module as Webhooks API Module
participant CreateModal as CreateWebhookModal
participant Table as WebhooksTable
User->>WebhooksPage: Visits /insight/webhooks
WebhooksPage->>API_Module: getWebhooks(clientId)
API_Module-->>WebhooksPage: List of webhooks
WebhooksPage->>User: Renders WebhooksTable or placeholder
User->>CreateModal: Opens Create Webhook Modal
CreateModal->>User: Guides through steps (Basic Info, Filter, Review)
User->>CreateModal: Submits form
CreateModal->>API_Module: createWebhook(payload, clientId)
API_Module-->>CreateModal: Webhook created
CreateModal-->>WebhooksPage: Close modal, refresh list
User->>Table: Clicks "Test" on a webhook
Table->>API_Module: testWebhook(payload, clientId)
API_Module-->>Table: Test result
Table->>User: Shows test result
User->>Table: Clicks "Delete" on a webhook
Table->>API_Module: deleteWebhook(webhookId, clientId)
API_Module-->>Table: Deletion confirmation
Table-->>WebhooksPage: Refresh list
sequenceDiagram
participant CreateModal
participant useAbiBatchProcessing
participant API_Module
CreateModal->>useAbiBatchProcessing: Fetch ABI for addresses/chains
useAbiBatchProcessing->>API_Module: Fetch ABI per contract
API_Module-->>useAbiBatchProcessing: ABI data
useAbiBatchProcessing-->>CreateModal: Extracted signatures
CreateModal->>User: Shows signature options
sequenceDiagram
participant CreateModal
participant useTestWebhook
participant API_Module
CreateModal->>useTestWebhook: testWebhookEndpoint(url, type)
useTestWebhook->>API_Module: testWebhook(payload, clientId)
API_Module-->>useTestWebhook: Test result
useTestWebhook-->>CreateModal: Result, triggers toast
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 8
♻️ Duplicate comments (6)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx (1)
3-19
: Organize imports according to the style guidelines.The imports should follow the Biome linter's organizeImports rule: React imports first, followed by third-party libraries, and then local imports. The TWTable import uses a relative path without '@/' prefix, which is inconsistent with other imports.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx (2)
32-34
: Implement a proper error state UI component.The error handling here only logs to the console without providing user feedback. Consider implementing a proper error state UI component that distinguishes between "no webhooks exist" and "failed to load webhooks." This would improve the user experience by clearly communicating when there's a system issue.
36-71
: 🛠️ Refactor suggestionClarify empty state vs. error state with better UI feedback.
The current UI handles the empty state when no webhooks exist, but it doesn't provide feedback when loading fails. Consider adding an error state UI component to differentiate between empty state and error state.
+ // Add state variable at the top of the component + let error: string | null = null; try { // existing code... } catch (err) { console.error("Error loading project or webhooks", err); + error = "Failed to load webhooks. Please try again later."; } return ( <div className="flex flex-col gap-8"> <div className="flex items-center justify-between"> <div> <h2 className="mb-1 font-bold text-2xl">Webhooks</h2> <p className="text-muted-foreground"> Create and manage webhooks to get notified about blockchain events, transactions and more.{" "} <TrackedUnderlineLink category="webhooks" label="learn-more" target="_blank" href="https://portal.thirdweb.com/insight/webhooks" > Learn more about webhooks. </TrackedUnderlineLink> </p> </div> - {webhooks.length > 0 && <CreateWebhookModal clientId={clientId} />} + {!error && webhooks.length > 0 && <CreateWebhookModal clientId={clientId} />} </div> + {error ? ( + <div className="flex flex-col items-center justify-center gap-4 rounded-lg border border-destructive p-12 text-center"> + <div> + <h3 className="mb-1 font-medium text-lg text-destructive">Error</h3> + <p className="text-muted-foreground">{error}</p> + </div> + <Button variant="outline" onClick={() => window.location.reload()}> + Try Again + </Button> + </div> + ) : {webhooks.length > 0 ? ( <WebhooksTable webhooks={webhooks} clientId={clientId} /> ) : ( <div className="flex flex-col items-center justify-center gap-4 rounded-lg border border-border p-12 text-center"> <div> <h3 className="mb-1 font-medium text-lg">No webhooks found</h3> <p className="text-muted-foreground"> Create a webhook to get started. </p> </div> <CreateWebhookModal clientId={clientId} /> </div> )} + } </div> );apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/abi-utils.ts (1)
26-29
: 🛠️ Refactor suggestionDuplicate utility – prefer
isAddress
fromthirdweb/utils
A custom
isValidAddress
helper is re-implemented here, but an officially-maintainedisAddress
validator already exists inthirdweb/utils
. Using the shared helper avoids duplicated logic and guarantees identical address-validation behaviour across the codebase.-import { keccak256, toFunctionSelector } from "thirdweb/utils"; +import { keccak256, toFunctionSelector, isAddress } from "thirdweb/utils"; ... -export const isValidAddress = (address: string): boolean => { - // Basic Ethereum address validation (0x followed by 40 hex characters) - return /^0x[a-fA-F0-9]{40}$/.test(address.trim()); -}; +// Re-export for local convenience if you need the same name elsewhere. +export const isValidAddress = isAddress;apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts (1)
19-24
:⚠️ Potential issueConditional-field validation is still missing
addresses
is always required andtoAddresses
is required even for “event” webhooks, causing form submission to fail when the hidden input is empty – exactly the issue flagged in previous reviews.Suggested fix (Zod discriminated union):
-import { z } from "zod"; +import { z } from "zod"; +import { isValidAddress } from "./abi-utils"; const addressList = z .string() .refine( (val) => val .split(",") .map((a) => a.trim()) .filter(Boolean) .every(isValidAddress), { message: "Enter at least one valid address" }, ); -export const webhookFormSchema = z.object({ … }); +export const webhookFormSchema = z.discriminatedUnion("filterType", [ + z.object({ + filterType: z.literal("event"), + addresses: addressList, + toAddresses: z.string().optional(), + fromAddresses: z.string().optional(), + …otherFields, + }), + z.object({ + filterType: z.literal("transaction"), + addresses: z.string().optional(), + toAddresses: addressList, + fromAddresses: z.string().optional(), + …otherFields, + }), +]);This enforces the correct field requirements and removes redundant
.nonempty()
calls on hidden inputs.Also applies to: 31-36
apps/dashboard/src/@/api/insight/webhooks.ts (1)
66-87
: 🛠️ Refactor suggestionDon't throw errors in server actions.
Server-side functions shouldn't throw errors as they won't be properly surfaced in client components in production environments (only visible in local development). Instead, return structured error responses.
As noted in a previous review comment, return error messages explicitly:
- if (!response.ok) { - const errorText = await response.text(); - throw new Error(`Failed to create webhook: ${errorText}`); - } - - return await response.json(); + if (!response.ok) { + return { + error: true, + errorMessage: await response.text() + }; + } + + const data = await response.json(); + return { + error: false, + data + };Apply this pattern to all API functions (
createWebhook
,getWebhooks
,deleteWebhook
, andtestWebhook
).Also applies to: 89-107, 109-131, 133-157
🧹 Nitpick comments (23)
apps/dashboard/src/@/components/blocks/multi-select.tsx (1)
168-170
: Simplify props spreading patternThe current IIFE pattern for props spreading is unnecessarily complex for creating a shallow copy of props.
Consider a more straightforward approach:
-{...(() => { - return { ...props }; -})()} +{...props}If the goal is to avoid DOM validation errors by filtering out specific props, a more explicit destructuring approach would be clearer:
-{...(() => { - return { ...props }; -})()} +{...omitCustomProps(props)}Where
omitCustomProps
could be a utility function that removes non-DOM props.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/layout.tsx (1)
6-34
: Layout component implementation looks goodThe async Layout component correctly fetches team and project data, handles error cases with appropriate redirects, and renders the InsightPageLayout with necessary props.
Small optimization: Consider destructuring the params directly in the function parameters for cleaner code:
export default async function Layout(props: { - params: Promise<{ team_slug: string; project_slug: string }>; + params: { team_slug: string; project_slug: string }; children: React.ReactNode; }) { - const { team_slug, project_slug } = await props.params; + const { team_slug, project_slug } = props.params;If params must remain a Promise, provide a clearer type annotation to indicate this.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/BasicInfoStep.tsx (1)
20-173
: Well-structured form component with clear UIThe BasicInfoStep component is well-implemented with proper form control integration, validation indicators, and good UX for the filter type selection.
Consider a few minor improvements:
- Add explicit type safety for the radio value casting:
onValueChange={(value: string) => { - const typedValue = value as "event" | "transaction"; + // Validate value is one of the allowed types + const typedValue = ["event", "transaction"].includes(value) + ? value as "event" | "transaction" + : "event"; // Default fallback field.onChange(typedValue);
- Enhance accessibility by adding ARIA attributes to the custom radio visuals:
<div className={cn( "flex h-5 w-5 items-center justify-center rounded-full border", field.value === "event" ? "border-primary" : "border-muted-foreground", )} + aria-hidden="true" >apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/InsightPageLayout.tsx (1)
66-108
: Consider extracting footer to a separate file for better code organization.The
InsightFooter
component is currently nested within the same file. Since this component has its own distinct responsibility of rendering a set of documentation links, it might be better to extract it to a separate file, especially if it grows in complexity or if the links need to be updated frequently.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/StepIndicator.tsx (1)
25-39
: Consider extracting step styling logic to a separate function for better readability.The complex conditional expression for determining step styling could be extracted to a helper function to improve readability and testability.
export default function StepIndicator({ currentStep }: StepIndicatorProps) { const steps = [ { id: WebhookFormSteps.BasicInfo, label: "Basic Info" }, { id: WebhookFormSteps.FilterDetails, label: "Configure Filters" }, { id: WebhookFormSteps.Review, label: "Review" }, ]; + const getStepStyles = (stepId: WebhookFormStep, index: number) => { + const currentStepIndex = steps.findIndex((s) => s.id === currentStep); + if (currentStep === stepId) { + return "bg-primary text-primary-foreground"; + } else if (index < currentStepIndex) { + return "bg-primary/80 text-primary-foreground"; + } else { + return "bg-muted text-muted-foreground"; + } + }; return ( <div className="mb-6 flex w-full items-center justify-center"> {steps.map((step, index) => ( <div key={step.id} className="flex items-center"> <div className={cn( "flex", "h-10", "w-10", "items-center", "justify-center", "rounded-full", "font-medium", "shrink-0", - currentStep === step.id - ? "bg-primary text-primary-foreground" - : index < steps.findIndex((s) => s.id === currentStep) - ? "bg-primary/80 text-primary-foreground" - : "bg-muted text-muted-foreground", + getStepStyles(step.id, index), )} >apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx (2)
79-86
: Improve robustness of event type extraction logic.The current implementation assumes specific filter keys exist but doesn't handle edge cases thoroughly. Consider adding more defensive programming to handle unexpected filter structures.
// Extract event type from filters - const eventType = filters["v1.events"] - ? "Event" - : filters["v1.transactions"] - ? "Transaction" - : "Unknown"; + let eventType = "Unknown"; + if (filters) { + if ("v1.events" in filters && filters["v1.events"]) { + eventType = "Event"; + } else if ("v1.transactions" in filters && filters["v1.transactions"]) { + eventType = "Transaction"; + } + }
181-185
: Use optional chaining for more concise date comparison.The date sorting is well-implemented but could be slightly more concise with optional chaining.
const sortedWebhooks = [...webhooks].sort((a, b) => { - const dateA = new Date(a.created_at).getTime(); - const dateB = new Date(b.created_at).getTime(); + const dateA = new Date(a?.created_at || 0).getTime(); + const dateB = new Date(b?.created_at || 0).getTime(); return dateB - dateA; });apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/CreateWebhookModal.tsx (1)
151-155
: Inconsistent address splitting between event & transaction filters
event
addresses are split on commas only, buttransaction
addresses use/[,\s]+/
, allowing whitespace. The UX becomes inconsistent:"0xA, 0xB" → valid for tx, invalid for event
Normalize with the same regex for both cases to avoid user confusion.
Also applies to: 176-182
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook.ts (2)
28-33
: Race-condition guard is fragile
isTestingMap
is read from the closure created in the previous render; a very fast successive click could still see the stale value asfalse
.Wrap
isTestingMap
in auseRef
or call the setter with a functional update to ensure atomicity, e.g.:-if (isTestingMap[uniqueId]) return false; +if (isTestingMap[uniqueId]) return false; +setIsTestingMap(prev => { + if (prev[uniqueId]) return prev; // double-check + return { ...prev, [uniqueId]: true }; +});This removes the tiny window where two calls may both decide testing is not in progress.
86-99
: Toast wording duplicates “Failed to send test event” detectionThe error discrimination searches the message string, but the API already returns
result.success
.
You can rely on the boolean and simplify the branch; string matching is brittle.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/ReviewStep.tsx (3)
167-174
: Simplify ABI presence checkUsing
Object.keys()
on a fabricated object is unnecessary and slightly obfuscates intent.- {Object.keys(form.watch("sigHashAbi") ? { dummy: true } : {}) - .length > 0 - ? "Provided" - : "None"} + {form.watch("sigHashAbi") ? "Provided" : "None"}
93-107
: Duplicate trimming / truncation logicThis inline IIFE duplicates the address-formatting code used elsewhere in the component.
Extract a small helper (formatAddresses(string | undefined)
) to improve readability and avoid future divergences.
33-38
: Multipleform.watch
calls per renderCalling
form.watch
inside render triggers a subscription for each key and causes this component to re-render on every keystroke—even when the watched field is unrelated to the area being typed in.Consider:
const { webhookUrl, filterType, ... } = form.getValues();or memoise watched values with
useMemo
to minimise re-renders.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/abi-utils.ts (2)
90-94
: String → bytes conversion should useTextEncoder
Uint8Array(canonicalSignature.split("").map(c => c.charCodeAt(0)))
works for ASCII, but fails for non-ASCII characters and is less performant.
A simpler and safer approach is:-const signatureBytes = new Uint8Array( - canonicalSignature.split("").map((c) => c.charCodeAt(0)), -); +const signatureBytes = new TextEncoder().encode(canonicalSignature);
46-60
: Potential duplicate entries in signatures arrayWhen multiple ABIs contain the same event/function, the current implementation will push duplicates, leading to repeated items in the dropdown.
Consider de-duplicating onsignature
to improve UX and avoid redundant renders.if (!signatures.some((s) => s.signature === hash)) { signatures.push({ … }); }Also applies to: 127-142
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/FilterDetailsStep.tsx (1)
24-24
:cn
utility is used with static class stringsEvery occurrence here concatenates only static class names;
cn
adds no value and costs bytes. Replacing with plain template strings improves clarity.Also applies to: 94-100
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useAbiProcessing.ts (4)
31-42
: Consider extracting address parsing logic to a reusable function.The address parsing logic (splitting by comma, trimming, and filtering) is repeated in multiple places in this file (lines 34-37, 66-69, 112-115). This could be extracted into a helper function to reduce duplication.
+ const parseAddresses = (addressesString: string) => { + return (addressesString || "") + .split(",") + .map((a) => a.trim()) + .filter(Boolean); + }; const pairs = useMemo(() => { const result: { chainId: string; address: string }[] = []; for (const chainId of chainIds) { - for (const address of (addresses || "") - .split(",") - .map((a) => a.trim()) - .filter(Boolean)) { + for (const address of parseAddresses(addresses)) { result.push({ chainId, address }); } } return result; }, [chainIds, addresses]);
48-49
: Explain the reason for the ESLint disable comment.Please add a brief comment explaining why the ESLint rule needs to be disabled here. This helps future developers understand the rationale and decide whether the rule can be re-enabled later.
64-107
: Simplify ABI processing logic.The nested conditional checks and loops make this code complex to follow. Consider breaking it down into smaller, clearly named helper functions to improve readability and maintainability.
const fetchedAbis = useMemo(() => { const abis: Record<string, AbiData> = {}; - for (const address of (addresses || "") - .split(",") - .map((a) => a.trim()) - .filter(Boolean)) { + for (const address of parseAddresses(addresses)) { if (!Array.isArray(results) || !Array.isArray(pairs)) continue; - const successful = results.length - ? results.filter( - (result, i) => - i < pairs.length && - pairs[i] && - pairs[i].address === address && - result.isSuccess && - result.data, - ) - : undefined; + const successful = getSuccessfulResultsForAddress(results, pairs, address); if ( successful && successful.length > 0 && successful[0] && successful[0].data !== undefined ) { const abi = successful[0].data; - let items: string[] = []; - if (Array.isArray(abi)) { - items = abi - .filter((item) => - type === "event" - ? item && item.type === "event" - : item && item.type === "function", - ) - .map((item) => item.name); - } + const items = extractItemNames(abi, type); abis[address] = { fetchedAt: new Date().toISOString(), status: "success", abi: abi, ...(type === "event" ? { events: items } : { functions: items }), }; } } return abis; }, [results, pairs, addresses, type]); +function getSuccessfulResultsForAddress(results, pairs, address) { + return results.length + ? results.filter( + (result, i) => + i < pairs.length && + pairs[i] && + pairs[i].address === address && + result.isSuccess && + result.data, + ) + : undefined; +} + +function extractItemNames(abi, type) { + let items: string[] = []; + if (Array.isArray(abi)) { + items = abi + .filter((item) => + type === "event" + ? item && item.type === "event" + : item && item.type === "function", + ) + .map((item) => item.name); + } + return items; +}
111-129
: Simplify isFetching calculation logic for better maintainability.The logic for determining if fetching is in progress is complex and hard to follow. Consider refactoring for clarity and to avoid potential bugs.
const isFetching = useMemo(() => { - const addressesList = (addresses || "") - .split(",") - .map((a) => a.trim()) - .filter(Boolean); + const addressesList = parseAddresses(addresses); return addressesList.some((address) => { const relevantResults = pairs .map((pair, i) => ({ pair, result: results[i] })) .filter(({ pair }) => pair.address === address); - const hasSuccess = relevantResults.some( - ({ result }) => result?.isSuccess && result.data, - ); - return ( - !hasSuccess && relevantResults.some(({ result }) => result?.isLoading) - ); + // If we have a successful result, we don't need to wait for others + const hasSuccess = relevantResults.some(({ result }) => + result?.isSuccess && result.data); + + // Only show loading if we don't have success yet and some are still loading + const isStillLoading = relevantResults.some(({ result }) => + result?.isLoading); + + return !hasSuccess && isStillLoading; }); }, [addresses, pairs, results]);apps/dashboard/src/@/api/insight/webhooks.ts (3)
66-87
: Consider adding request timeout and retry logic.API calls could time out or fail temporarily. Adding timeout limits and retry logic would improve reliability.
const response = await fetch(`${THIRDWEB_INSIGHT_API_DOMAIN}/v1/webhooks`, { method: "POST", headers: { "Content-Type": "application/json", "x-client-id": clientId, Authorization: `Bearer ${authToken}`, }, body: JSON.stringify(payload), + // Add timeout to prevent hanging requests + signal: AbortSignal.timeout(10000), // 10 second timeout });Consider also implementing a retry mechanism with exponential backoff for transient failures.
Also applies to: 89-107, 109-131, 133-157
81-86
: Add response validation.The API response is directly returned without validating its structure. Consider adding validation to ensure the response matches the expected interface.
+// Add a helper function for validation +function validateWebhookResponse(data: unknown): data is WebhookSingleResponse { + return ( + typeof data === "object" && + data !== null && + "data" in data && + typeof data.data === "object" && + data.data !== null && + "id" in data.data && + typeof data.data.id === "string" + ); +} // In each function: - return await response.json(); + const data = await response.json(); + if (!validateWebhookResponse(data)) { + return { + error: true, + errorMessage: "Invalid response format from API" + }; + } + return { + error: false, + data + };Also applies to: 101-106, 125-130, 151-156
71-79
: Add user agent header to API requests.For better tracking and debugging, consider adding a user agent header that identifies the client making the request.
headers: { "Content-Type": "application/json", "x-client-id": clientId, Authorization: `Bearer ${authToken}`, + "User-Agent": "ThirdwebDashboard/1.0", },
Also applies to: 93-99, 114-122, 138-149
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/dashboard/src/@/api/insight/webhooks.ts
(1 hunks)apps/dashboard/src/@/components/blocks/multi-select.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/InsightPageLayout.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/layout.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/page.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/CreateWebhookModal.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/BasicInfoStep.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/FilterDetailsStep.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/ReviewStep.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/StepIndicator.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useAbiProcessing.ts
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook.ts
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/abi-utils.ts
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx
(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/page.tsx (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/insight-ftux.tsx (1)
InsightFTUX
(6-63)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/layout.tsx (3)
apps/dashboard/src/@/api/team.ts (1)
getTeamBySlug
(11-30)apps/dashboard/src/@/api/projects.ts (1)
getProject
(29-48)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/InsightPageLayout.tsx (1)
InsightPageLayout
(6-64)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/BasicInfoStep.tsx (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts (1)
WebhookFormValues
(48-50)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/StepIndicator.tsx (1)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts (2)
WebhookFormStep
(73-73)WebhookFormSteps
(75-79)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook.ts (1)
apps/dashboard/src/@/api/insight/webhooks.ts (1)
testWebhook
(133-157)
apps/dashboard/src/@/api/insight/webhooks.ts (2)
apps/dashboard/src/app/(app)/api/lib/getAuthToken.ts (1)
getAuthToken
(6-14)apps/dashboard/src/constants/urls.ts (1)
THIRDWEB_INSIGHT_API_DOMAIN
(24-25)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts (2)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/abi-utils.ts (1)
isValidAddress
(26-29)packages/thirdweb/src/exports/utils.ts (1)
Abi
(200-200)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useAbiProcessing.ts (2)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_utils/webhook-types.ts (3)
AbiData
(64-71)EventSignature
(52-56)FunctionSignature
(58-62)packages/thirdweb/src/exports/thirdweb.ts (1)
getContract
(69-69)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/page.tsx (1)
32-36
: LGTM! Simplified conditional renderingThe simplified conditional rendering approach improves code readability and removes unnecessary fragment wrapping.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/InsightPageLayout.tsx (1)
1-64
: Clean and well-structured layout component.The
InsightPageLayout
component follows good practices by:
- Clearly defining its props interface
- Using composable UI components like
TabPathLinks
- Implementing responsive design with appropriate spacing
- Creating a consistent page structure with header, nav, content, and footer sections
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/StepIndicator.tsx (1)
13-62
: Well-implemented step indicator with dynamic styling.The component effectively visualizes a multi-step process with proper handling of:
- Current, completed, and upcoming step states
- Visual transitions between steps
- Accessible markup with semantic labels
The use of the
cn
utility for conditional class names is a good practice for managing complex class combinations.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx (1)
66-69
: Add accessibility title attribute to support truncated content.Good use of the
title
attribute on the truncated name to show the full name on hover. This helps with accessibility and usability.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/CreateWebhookModal.tsx (2)
171-186
:secret
field ignored when building the payloadThe form collects a
secret
(line 64) but it’s never forwarded to the backend.
If the API supports signing secrets, omitting it will leave newly-created webhooks unauthenticated.Do we need to include a
secret
property in the payload? If yes, extend both payload branches accordingly.
150-176
: Potential type mismatch forchain_ids
chainIds
are stored as numbers but forwarded directly.
If the API expects strings (as implied bychain_ids?: string[]
), runtime coercion is necessary.Confirm the expected type and, if needed, convert with
.map(id => id.toString())
.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useAbiProcessing.ts (3)
14-30
: Well-structured hook definition with clear typing.The hook accepts a well-defined set of parameters with proper TypeScript typing, making it clear what inputs are required and what outputs to expect.
62-62
: Be cautious with useQueries for large datasets.While
useQueries
is a good choice for parallel data fetching, if the number of queries can be very large (many contracts across many chains), it might lead to performance issues due to high memory usage. Consider implementing pagination or chunking for very large datasets.A previous review comment on line 62 advised: "Avoid using
useQueries
- If the number of queries can be large here it will crash the page due to a large amount of memory used. If possible, please do all request in a singleuseQuery
"
45-60
:⚠️ Potential issueAdd 'type' to dependency array of useMemo.
The query key includes
type
at line 46, but it's not included in the dependency array at line 60. This could lead to stale query keys when the type changes.- }, [pairs, isOpen, addresses, chainIds, type, thirdwebClient]); + }, [pairs, isOpen, addresses, chainIds, thirdwebClient, type]);Likely an incorrect or invalid review comment.
apps/dashboard/src/@/api/insight/webhooks.ts (1)
6-65
: Well-structured TypeScript interfaces.The interfaces are clearly defined with appropriate types and relationships between them. This provides excellent type safety for the API client.
const handleDeleteWebhook = async (webhookId: string) => { | ||
if (isDeleting[webhookId]) return; | ||
|
||
try { | ||
setIsDeleting((prev) => ({ ...prev, [webhookId]: true })); | ||
await deleteWebhook(webhookId, clientId); | ||
toast.success("Webhook deleted successfully"); | ||
router.refresh(); | ||
} catch (error) { | ||
console.error("Error deleting webhook:", error); | ||
toast.error("Failed to delete webhook", { | ||
description: | ||
error instanceof Error | ||
? error.message | ||
: "An unexpected error occurred", | ||
}); | ||
} finally { | ||
setIsDeleting((prev) => ({ ...prev, [webhookId]: false })); | ||
} | ||
}; |
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.
🛠️ Refactor suggestion
Add confirmation dialog before webhook deletion.
The current implementation immediately deletes the webhook upon clicking the delete button without any confirmation, which could lead to accidental deletions. Consider adding a confirmation dialog to prevent users from accidentally deleting webhooks.
import { useState } from "react";
import { toast } from "sonner";
+ import {
+ AlertDialog,
+ AlertDialogAction,
+ AlertDialogCancel,
+ AlertDialogContent,
+ AlertDialogDescription,
+ AlertDialogFooter,
+ AlertDialogHeader,
+ AlertDialogTitle,
+ } from "@/components/ui/alert-dialog";
// ...
export function WebhooksTable({ webhooks, clientId }: WebhooksTableProps) {
const [isDeleting, setIsDeleting] = useState<Record<string, boolean>>({});
+ const [webhookToDelete, setWebhookToDelete] = useState<string | null>(null);
const { testWebhookEndpoint, isTestingMap } = useTestWebhook(clientId);
const router = useDashboardRouter();
+ const openDeleteDialog = (webhookId: string) => {
+ setWebhookToDelete(webhookId);
+ };
+
+ const closeDeleteDialog = () => {
+ setWebhookToDelete(null);
+ };
const handleDeleteWebhook = async (webhookId: string) => {
if (isDeleting[webhookId]) return;
try {
setIsDeleting((prev) => ({ ...prev, [webhookId]: true }));
await deleteWebhook(webhookId, clientId);
toast.success("Webhook deleted successfully");
router.refresh();
} catch (error) {
console.error("Error deleting webhook:", error);
toast.error("Failed to delete webhook", {
description:
error instanceof Error
? error.message
: "An unexpected error occurred",
});
} finally {
setIsDeleting((prev) => ({ ...prev, [webhookId]: false }));
+ closeDeleteDialog();
}
};
// ...
// In the actions cell:
// ...
<Button
size="icon"
variant="outline"
className="h-8 w-8 text-red-500 hover:border-red-700 hover:text-red-700"
- onClick={() => handleDeleteWebhook(webhook.id)}
+ onClick={() => openDeleteDialog(webhook.id)}
disabled={isDeleting[webhook.id]}
>
// ...
// Add confirmation dialog:
+ return (
+ <div className="w-full">
+ <TWTable
+ data={sortedWebhooks}
+ columns={columns}
+ isPending={false}
+ isFetched={true}
+ title="Webhooks"
+ tableContainerClassName="mt-4"
+ />
+
+ <AlertDialog open={!!webhookToDelete} onOpenChange={(isOpen) => !isOpen && closeDeleteDialog()}>
+ <AlertDialogContent>
+ <AlertDialogHeader>
+ <AlertDialogTitle>Are you sure?</AlertDialogTitle>
+ <AlertDialogDescription>
+ This action cannot be undone. This will permanently delete the webhook.
+ </AlertDialogDescription>
+ </AlertDialogHeader>
+ <AlertDialogFooter>
+ <AlertDialogCancel>Cancel</AlertDialogCancel>
+ <AlertDialogAction
+ onClick={() => webhookToDelete && handleDeleteWebhook(webhookToDelete)}
+ disabled={webhookToDelete ? isDeleting[webhookToDelete] : false}
+ >
+ {webhookToDelete && isDeleting[webhookToDelete] ? <Spinner className="h-4 w-4 mr-2" /> : null}
+ Delete
+ </AlertDialogAction>
+ </AlertDialogFooter>
+ </AlertDialogContent>
+ </AlertDialog>
+ </div>
+ );
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx
around lines 31 to 50, the handleDeleteWebhook function deletes a webhook
immediately without user confirmation. To fix this, add a confirmation dialog
before proceeding with the deletion. Implement a prompt or modal that asks the
user to confirm the deletion, and only call deleteWebhook if the user confirms.
const handleTestWebhook = async (webhook: WebhookResponse) => { | ||
await testWebhookEndpoint( | ||
webhook.webhook_url, | ||
webhook.filters["v1.events"] ? "event" : "transaction", | ||
webhook.id, | ||
); | ||
}; |
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 for webhook test operation.
The handleTestWebhook
function doesn't have any error handling. If the test operation fails, users won't receive any notification. Consider adding try/catch and providing feedback to users.
const handleTestWebhook = async (webhook: WebhookResponse) => {
+ try {
await testWebhookEndpoint(
webhook.webhook_url,
webhook.filters["v1.events"] ? "event" : "transaction",
webhook.id,
);
+ } catch (error) {
+ console.error("Error testing webhook:", error);
+ toast.error("Failed to test webhook", {
+ description:
+ error instanceof Error
+ ? error.message
+ : "An unexpected error occurred",
+ });
+ }
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleTestWebhook = async (webhook: WebhookResponse) => { | |
await testWebhookEndpoint( | |
webhook.webhook_url, | |
webhook.filters["v1.events"] ? "event" : "transaction", | |
webhook.id, | |
); | |
}; | |
const handleTestWebhook = async (webhook: WebhookResponse) => { | |
try { | |
await testWebhookEndpoint( | |
webhook.webhook_url, | |
webhook.filters["v1.events"] ? "event" : "transaction", | |
webhook.id, | |
); | |
} catch (error) { | |
console.error("Error testing webhook:", error); | |
toast.error("Failed to test webhook", { | |
description: | |
error instanceof Error | |
? error.message | |
: "An unexpected error occurred", | |
}); | |
} | |
}; |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/webhooks-table.tsx
around lines 52 to 58, the handleTestWebhook function lacks error handling for
the testWebhookEndpoint call. Wrap the await call in a try/catch block to catch
any errors during the test operation. In the catch block, provide user feedback
such as displaying an error notification or message to inform them of the
failure.
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx
Show resolved
Hide resolved
try { | ||
// Await params before accessing properties | ||
const resolvedParams = await params; | ||
const team_slug = resolvedParams.team_slug; | ||
const project_slug = resolvedParams.project_slug; | ||
|
||
const project = await getProject(team_slug, project_slug); | ||
|
||
if (!project) { | ||
notFound(); | ||
} |
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.
🛠️ Refactor suggestion
Consider handling 404 consistently for all error cases.
Currently, notFound()
is called only when a project isn't found, but it should be triggered for any case where required data is missing or inaccessible. Missing params or team slug should trigger the same behavior.
try {
// Await params before accessing properties
const resolvedParams = await params;
const team_slug = resolvedParams.team_slug;
const project_slug = resolvedParams.project_slug;
+ if (!team_slug || !project_slug) {
+ notFound();
+ }
const project = await getProject(team_slug, project_slug);
if (!project) {
notFound();
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
try { | |
// Await params before accessing properties | |
const resolvedParams = await params; | |
const team_slug = resolvedParams.team_slug; | |
const project_slug = resolvedParams.project_slug; | |
const project = await getProject(team_slug, project_slug); | |
if (!project) { | |
notFound(); | |
} | |
try { | |
// Await params before accessing properties | |
const resolvedParams = await params; | |
const team_slug = resolvedParams.team_slug; | |
const project_slug = resolvedParams.project_slug; | |
if (!team_slug || !project_slug) { | |
notFound(); | |
} | |
const project = await getProject(team_slug, project_slug); | |
if (!project) { | |
notFound(); | |
} |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/page.tsx
around lines 14 to 24, the code calls notFound() only when the project is not
found, but it should also handle cases where required parameters like team_slug
or project_slug are missing or invalid. Update the code to check for the
presence and validity of these parameters before fetching the project, and call
notFound() if any are missing or invalid to ensure consistent 404 handling for
all error cases.
import { useTestWebhook } from "app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook"; | ||
import { XIcon } from "lucide-react"; |
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.
🛠️ Refactor suggestion
Invalid import alias – may break module resolution
The path starts with app/(app)
instead of the established @/
alias or a relative path.
Unless a paths
mapping for the bare app/*
root exists in tsconfig.json
, the build will fail with “Cannot find module”.
Replace with a relative import ("./_hooks/useTestWebhook"
) or the correct alias.
-import { useTestWebhook } from "app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook";
+import { useTestWebhook } from "./_hooks/useTestWebhook";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { useTestWebhook } from "app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook"; | |
import { XIcon } from "lucide-react"; | |
import { useTestWebhook } from "./_hooks/useTestWebhook"; | |
import { XIcon } from "lucide-react"; |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/CreateWebhookModal.tsx
at lines 23-24, the import path for useTestWebhook uses an invalid alias
starting with app/(app), which may cause module resolution errors. Replace this
import with a relative path like "./_hooks/useTestWebhook" or use the correct
alias such as
"@/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useTestWebhook"
to ensure proper module resolution.
type WebhookFormValues, | ||
webhookFormSchema, | ||
} from "./_utils/webhook-types"; | ||
const { createWebhook } = await import("@/api/insight/webhooks"); |
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.
Top-level await
is not allowed in client components
"use client"
files are transpiled for the browser; top-level await
here (await import("@/api/insight/webhooks")
) produces a syntax error in every major bundler.
Use a static import or perform the dynamic import inside an async callback.
-const { createWebhook } = await import("@/api/insight/webhooks");
+import { createWebhook } from "@/api/insight/webhooks";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const { createWebhook } = await import("@/api/insight/webhooks"); | |
import { createWebhook } from "@/api/insight/webhooks"; |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/CreateWebhookModal.tsx
at line 36, the top-level await used for dynamic import causes syntax errors in
client components. To fix this, replace the top-level await with a static import
at the top of the file or move the dynamic import inside an async function or
callback where await is allowed.
const watchFilterType = form.watch("filterType") as | ||
| "event" | ||
| "transaction" | ||
| undefined; | ||
const selectedChainIds = form.watch("chainIds").map(Number) || []; | ||
const contractAddresses = form.watch("addresses") || ""; |
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.
chainIds
may be undefined
– protect against runtime error
form.watch("chainIds").map(Number)
will throw if chainIds
hasn’t been touched yet (initial render). Guard first, then map:
-const selectedChainIds = form.watch("chainIds").map(Number) || [];
+const selectedChainIds = (form.watch("chainIds") || []).map(Number);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const watchFilterType = form.watch("filterType") as | |
| "event" | |
| "transaction" | |
| undefined; | |
const selectedChainIds = form.watch("chainIds").map(Number) || []; | |
const contractAddresses = form.watch("addresses") || ""; | |
const watchFilterType = form.watch("filterType") as | |
| "event" | |
| "transaction" | |
| undefined; | |
- const selectedChainIds = form.watch("chainIds").map(Number) || []; | |
+ const selectedChainIds = (form.watch("chainIds") || []).map(Number); | |
const contractAddresses = form.watch("addresses") || ""; |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_components/FilterDetailsStep.tsx
around lines 61 to 66, the code calls map on form.watch("chainIds") without
checking if it is undefined, which can cause a runtime error on initial render.
Fix this by first checking if form.watch("chainIds") is defined before mapping,
for example by using a conditional or defaulting to an empty array before
calling map(Number).
return abis; | ||
}, [results, pairs, addresses, type]); | ||
|
||
const signatures = extractSignatures(Object.values(fetchedAbis)); |
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.
🛠️ Refactor suggestion
Add error handling for signature extraction.
There's no error handling for the extractSignatures
function. If it throws an error, it will crash the component using this hook. Consider wrapping it in a try-catch block.
- const signatures = extractSignatures(Object.values(fetchedAbis));
+ let signatures: EventSignature[] | FunctionSignature[] = [];
+ try {
+ signatures = extractSignatures(Object.values(fetchedAbis));
+ } catch (error) {
+ console.error("Error extracting signatures:", error);
+ // Return empty array instead of crashing
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const signatures = extractSignatures(Object.values(fetchedAbis)); | |
// Wrap signature extraction in try/catch to avoid crashes | |
let signatures: EventSignature[] | FunctionSignature[] = []; | |
try { | |
signatures = extractSignatures(Object.values(fetchedAbis)); | |
} catch (error) { | |
console.error("Error extracting signatures:", error); | |
// Fallback to empty array instead of letting the hook throw | |
} |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/insight/webhooks/components/_hooks/useAbiProcessing.ts
at line 109, the call to extractSignatures lacks error handling, risking a crash
if it throws. Wrap the extractSignatures call in a try-catch block to catch any
errors, handle them gracefully (e.g., log the error or set an error state), and
prevent the component from crashing.
|
||
export default async function WebhooksPage({ | ||
params, | ||
}: { params: { team_slug: string; project_slug: string } }) { |
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.
this type is wrong, params is type Promise<{...}>
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.
There are still too many code quality issues here to ship this as is. Coderabbit also found even more than myself.
Add Webhooks Feature to Insight Dashboard
This PR adds a new webhooks feature to the Insight dashboard, allowing users to create, test, and manage webhooks for blockchain events and transactions. The implementation includes:
The PR also includes several UI improvements:
The webhooks feature enables users to receive real-time notifications about on-chain events through their own endpoints, supporting both event and transaction monitoring with customizable filters.
PR-Codex overview
This PR focuses on enhancing the webhook functionality in the dashboard application by introducing new components, updating existing ones, and implementing better state management for webhooks, including creation, testing, and display.
Detailed summary
Button
component to use a functional approach for props.getProject
andgetTeamBySlug
API calls inlayout.tsx
.Layout
component for handling team and project insights.StepIndicator
component for multi-step forms.Page
component to streamline rendering based on conditions.WebhooksPage
for managing webhooks with error handling.webhookFormSchema
for form validation in webhook creation.CreateWebhookModal
for a user-friendly interface to create webhooks.FilterDetailsStep
andReviewStep
for the webhook creation process.useTestWebhook
hook.Demo
CleanShot 2025-05-07 at 18.55.41.mp4 (uploaded via Graphite)
Summary by CodeRabbit