Add custom webhook function support and refactor fetch logic#198
Add custom webhook function support and refactor fetch logic#198priyanshu6238 merged 4 commits intoglific-masterfrom
Conversation
…handling and add functionality to select webhook
📝 WalkthroughWalkthroughRefactors webhook option loading into a new fetchWebhookOptions helper that HTTP-fetches and maps remote webhook configs; WebhookRouterForm now calls that helper on mount and its function Select supports creating arbitrary options via new props (allowCreate, createArbitraryOption, createPrefix). Changes
Sequence Diagram(s)sequenceDiagram
participant Form as WebhookRouterForm
participant Helper as helpers.fetchWebhookOptions
participant API as Remote API
participant Select as SelectElement
Form->>Helper: fetchWebhookOptions(endpoint, currentUrl)
Helper->>API: HTTP GET /webhook/options
API-->>Helper: webhook config payload
Helper-->>Form: { webhookOptions, webhookFunction, isLoading }
Form->>Select: render with webhookOptions + allowCreate/createArbitraryOption
Select->>Form: user creates arbitrary option (createArbitraryOption)
Form-->>Select: add new option, set webhookFunction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/flow/routers/webhook/helpers.ts`:
- Around line 169-182: fetchWebhookOptions currently assumes data.webhook is an
array and uses (data.webhook || []).map(...), which will throw if data.webhook
is a non-array; update the function (fetchWebhookOptions) to normalize the
payload first (e.g., const webhooks = Array.isArray(data.webhook) ? data.webhook
: []) and then build webhookOptions from webhooks so mapping is safe and the UI
won’t crash on non-array payloads; keep the same mapping logic and return shape
(webhookOptions, webhookFunction, isLoading).
In `@src/components/flow/routers/webhook/WebhookRouterForm.tsx`:
- Around line 383-385: Remove the stray debug console.log call in
WebhookRouterForm: locate the line using console.log(this.state.webhookFunction)
near where tabs.reverse() is called (inside the render/return flow) and delete
it (or replace with a proper logger if persistent logging is required); ensure
no other leftover console.debug/console.log calls remain in the
WebhookRouterForm component before merging.
- Around line 419-423: The createArbitraryOption passed to TembaSelectElement in
WebhookRouterForm currently mutates state by calling
this.handleWebhookFunctionChanged and then returning an option, causing a double
update when the select's onChange (this.handleWebhookFunctionChanged) fires;
change createArbitraryOption to be pure—only construct and return { name: val,
value: val, id: val, label: val } with no side effects—and let the existing
onChange={this.handleWebhookFunctionChanged} perform the single state update and
body reset.
| this.setState(stateUpdate); | ||
| } catch (error) { | ||
| console.error('Error fetching webhook options:', error); | ||
| } finally { |
There was a problem hiding this comment.
because isLoading is set to false in the fetchWebhookOptions function itself
There was a problem hiding this comment.
but it would not get called on any failiure right. So it doesn't gets called when it reaches the catch block.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/flow/routers/webhook/WebhookRouterForm.tsx (2)
68-71:⚠️ Potential issue | 🟡 Minor
isLoadingis not initialized in the constructor.
WebhookRouterFormStatedeclaresisLoading: boolean, but the constructor doesn't set it. Between mount andcomponentDidMount, it will beundefined. Initialize it to prevent the type mismatch and avoid a brief render with indeterminate state.Proposed fix
this.state = { ...nodeToState(this.props.nodeSettings), - webhookOptions: [] + webhookOptions: [], + isLoading: false };
78-88:⚠️ Potential issue | 🟠 MajorFix null-safety bug in
fetchWebhookOptions:data.webhookmay be undefinedAt
src/components/flow/routers/webhook/helpers.ts(lines 176–183),data.webhook.map(...)will throw a TypeError ifdata.webhookis undefined or null, since the?? []operator runs after.map(). Although the error is caught in the component's try-catch block, webhook options silently fail to load.Use optional chaining instead:
Suggested fix
const webhookOptions: WebhookOption[] = data.webhook?.map((webhook: any) => ({ name: webhook.name, value: webhook.name, id: webhook.name, label: webhook.name, body: webhook.body })) ?? [];
| return method === Methods.GET ? '' : DEFAULT_BODY; | ||
| }; | ||
|
|
||
| export interface WebhookOption { |

Summary
Summary by CodeRabbit
New Features
Bug Fixes / Improvements