-
Notifications
You must be signed in to change notification settings - Fork 400
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
add optional config to webhooks in engine #5817
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. |
<FormControl> | ||
<FormLabel>Config</FormLabel> | ||
<Input | ||
type="json" |
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 HTML specification does not include type="json"
as a valid input type. This should be changed to type="text"
to ensure consistent behavior across browsers. The JSON validation can still be performed on the input's value while maintaining standard HTML compliance.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5817 +/- ##
=======================================
Coverage 53.34% 53.34%
=======================================
Files 1101 1101
Lines 59124 59124
Branches 4825 4825
=======================================
Hits 31541 31541
Misses 26864 26864
Partials 719 719
*This pull request uses carry forward flags. Click here to find out more. |
size-limit report 📦
|
import { Button, FormLabel } from "tw-components"; | ||
import { beautifyString } from "./webhooks-table"; | ||
|
||
interface AddWebhookButtonProps { | ||
instanceUrl: string; | ||
authToken: string; | ||
supportesWebhookConfig: 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.
The prop name supportesWebhookConfig
appears to have a spelling error - it should be supportsWebhookConfig
. This typo should be corrected across all component interfaces and usages to maintain consistency and prevent confusion.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
PR-Codex overview
This PR focuses on enhancing the webhook functionality within the
useEngine
hook and related components by adding support for a newconfig
property, allowing for more detailed webhook configurations.Detailed summary
"WEBHOOK_CONFIG"
to feature flags.config
property inEngineWebhook
andCreateWebhookInput
.useEngineSystemHealth
inEngineWebhooks
to check forWEBHOOK_CONFIG
support.AddWebhookButton
to handleconfig
input and validation.WebhooksTable
to displayconfig
when supported.config
details as needed.