delete subdomain on server delete #92
Conversation
📝 WalkthroughWalkthroughAdds a Server deletion hook in the Subdomains plugin provider that iterates related subdomains and attempts to delete each one, catching and reporting any exceptions during deletion. Changes
Sequence Diagram(s)sequenceDiagram
participant Server
participant Provider
participant Subdomain
participant Reporter
Server->>Provider: deleting event
Provider->>Subdomain: fetch related subdomains
loop for each subdomain
Provider->>Subdomain: call delete()
alt delete succeeds
Subdomain-->>Provider: deletion confirmed
else delete throws Exception
Subdomain-->>Provider: throws Exception
Provider->>Reporter: reportException(Exception)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 Fix all issues with AI agents
In `@subdomains/src/Providers/SubdomainsPluginProvider.php`:
- Around line 37-43: The Server::deleting listener currently has an empty catch
block which breaks Pint's braces_position rule and swallows exceptions; update
the listener around Server::deleting so the catch block uses proper braces on
their own lines and does not silently discard errors—log the caught Exception
(for example via Log::error or logger()->error) including context (Server id and
$subdomain->id/name) when handling exceptions thrown by $subdomain->delete();
keep deletion resilient by catching and logging but not rethrowing.
🧹 Nitpick comments (2)
subdomains/src/Providers/SubdomainsPluginProvider.php (2)
38-38: PHPStan cannot resolve dynamically registered relation — consider suppressing.The
subdomains()relation is dynamically registered viaresolveRelationUsingon line 35, so the code will work at runtime. However, PHPStan cannot statically analyze dynamic relations, causing the CI failures.Since
Serveris an external model you don't control, consider suppressing the PHPStan error inline:♻️ Proposed fix
Server::deleting(function (Server $server) { + /** `@phpstan-ignore` method.notFound */ foreach ($server->subdomains()->get() as $subdomain) {Alternatively, add the error to your PHPStan baseline if you prefer not to use inline ignores.
10-12: Consider reordering imports per PSR-12 conventions.
Exceptionis a global PHP class and conventionally should be grouped before framework imports.♻️ Suggested ordering
use App\Filament\Admin\Resources\Servers\ServerResource; use App\Models\Role; use App\Models\Server; use Boy132\Subdomains\Filament\Admin\Resources\Servers\RelationManagers\SubdomainRelationManager; use Boy132\Subdomains\Models\Subdomain; +use Exception; use Illuminate\Support\Facades\Http; -use Exception; use Illuminate\Support\ServiceProvider;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
subdomains/src/Providers/SubdomainsPluginProvider.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-04T11:13:45.197Z
Learnt from: HarlequinSin
Repo: pelican-dev/plugins PR: 67
File: subdomains/src/Models/Subdomain.php:49-65
Timestamp: 2026-01-04T11:13:45.197Z
Learning: In Subdomain model creating hook at subdomains/src/Models/Subdomain.php, the code uses setRelation('server', Filament::getTenant()) because the model hasn't been saved yet and load() won't work during creation. This is intentional for Filament-context creation.
Applied to files:
subdomains/src/Providers/SubdomainsPluginProvider.php
🧬 Code graph analysis (1)
subdomains/src/Providers/SubdomainsPluginProvider.php (3)
subdomains/src/Models/Subdomain.php (1)
server(48-51)subdomains/src/Models/CloudflareDomain.php (1)
subdomains(31-34)robo-avatars/src/RoboAvatarsSchema.php (1)
get(20-23)
🪛 GitHub Actions: Lint
subdomains/src/Providers/SubdomainsPluginProvider.php
[error] 1-1: vendor/bin/pint --test failed with exit code 1 due to style issue: braces_position (Laravel).
🪛 GitHub Check: PHPStan (8.2)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.3)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.4)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.5)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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)
subdomains/src/Providers/SubdomainsPluginProvider.php (1)
5-12: Fix ordered imports to satisfy Pint.The lint pipeline is failing on
ordered_imports. Reorder theusestatements alphabetically to pass Pint.✅ Proposed fix
use App\Filament\Admin\Resources\Servers\ServerResource; use App\Models\Role; use App\Models\Server; use Boy132\Subdomains\Filament\Admin\Resources\Servers\RelationManagers\SubdomainRelationManager; use Boy132\Subdomains\Models\Subdomain; -use Illuminate\Support\Facades\Http; use Exception; +use Illuminate\Support\Facades\Http; use Illuminate\Support\ServiceProvider;
🤖 Fix all issues with AI agents
In `@subdomains/src/Providers/SubdomainsPluginProvider.php`:
- Around line 35-39: PHPStan can't see the runtime relation registered via
Server::resolveRelationUsing() so add a static declaration for the relation to
satisfy analysis: add a `@method` HasMany subdomains() annotation to the Server
model class (App\Models\Server) or alternatively create a phpstan stub declaring
method subdomains(): HasMany; if you prefer suppressing the error instead, add
an ignoreErrors entry for larastan.noMethodFound with a message matching 'Call
to an undefined method.*subdomains' in phpstan.neon; reference the
Server::resolveRelationUsing and Server::subdomains() symbols when making the
change.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
subdomains/src/Providers/SubdomainsPluginProvider.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-04T11:13:45.197Z
Learnt from: HarlequinSin
Repo: pelican-dev/plugins PR: 67
File: subdomains/src/Models/Subdomain.php:49-65
Timestamp: 2026-01-04T11:13:45.197Z
Learning: In Subdomain model creating hook at subdomains/src/Models/Subdomain.php, the code uses setRelation('server', Filament::getTenant()) because the model hasn't been saved yet and load() won't work during creation. This is intentional for Filament-context creation.
Applied to files:
subdomains/src/Providers/SubdomainsPluginProvider.php
🧬 Code graph analysis (1)
subdomains/src/Providers/SubdomainsPluginProvider.php (2)
subdomains/src/Models/Subdomain.php (1)
server(48-51)subdomains/src/Models/CloudflareDomain.php (1)
subdomains(31-34)
🪛 GitHub Actions: Lint
subdomains/src/Providers/SubdomainsPluginProvider.php
[error] 1-1: Pint: ordered_imports violation detected in SubdomainsPluginProvider.php. Ensure imports are alphabetically ordered.
🪛 GitHub Check: PHPStan (8.2)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.3)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.4)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
🪛 GitHub Check: PHPStan (8.5)
subdomains/src/Providers/SubdomainsPluginProvider.php
[failure] 38-38:
Call to an undefined method App\Models\Server::subdomains().
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Boy132
left a comment
Boy132
left a comment
There was a problem hiding this comment.
Small nitpick, otherwise looks good.
Co-authored-by: Boy132 <Boy132@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@subdomains/src/Providers/SubdomainsPluginProvider.php`:
- Line 10: Replace the current Exception-only handling with Throwable so PHP
Errors/TypeErrors don't abort deletion: update the import from "use Exception;"
to "use Throwable;" (or remove the Exception import and reference Throwable) and
change the catch(Exception $e) blocks that wrap the $subdomain->delete() call to
catch(Throwable $e) in the SubdomainsPluginProvider class where
$subdomain->delete() is invoked, preserving the existing logging/handling inside
the catch.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
subdomains/src/Providers/SubdomainsPluginProvider.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-04T11:13:45.197Z
Learnt from: HarlequinSin
Repo: pelican-dev/plugins PR: 67
File: subdomains/src/Models/Subdomain.php:49-65
Timestamp: 2026-01-04T11:13:45.197Z
Learning: In Subdomain model creating hook at subdomains/src/Models/Subdomain.php, the code uses setRelation('server', Filament::getTenant()) because the model hasn't been saved yet and load() won't work during creation. This is intentional for Filament-context creation.
Applied to files:
subdomains/src/Providers/SubdomainsPluginProvider.php
🧬 Code graph analysis (1)
subdomains/src/Providers/SubdomainsPluginProvider.php (2)
subdomains/src/Models/Subdomain.php (1)
server(48-51)subdomains/src/Models/CloudflareDomain.php (1)
subdomains(31-34)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Boy132
left a comment
Boy132
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
2 participants
Fixes #88
Added event listener for "deleting" event and deletes subdomain.
Not throwing exception to not interfere with server deletion.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.