Fixed webhooks on v4 and nested values #1704
Conversation
📝 WalkthroughWalkthroughReplaced enum value checks with enum instances, changed ToggleButtons default to enum case, adjusted embed color conversion and Regular headers processing, removed dehydratedWhenHidden from sections, switched replaceVars to data_get, made getWebhookSampleData static with a richer payload, and refactored the Blade webhook section layout. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant F as Filament Form
participant P as EditWebhookConfiguration
participant M as WebhookConfiguration
U->>F: Open form / select type / edit
F->>P: mutateFormDataBeforeFill(data)
Note right of P #dfefff: Use enum instances<br/>Ensure Regular headers default to []
P-->>F: filled data shown
U->>F: Save
F->>P: mutateFormDataBeforeSave(data)
Note right of P #dfefff: Use enum instances<br/>If embed present, convert color via hexdec(str_replace('#', '', ...))<br/>Re-key headers when set
P-->>F: sanitized data
F->>M: persist(update/create)
sequenceDiagram
autonumber
participant W as WebhookConfiguration
participant S as getWebhookSampleData()
participant R as replaceVars
W->>S: static::getWebhookSampleData()
Note right of S #f7f7df: Returns richer, nested sample payload
S-->>W: sample eventData
W->>R: replaceVars(template, eventData)
Note right of R #f7f7df: Uses data_get for dot-path resolution
R-->>W: rendered payload
W-->>W: dispatch webhook
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php (2)
44-69: 'type' comparison likely never matches unless form stores enum instancesYou changed to direct enum comparisons. If the form still submits the backing value, these blocks won’t run. Pair with the Resource change (use ->enum(WebhookType::class) and default to enum) or revert to ->value comparisons.
Option A (recommended, enum state end-to-end — matches Resource diff):
- Keep these enum comparisons as-is.
Option B (backing values):
- if (($data['type'] ?? null) === WebhookType::Discord) { + if (($data['type'] ?? null) === WebhookType::Discord->value) {Also harden color handling to avoid TypeError when color is null/empty:
- $embed['color'] = hexdec(str_replace('#', '', data_get($embed, 'color'))); - $embed = collect($embed)->filter(fn ($key) => is_array($key) ? array_filter($key, fn ($arr_key) => !empty($arr_key)) : !empty($key))->all(); + $color = data_get($embed, 'color'); + if ($color !== null && $color !== '') { + $embed['color'] = hexdec(ltrim((string) $color, '#')); + } else { + unset($embed['color']); + } + $embed = collect($embed)->filter(fn ($val) => is_array($val) ? array_filter($val, fn ($arr_key) => !empty($arr_key)) : !empty($val))->all();
85-112: Fill path: match enum comparison, guard color, and cast flags to intPrevents TypeErrors when color/flags are missing.
- if (($data['type'] ?? null) === WebhookType::Discord) { + if (($data['type'] ?? null) === WebhookType::Discord) { $embeds = data_get($data, 'payload.embeds', []); foreach ($embeds as &$embed) { - $embed['color'] = '#' . dechex(data_get($embed, 'color')); - $embed = collect($embed)->filter(fn ($key) => is_array($key) ? array_filter($key, fn ($arr_key) => !empty($arr_key)) : !empty($key))->all(); + $color = data_get($embed, 'color'); + if ($color !== null && $color !== '') { + $embed['color'] = sprintf('#%06X', (int) $color); + } else { + unset($embed['color']); + } + $embed = collect($embed)->filter(fn ($val) => is_array($val) ? array_filter($val, fn ($arr_key) => !empty($arr_key)) : !empty($val))->all(); } - $flags = data_get($data, 'payload.flags'); - $flags = collect(range(0, PHP_INT_SIZE * 8 - 1)) + $flags = (int) data_get($data, 'payload.flags', 0); + $flags = collect(range(0, PHP_INT_SIZE * 8 - 1)) ->filter(fn ($i) => ($flags & (1 << $i)) !== 0) ->map(fn ($i) => 1 << $i) ->values();Also align the Regular check below to enum (if adopting enum form state):
- if (($data['type'] ?? null) === WebhookType::Regular->value) { + if (($data['type'] ?? null) === WebhookType::Regular) { $data['headers'] = $data['headers'] ?? []; }
🧹 Nitpick comments (2)
app/Models/WebhookConfiguration.php (1)
191-197: Static sample data source: LGTM — but align default event vs sample “event”run() defaults to 'eloquent.created: App\Models\Server' while the sample contains 'updated: Server' (and will be overwritten in ProcessWebhook). Either switch the default event to “updated” or drop 'event' from the sample to avoid confusion.
app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php (1)
71-78: Guard headers shape during re-keyingisset() can be true for non-arrays; foreach would error. Also coerce values to strings.
- if (($data['type'] ?? null) === WebhookType::Regular && isset($data['headers'])) { + if (($data['type'] ?? null) === WebhookType::Regular && is_array($data['headers'] ?? null)) { $newHeaders = []; foreach ($data['headers'] as $key => $value) { $newKey = str_replace(' ', '-', $key); - $newHeaders[$newKey] = $value; + $newHeaders[$newKey] = (string) $value; } $data['headers'] = $newHeaders; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php(3 hunks)app/Filament/Admin/Resources/Webhooks/WebhookResource.php(1 hunks)app/Models/WebhookConfiguration.php(2 hunks)resources/views/filament/components/webhooksection.blade.php(0 hunks)
💤 Files with no reviewable changes (1)
- resources/views/filament/components/webhooksection.blade.php
🧰 Additional context used
🧬 Code graph analysis (1)
app/Models/WebhookConfiguration.php (2)
app/Filament/Admin/Widgets/DiscordPreview.php (1)
getWebhookSampleData(118-162)app/Jobs/ProcessWebhook.php (1)
ProcessWebhook(17-78)
🔇 Additional comments (1)
app/Models/WebhookConfiguration.php (1)
202-398: Richer sample payload looks goodGood coverage of nested fields (allocation, variables, server_variables). No blockers from me.
| $trimmed = trim($matches[1]); | ||
|
|
||
| return Arr::get($replacement, $trimmed, $trimmed); | ||
| return data_get($replacement, $trimmed, $trimmed); | ||
| }, | ||
| $subject |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return only strings from replaceVars to avoid "Array to string" and broken JSON
data_get may return arrays/objects; preg_replace_callback expects a string. Encode complex values and cast scalars.
- return data_get($replacement, $trimmed, $trimmed);
+ $value = data_get($replacement, $trimmed, $trimmed);
+ if (is_array($value) || is_object($value)) {
+ return json_encode($value, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
+ }
+ return (string) $value;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $trimmed = trim($matches[1]); | |
| return Arr::get($replacement, $trimmed, $trimmed); | |
| return data_get($replacement, $trimmed, $trimmed); | |
| }, | |
| $subject | |
| $trimmed = trim($matches[1]); | |
| $value = data_get($replacement, $trimmed, $trimmed); | |
| if (is_array($value) || is_object($value)) { | |
| return json_encode($value, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); | |
| } | |
| return (string) $value; | |
| }, | |
| $subject |
🤖 Prompt for AI Agents
In app/Models/WebhookConfiguration.php around lines 182 to 186, the
preg_replace_callback currently returns whatever data_get yields which may be an
array/object and causes "Array to string" or broken JSON; modify the callback so
it always returns a string: retrieve the value with data_get, then if value is
null return empty string, if value is scalar cast to string, and if value is
array/object json_encode it (optionally with
JSON_UNESCAPED_SLASHES|JSON_UNESCAPED_UNICODE), ensuring the callback never
returns non-string types.
Co-authored-by: MartinOscar <40749467+rmartinoscar@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Models/WebhookConfiguration.php (1)
66-72: Fix spread of original events: getOriginal('events') is JSON string (cannot be unpacked).Unpacking a string will throw “Cannot unpack non-array/Traversable”. Decode or normalize before merging.
- $changedEvents = collect([ - ...($webhookConfiguration->events), - ...$webhookConfiguration->getOriginal('events', '[]'), - ])->unique(); + $current = $webhookConfiguration->events ?? []; + $originalRaw = $webhookConfiguration->getOriginal('events'); + $original = is_string($originalRaw) ? (json_decode($originalRaw, true) ?: []) : ($originalRaw ?? []); + $changedEvents = collect(array_merge($current, $original))->unique();
♻️ Duplicate comments (1)
app/Models/WebhookConfiguration.php (1)
178-186: replaceVars must always return a string; encode arrays/objects and keep unknown placeholders intact.Current callback may return arrays/objects, breaking preg_replace_callback and JSON. Also, falling back to the trimmed key (without braces) is leaky.
- function ($matches) use ($replacement) { - $trimmed = trim($matches[1]); - - return data_get($replacement, $trimmed, $trimmed); - }, + function ($matches) use ($replacement) { + $trimmed = trim($matches[1]); + $value = data_get($replacement, $trimmed); + if (is_array($value) || is_object($value)) { + return json_encode($value, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + } + if ($value === null) { + return $matches[0]; // keep {{placeholder}} if missing + } + return (string) $value; + },If you prefer blanking missing keys, replace
return $matches[0];withreturn '';.
🧹 Nitpick comments (3)
app/Models/WebhookConfiguration.php (1)
79-86: Optional: collapse cache updates to reduce N queries.Iterating events and querying per-event can be costly. Consider 1 query to fetch all configs containing any of the events, then group results per event before caching.
resources/views/filament/components/webhooksection.blade.php (2)
33-42: Render label schema as HTML like other sections for consistency.Use toHtmlString() for BEFORE_LABEL/AFTER_LABEL blocks to mirror usage elsewhere and avoid dumping objects.
- {{ $getChildSchema($schemaComponent::BEFORE_LABEL_SCHEMA_KEY) }} + {{ $getChildSchema($schemaComponent::BEFORE_LABEL_SCHEMA_KEY)?->toHtmlString() }} ... - {{ $getChildSchema($schemaComponent::AFTER_LABEL_SCHEMA_KEY) }} + {{ $getChildSchema($schemaComponent::AFTER_LABEL_SCHEMA_KEY)?->toHtmlString() }}
70-72: Livewire in heading: confirm intent and scope.Embedding DiscordPreview in the heading will mount an extra Livewire instance each render. If this section is used beyond Discord webhooks, gate it (e.g., by type) or lazy-load.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php(2 hunks)app/Filament/Admin/Resources/Webhooks/WebhookResource.php(1 hunks)app/Models/WebhookConfiguration.php(2 hunks)resources/views/filament/components/webhooksection.blade.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/Filament/Admin/Resources/Webhooks/WebhookResource.php
- app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php
🧰 Additional context used
🧬 Code graph analysis (1)
app/Models/WebhookConfiguration.php (2)
app/Filament/Admin/Widgets/DiscordPreview.php (1)
getWebhookSampleData(118-162)app/Jobs/ProcessWebhook.php (1)
ProcessWebhook(17-78)
🔇 Additional comments (4)
app/Models/WebhookConfiguration.php (2)
193-194: Static call for sample data — LGTM.Using
static::getWebhookSampleData()aligns with the static method and eases testing.
201-201: Static getWebhookSampleData — LGTM.Signature change is consistent with the new call site.
resources/views/filament/components/webhooksection.blade.php (2)
49-69: Verify Filament v4 props.Confirm x-filament::section supports
content-before,has-content-el, andheading-tagin your pinned v4 version.
22-32: Wrapper composition and attribute merges — LGTM.Clean separation of outer wrapper and inner section; attribute merges look correct.
2 participants
This pull request repairs the variables such as {{allocation.id}} not working, and editing the old webhook example to match the exact one being sent on updated: Server.