-
Notifications
You must be signed in to change notification settings - Fork 530
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.03% 55.02% -0.01%
==========================================
Files 897 898 +1
Lines 57296 57352 +56
Branches 3959 3969 +10
==========================================
+ Hits 31530 31560 +30
- Misses 25670 25692 +22
- Partials 96 100 +4
🚀 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
8f744ae
to
d9a7521
Compare
d9a7521
to
40f5370
Compare
40f5370
to
7f83945
Compare
...eam/[team_slug]/[project_slug]/insight/webhooks/components/_components/FilterDetailsStep.tsx
Outdated
Show resolved
Hide resolved
@jnsdls would love to get your help get this through the door. Im sure I didn't reuse enough components/helpers from dashboard and the structure of the implementation might be different from the rest (I tried to keep it encapsulated for future iterations by the professionals). I also touched the multichainselector and the sidebar due to typing warnings |
7f83945
to
527f82a
Compare
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.
partial review
const response = await fetch(`${THIRDWEB_INSIGHT_API_DOMAIN}/v1/webhooks`, { | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
"x-client-id": clientId, | ||
}, | ||
body: JSON.stringify(payload), | ||
}); |
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.
I hope this doesn't actually work - clientId cannot be enough to actually authenticate this request
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.
yes it works
const response = await fetch( | ||
`${THIRDWEB_INSIGHT_API_DOMAIN}/v1/webhooks/${webhookId}`, | ||
{ | ||
method: "DELETE", | ||
headers: { | ||
"x-client-id": clientId, | ||
}, | ||
}, | ||
); | ||
|
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.
same
const response = await fetch(`${THIRDWEB_INSIGHT_API_DOMAIN}/v1/webhooks`, { | ||
method: "GET", | ||
headers: { | ||
"x-client-id": clientId, | ||
}, | ||
}); |
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.
same
const response = await fetch( | ||
`${THIRDWEB_INSIGHT_API_DOMAIN}/v1/webhooks/test`, | ||
{ | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
"x-client-id": clientId, | ||
}, | ||
body: JSON.stringify(payload), |
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.
same
{props.customTrigger || ( | ||
<Button | ||
ref={ref} | ||
{...props} | ||
{...(() => { | ||
// Extract customTrigger from props to avoid passing it to the DOM | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
const { customTrigger, ...restProps } = props; | ||
return restProps; | ||
})()} |
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 sure is a fancy way of doing {customTrigger, ...props}
above
const isTesting = isTestingMap[webhookUrl || ""]; | ||
// Get test result if available | ||
const testResult = testResults[webhookUrl || ""]; | ||
const isRecent = isRecentResult(webhookUrl || ""); |
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 does recency matter?
} | ||
|
||
export function useAbiProcessing({ | ||
form, |
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.
tbh passing this form
around everywhere makes me feel weird, why not just pass the values we need into the hook?
|
||
try { | ||
// For each address, try to fetch ABI | ||
for (const address of 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.
can we do this in a Promise.all instead? depending on how many contract this is this is going to make N network requests sequentially, could take significant amounts of time...
const { data: fetchedEventAbiData, isFetching: isFetchingEventAbi } = | ||
useQuery<AbiResponse | null, Error>({ |
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.
what you want here is useQueries - because that will let you create one query per chain+contract pair which means once you've fetched it once it can be cached even if you then add a second chain+contract pair, as it is now you are invalidating the query and refetching everything every time you add or remove a contract+chain pair
const invalidAddresses = addresses.filter( | ||
(addr) => !isValidAddress(addr), | ||
); | ||
if (invalidAddresses.length > 0) { | ||
return null; | ||
} |
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 silent filter feels weird, we should probably surface that something entered is not a valid contract address?
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
webhooks
feature in the dashboard by improving the layout, adding new components, and refining existing functionality for better user experience and performance.Detailed summary
Layout
component for insights with project and team data retrieval.StepIndicator
for webhook creation steps.BasicInfoStep
,FilterDetailsStep
, andReviewStep
components for structured webhook setup.Demo
CleanShot 2025-05-07 at 18.55.41.mp4 (uploaded via Graphite)