Added plugin system for webhooks and removed discord webhooks - #2498
Open
JoanFo1456 wants to merge 5 commits into
Open
Added plugin system for webhooks and removed discord webhooks#2498JoanFo1456 wants to merge 5 commits into
JoanFo1456 wants to merge 5 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR replaces the fixed webhook enum and Discord-specific paths with registered schemas. It adds shared forms and previews, application API endpoints, schema-driven validation and delivery, plugin availability handling, and integration coverage. ChangesWebhook schema platform
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
app/Extensions/Webhooks/WebhookTypeService.php (1)
18-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider surfacing duplicate type ids.
register()silently discards a schema when the id already exists. A plugin author gets no signal that the id clashed. Log a warning to make the conflict visible.♻️ Proposed change
public function register(WebhookSchemaInterface $schema): void { if (array_key_exists($schema->getId(), $this->schemas)) { + logger()->warning('Webhook type already registered, ignoring duplicate.', [ + 'type' => $schema->getId(), + 'schema' => $schema::class, + ]); + return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Extensions/Webhooks/WebhookTypeService.php` around lines 18 - 25, Update WebhookTypeService::register to log a warning when array_key_exists detects a duplicate schema id before returning, including the conflicting id in the warning; preserve the existing behavior of retaining the original schema and skipping replacement.app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php (1)
43-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSchema mutation hooks are copied across page classes. Each Filament webhook page repeats the same lookup of
WebhookTypes::get($data['type'] ?? null)followed by a schema mutation call. The shared root cause is a missing trait for this hook.
app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php#L43-L59: movemutateFormDataBeforeSaveandmutateFormDataBeforeFillinto a new trait, for exampleMutatesWebhookFormData, and use the trait here.app/Filament/Server/Resources/Webhooks/Pages/CreateWebhook.php#L47-L49: use the same trait and call its save-time helper after the server and scope assignments.The admin create and edit pages contain the same block, so apply the trait there as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php` around lines 43 - 59, The webhook form-data mutation logic is duplicated across the server pages. In app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php lines 43-59, move mutateFormDataBeforeSave and mutateFormDataBeforeFill into a shared MutatesWebhookFormData trait and use it in EditWebhook; in app/Filament/Server/Resources/Webhooks/Pages/CreateWebhook.php lines 47-49, use the same trait and invoke its save-time mutation helper after server and scope assignments. Apply the shared trait to the corresponding admin create and edit pages as well.app/Extensions/Webhooks/Schemas/BaseSchema.php (1)
122-125: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCast the config value to
int.
config('panel.webhook.timeout')resolves fromenv('APP_WEBHOOK_TIMEOUT', 30). Laravel returns environment values as strings, so this method returns a string that PHP coerces toint. If an operator sets a non-numeric value, the coercion raises aTypeErrorinside the queued job. An explicit cast removes that failure mode.♻️ Proposed change
protected function getTimeout(): int { - return config('panel.webhook.timeout', 30); + return (int) config('panel.webhook.timeout', 30); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Extensions/Webhooks/Schemas/BaseSchema.php` around lines 122 - 125, Update the getTimeout() method to explicitly cast the panel.webhook.timeout configuration value to int before returning it, while retaining the existing 30 default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Extensions/Webhooks/WebhookForm.php`:
- Around line 19-29: Update typeSelector() so stored webhook types that are no
longer returned by WebhookTypes::getOptions() remain valid during editing:
include the current record’s unavailable type as a disabled option, or
conditionally relax ToggleButtons’ implicit option validation for that record,
while preserving unavailable_type display behavior.
In `@app/Extensions/Webhooks/WebhookPreview.php`:
- Around line 48-52: Update WebhookPreview to accept and use a resolved scope
property when building sample data, falling back to the record scope only when
appropriate so server create previews select getServerWebhookSampleData(). Pass
the section’s resolved scope into the component when it mounts from
webhook-preview-section.blade.php, preserving existing behavior for global
previews and saved records.
In `@app/Filament/Admin/Resources/Webhooks/WebhookResource.php`:
- Line 177: Update the endpoint field’s afterStateUpdated callback to read the
current type and apply WebhookTypeService::detect($state) only when the type is
empty or equals WebhookTypeService::Default; otherwise preserve the user’s
explicit selection. Import and use App\Extensions\Webhooks\WebhookTypeService.
In `@app/Filament/Server/Resources/Webhooks/WebhookResource.php`:
- Around line 133-135: Update the endpoint TextInput in WebhookResource to reuse
the existing shared endpoint validation rule if one exists, ensuring URL
validation restricts accepted schemes to HTTP/HTTPS as appropriate. Preserve the
required constraint and align the server panel form validation with the API
request classes.
- Line 136: Update the endpoint after-state handler in WebhookResource so it
only assigns the detected type when WebhookTypes::detect($state) identifies a
schema or when the current type is empty; otherwise preserve the manually
selected or existing type, including legacy plugin types. Use the current form
state to avoid overwriting it with the detector’s regular fallback.
In `@app/Http/Requests/Api/Application/Webhooks/GetWebhookRequest.php`:
- Around line 9-14: Add validation rules to GetWebhookRequest so per_page is a
positive, bounded integer and scope accepts only WebhookScope::global or
WebhookScope::server. Ensure index() and events() consume the validated values,
preventing invalid scope values from defaulting to WebhookScope::Global.
In `@app/Http/Requests/Api/Application/Webhooks/StoreWebhookRequest.php`:
- Around line 47-53: Guard dynamic request inputs in StoreWebhookRequest,
including resolveType and the rules()/withValidator() resolver paths, so array
values for type, endpoint, or scope are rejected or normalized to a single
string before calling WebhookTypes::get(), WebhookTypes::detect(), or
WebhookScope::tryFrom(). Preserve valid scalar inputs and ensure malformed
arrays produce normal validation errors; the inherited UpdateWebhookRequest
behavior must receive the same protection.
In `@app/Http/Requests/Api/Application/Webhooks/UpdateWebhookRequest.php`:
- Around line 42-45: Update hasServer() to detect whether server_id was
explicitly supplied, including null, and avoid falling back to
record()->server_id in that case; only use the stored server ID when the field
is omitted, so an explicit null is rejected for server-scoped webhooks.
- Around line 18-25: Update the validation logic in UpdateWebhookRequest::rules
so required rules for schema payload fields are relaxed when PATCH omits
payload, while preserving full payload validation whenever payload is present.
Do not broadly convert required rules for unrelated fields such as name,
endpoint, or events; target the payload validation rules specifically. Add a
regression test using a schema with a required payload field and a PATCH that
changes only another attribute.
- Around line 33-57: Update the PATCH handling around resolveScope(),
resolveType(), and persistence so inferred scope is stored when server_id is
provided without scope, and inferred type is stored when endpoint is provided
without type. Merge these resolved values into the validated attributes before
saving, preserving explicitly supplied scope and type values and keeping
validation, event selection, and delivery consistent.
In `@app/Jobs/ProcessWebhook.php`:
- Around line 59-60: Update the failure reporting in ProcessWebhook so it no
longer includes the full webhookConfiguration->endpoint. Report the webhook
configuration ID together with a redacted host or URL, ensuring credentials and
bearer tokens cannot reach logs or error reporting.
In `@app/Transformers/Api/Application/WebhookConfigurationTransformer.php`:
- Around line 71-79: Replace the unbounded includeDeliveries implementation with
a bounded, paginated webhook-deliveries endpoint or a documented relation that
returns only recent deliveries. Ensure the query excludes soft-deleted rows and
applies explicit ordering and limits, rather than loading the full webhooks
history through getRelation('webhooks'); update the surrounding transformer/API
wiring to use that bounded source.
In `@resources/views/filament/components/webhook-preview-section.blade.php`:
- Around line 76-81: Update the preview payload construction in the foreach over
$previewFields to resolve dotted field names as nested paths within formData,
rather than accessing them as literal top-level keys. Preserve null fallback for
missing values and continue dispatching the resolved payload through
webhook-form-changed.
---
Nitpick comments:
In `@app/Extensions/Webhooks/Schemas/BaseSchema.php`:
- Around line 122-125: Update the getTimeout() method to explicitly cast the
panel.webhook.timeout configuration value to int before returning it, while
retaining the existing 30 default.
In `@app/Extensions/Webhooks/WebhookTypeService.php`:
- Around line 18-25: Update WebhookTypeService::register to log a warning when
array_key_exists detects a duplicate schema id before returning, including the
conflicting id in the warning; preserve the existing behavior of retaining the
original schema and skipping replacement.
In `@app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php`:
- Around line 43-59: The webhook form-data mutation logic is duplicated across
the server pages. In
app/Filament/Server/Resources/Webhooks/Pages/EditWebhook.php lines 43-59, move
mutateFormDataBeforeSave and mutateFormDataBeforeFill into a shared
MutatesWebhookFormData trait and use it in EditWebhook; in
app/Filament/Server/Resources/Webhooks/Pages/CreateWebhook.php lines 47-49, use
the same trait and invoke its save-time mutation helper after server and scope
assignments. Apply the shared trait to the corresponding admin create and edit
pages as well.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f7e06f1d-74a0-45dc-9940-da85fb7d2820
📒 Files selected for processing (40)
app/Enums/WebhookType.phpapp/Extensions/Webhooks/Schemas/BaseSchema.phpapp/Extensions/Webhooks/Schemas/RegularSchema.phpapp/Extensions/Webhooks/Schemas/WebhookSchemaInterface.phpapp/Extensions/Webhooks/WebhookForm.phpapp/Extensions/Webhooks/WebhookPreview.phpapp/Extensions/Webhooks/WebhookTypeService.phpapp/Facades/WebhookTypes.phpapp/Filament/Admin/Resources/Webhooks/Pages/CreateWebhookConfiguration.phpapp/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.phpapp/Filament/Admin/Resources/Webhooks/WebhookResource.phpapp/Filament/Server/Resources/Webhooks/Pages/CreateWebhook.phpapp/Filament/Server/Resources/Webhooks/Pages/EditWebhook.phpapp/Filament/Server/Resources/Webhooks/WebhookResource.phpapp/Http/Controllers/Api/Application/Webhooks/WebhookController.phpapp/Http/Requests/Api/Application/Webhooks/DeleteWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/GetWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/StoreWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/TestWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/UpdateWebhookRequest.phpapp/Jobs/ProcessWebhook.phpapp/Livewire/DiscordPreview.phpapp/Models/ApiKey.phpapp/Models/WebhookConfiguration.phpapp/Providers/Extensions/WebhookServiceProvider.phpapp/Providers/Filament/FilamentServiceProvider.phpapp/Transformers/Api/Application/WebhookConfigurationTransformer.phpapp/Transformers/Api/Application/WebhookDeliveryTransformer.phpbootstrap/providers.phpconfig/panel.phpdatabase/Factories/WebhookConfigurationFactory.phpdatabase/migrations/2025_04_09_015500_add_webhook_configurations_type_column.phplang/en/admin/webhook.phpresources/css/discord-preview.cssresources/views/filament/components/webhook-preview-section.blade.phpresources/views/livewire/discord-preview.blade.phproutes/api-application.phptests/Integration/Api/Application/ApplicationApiIntegrationTestCase.phptests/Integration/Api/Application/WebhookControllerTest.phptests/Integration/Webhooks/WebhookSchemaExtensionTest.php
💤 Files with no reviewable changes (5)
- app/Providers/Filament/FilamentServiceProvider.php
- resources/views/livewire/discord-preview.blade.php
- app/Livewire/DiscordPreview.php
- resources/css/discord-preview.css
- app/Enums/WebhookType.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Http/Requests/Api/Application/Webhooks/UpdateWebhookRequest.php`:
- Around line 37-39: Update
app/Http/Requests/Api/Application/Webhooks/UpdateWebhookRequest.php at lines
37-39, 47-57, and 93-103 to construct the effective webhook configuration from
stored values plus validated request data before persisting changes: apply
payload rules when the effective type changes even if payload is omitted,
validate retained stored events when the effective scope changes, and save
discriminator changes only after effective-state validation succeeds. Add
regression coverage in
tests/Integration/Api/Application/WebhookControllerTest.php lines 195-264 for
changing to a required-payload type without payload and changing to server scope
while retaining global-only events.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ffbdf4dc-f7bb-46b0-929b-c07989debd05
📒 Files selected for processing (21)
app/Extensions/Webhooks/Schemas/BaseSchema.phpapp/Extensions/Webhooks/WebhookForm.phpapp/Extensions/Webhooks/WebhookPreview.phpapp/Extensions/Webhooks/WebhookTypeService.phpapp/Facades/WebhookTypes.phpapp/Filament/Admin/Resources/Webhooks/Pages/CreateWebhookConfiguration.phpapp/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.phpapp/Filament/Admin/Resources/Webhooks/WebhookResource.phpapp/Filament/Server/Resources/Webhooks/Pages/CreateWebhook.phpapp/Filament/Server/Resources/Webhooks/Pages/EditWebhook.phpapp/Filament/Server/Resources/Webhooks/WebhookResource.phpapp/Http/Controllers/Api/Application/Webhooks/WebhookController.phpapp/Http/Requests/Api/Application/Webhooks/GetWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/StoreWebhookRequest.phpapp/Http/Requests/Api/Application/Webhooks/UpdateWebhookRequest.phpapp/Jobs/ProcessWebhook.phpapp/Traits/Filament/MutatesWebhookFormData.phpapp/Transformers/Api/Application/WebhookConfigurationTransformer.phplang/en/admin/webhook.phpresources/views/filament/components/webhook-preview-section.blade.phptests/Integration/Api/Application/WebhookControllerTest.php
🚧 Files skipped from review as they are similar to previous changes (11)
- resources/views/filament/components/webhook-preview-section.blade.php
- app/Facades/WebhookTypes.php
- app/Transformers/Api/Application/WebhookConfigurationTransformer.php
- app/Filament/Server/Resources/Webhooks/WebhookResource.php
- app/Filament/Admin/Resources/Webhooks/Pages/EditWebhookConfiguration.php
- lang/en/admin/webhook.php
- app/Extensions/Webhooks/Schemas/BaseSchema.php
- app/Http/Controllers/Api/Application/Webhooks/WebhookController.php
- app/Jobs/ProcessWebhook.php
- app/Filament/Admin/Resources/Webhooks/Pages/CreateWebhookConfiguration.php
- app/Extensions/Webhooks/WebhookForm.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes some changes that I think are necessary.
I removed discord webhooks to make them as a plugin myself so I can keep them Up to Date easier. Also thanks to this, people can make their own plugins of Webhooks, should be flexible for people and everything. Also allows adding new events.
Until this PR is merged I won't upload webhooks plugin, so if you need Discord Webhooks plugin to test this PR let me know on DM or somewhere.
PD: This PR was done with the help of AI, so if somewhere is really slop please let me know. Did it so we can just stop having problems with them...