Dropdown for webhooks #176
Conversation
WalkthroughThe pull request introduces dynamic webhook function selection for the webhook router component, replacing static URL input with a dropdown selector for FUNCTION method type. It updates state management to track selected webhook functions and fetches options from an async endpoint. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form as WebhookRouterForm
participant API as Webhook API
participant State as Form State
Note over Form: Component Mount
Form->>API: Fetch webhook options
API-->>Form: Return webhookOptions
Form->>State: Initialize webhookOptions
User->>Form: Select webhook function
Form->>Form: handleWebhookFunctionChanged()
rect rgb(200, 220, 255)
Note over Form: Update state with selected function
Form->>State: Set webhookFunction
Form->>State: Set URL from webhook metadata
Form->>State: Set body (if applicable)
end
alt Function Selected
Form-->>User: Display function details
else Function Cleared
Form-->>User: Reset form fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/flow/routers/webhook/WebhookRouterForm.tsx (1)
270-285: Critical: Add validation to ensure webhook function is selected for FUNCTION method.Currently, the FUNCTION method only validates the result name. This allows saving a webhook without selecting a function, resulting in an empty URL that will cause runtime failures.
Apply this diff to add validation:
private handleSave(): void { let valid = false; if (this.state.method.value.name === 'FUNCTION') { - valid = this.handleUpdate({ resultName: this.state.resultName.value }, true); + // Validate that a function is selected + if (!this.state.webhookFunction.value) { + const updates: Partial<WebhookRouterFormState> = { + webhookFunction: { + value: null, + validationFailures: [{ message: i18n.t('forms.function_required', 'Please select a function') }] + } + }; + this.setState(mergeForm(this.state, updates) as WebhookRouterFormState); + return; + } + valid = this.handleUpdate({ resultName: this.state.resultName.value }, true); } else { valid = this.handleUpdate( { url: this.state.url.value, resultName: this.state.resultName.value }, true ); } if (valid) { this.props.updateRouter(stateToNode(this.props.nodeSettings, this.state)); this.props.onClose(false); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/components/flow/routers/webhook/__snapshots__/WebhookRouterForm.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
package.json(1 hunks)src/components/flow/routers/webhook/WebhookRouterForm.tsx(7 hunks)src/components/flow/routers/webhook/helpers.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/flow/routers/webhook/WebhookRouterForm.tsx (7)
src/store/nodeEditor.ts (2)
FormEntry(22-25)mergeForm(63-156)src/components/flow/props.ts (1)
RouterFormProps(35-47)src/config/ConfigProvider.tsx (1)
fakePropType(4-4)src/components/flow/routers/webhook/helpers.ts (3)
nodeToState(55-110)getDefaultBody(156-158)isValidJson(160-170)src/store/validators.ts (1)
validate(118-132)src/temba/TembaSelectElement.tsx (1)
TembaSelectElement(13-26)src/components/form/textinput/TextInputElement.tsx (1)
TextInputElement(35-94)
🔇 Additional comments (8)
package.json (1)
5-5: LGTM!The version bump is appropriate for the webhook dropdown feature additions in this PR.
src/components/flow/routers/webhook/helpers.ts (1)
66-67: LGTM with a minor concern about stale function references.The additions to the initial state properly support the FUNCTION method workflow. The WebhookRouterForm handles pre-selection in componentDidMount (lines 90-101 in WebhookRouterForm.tsx).
However, if a previously saved function name no longer exists in the fetched options (e.g., deleted or renamed), the webhookFunction will be set to null, potentially creating a confusing UX where the URL field contains a function name but nothing is selected in the dropdown.
Consider adding fallback logic in WebhookRouterForm.tsx componentDidMount to display a warning or create a placeholder option when a saved function name doesn't match current options.
src/components/flow/routers/webhook/WebhookRouterForm.tsx (6)
19-19: LGTM!The new imports are properly utilized for the FUNCTION method dropdown implementation.
Also applies to: 36-36
52-53: LGTM!The state interface additions and context setup properly enable dynamic webhook function loading from the config endpoint.
Also applies to: 60-62
103-125: LGTM!The webhook function selection handler properly synchronizes the webhookFunction, URL, and body fields when a function is selected or cleared.
155-158: LGTM!Correctly skips setting default body for FUNCTION methods since the body is populated from the selected webhook function.
197-205: LGTM!The conditional JSON validation appropriately allows empty bodies for FUNCTION methods while maintaining validation for non-empty content.
394-415: LGTM!The conditional rendering appropriately switches between the webhook function dropdown for FUNCTION methods and the URL input field for other HTTP methods.
| async componentDidMount() { | ||
| const endpoint = this.context.config.endpoints.completion; | ||
| const response = await fetch(endpoint); | ||
| const data = await response.json(); | ||
|
|
||
| const webhookOptions = (data.webhook || []).map((webhook: any) => ({ | ||
| name: webhook.name, | ||
| value: webhook.name, | ||
| id: webhook.name, | ||
| label: webhook.name, | ||
| body: webhook.body | ||
| })); | ||
|
|
||
| if (this.state.method.value.value === Methods.FUNCTION && this.state.url.value) { | ||
| const functionName = this.state.url.value; | ||
| const selectedOption = webhookOptions.find((opt: any) => opt.name === functionName); | ||
|
|
||
| this.setState({ | ||
| webhookOptions, | ||
| webhookFunction: { value: selectedOption || null } | ||
| }); | ||
| } else { | ||
| this.setState({ webhookOptions }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add error handling and loading state for the async fetch.
The fetch call lacks error handling, which could cause unhandled promise rejections or a broken component state if the endpoint fails or returns unexpected data.
Apply this diff to add proper error handling:
async componentDidMount() {
- const endpoint = this.context.config.endpoints.completion;
- const response = await fetch(endpoint);
- const data = await response.json();
-
- const webhookOptions = (data.webhook || []).map((webhook: any) => ({
- name: webhook.name,
- value: webhook.name,
- id: webhook.name,
- label: webhook.name,
- body: webhook.body
- }));
-
- if (this.state.method.value.value === Methods.FUNCTION && this.state.url.value) {
- const functionName = this.state.url.value;
- const selectedOption = webhookOptions.find((opt: any) => opt.name === functionName);
-
- this.setState({
- webhookOptions,
- webhookFunction: { value: selectedOption || null }
- });
- } else {
- this.setState({ webhookOptions });
- }
+ try {
+ const endpoint = this.context.config.endpoints.completion;
+ const response = await fetch(endpoint);
+
+ if (!response.ok) {
+ console.error('Failed to fetch webhook options:', response.statusText);
+ return;
+ }
+
+ const data = await response.json();
+ const webhookOptions = (data.webhook || []).map((webhook: any) => ({
+ name: webhook.name,
+ value: webhook.name,
+ id: webhook.name,
+ label: webhook.name,
+ body: webhook.body
+ }));
+
+ if (this.state.method.value.value === Methods.FUNCTION && this.state.url.value) {
+ const functionName = this.state.url.value;
+ const selectedOption = webhookOptions.find((opt: any) => opt.name === functionName);
+
+ this.setState({
+ webhookOptions,
+ webhookFunction: { value: selectedOption || null }
+ });
+ } else {
+ this.setState({ webhookOptions });
+ }
+ } catch (error) {
+ console.error('Error loading webhook options:', error);
+ }
}🤖 Prompt for AI Agents
In src/components/flow/routers/webhook/WebhookRouterForm.tsx around lines 77 to
101, the async fetch has no error handling or loading state; wrap the fetch and
JSON parse in a try/catch/finally, set a loading flag in state before the
request and clear it in finally, check response.ok and handle non-200 responses
by throwing or setting an error state, and on error set webhookOptions to an
empty array (and webhookFunction to null if applicable) so the component remains
stable; also optionally store the error message in state for UI/display.
Summary by CodeRabbit
New Features
Chores