-
Notifications
You must be signed in to change notification settings - Fork 249
feat: Allow to add webhooks from Create Alert modal #767
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
feat: Allow to add webhooks from Create Alert modal #767
Conversation
ernestii
commented
Apr 19, 2025
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR adds the ability to create and add webhooks directly from the Create Alert modal. The key changes include:
- Integrating a modal in Alerts.tsx to host the CreateWebhookForm.
- Refetching the list of webhooks upon successful webhook creation and updating the form field.
- Exporting and updating the CreateWebhookForm component in TeamPage.tsx, including a change in its onSuccess signature to receive a webhook ID.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/app/src/components/Alerts.tsx | Adds modal logic and uses a new webhook creation form to update webhook state. |
packages/app/src/TeamPage.tsx | Exports and adjusts CreateWebhookForm to pass the generated webhook ID on success. |
@@ -711,7 +711,7 @@ function CreateWebhookForm({ | |||
color: 'green', | |||
message: `Webhook created successfully`, | |||
}); | |||
onSuccess(); | |||
onSuccess(response.data?._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.
Potential issue: If response.data is undefined, onSuccess will be called with undefined. Consider adding an explicit null check or a fallback to ensure that a valid webhook ID is passed.
onSuccess(response.data?._id); | |
if (response.data && response.data._id) { | |
onSuccess(response.data._id); | |
} else { | |
console.warn('Webhook creation succeeded, but no ID was returned.'); | |
notifications.show({ | |
color: 'yellow', | |
message: 'Webhook created, but no ID was returned by the server.', | |
}); | |
} |
Copilot uses AI. Check for mistakes.
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.
@ernestii what happens when response.data
is empty in the upstream onSuccess
event? Depending on that, this may be a valid suggestion (except for the notification).
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.
technically api should never return 200 with empty data, it'll return 500 after catching the error
await webhook.save();
res.json({
data: webhook,
});
} catch (err) {
next(err);
}
onSuccess
ultimately is used for selecting the newly created webhook in the dropdown, so I made the webhookId
optional upstream
eb18f94
to
a1d4a36
Compare
…ing-webhook-opened-a-modal