fix: domain autoload#134
Conversation
WalkthroughReplaces ad-hoc request-based tenant resolution with Filament's centralized tenant APIs across event components, middleware, and panel providers; adjusts provider ordering and FilamentServiceProvider tenant mapping; adds tenant mounting to ParticipantDashboard and updates tests to set current Filament panel where needed. Changes
Sequence Diagram(s)sequenceDiagram
participant Req as HTTP Request
participant MW as GuestTenantIdentifier
participant Panel as Filament Panel
participant Fil as Filament API
participant Comp as Event Component
Note over Req,MW: request enters app
Req->>MW: incoming request
MW->>Panel: panel->getTenant(route('tenant'))
Panel-->>MW: Tenant
MW->>Fil: Filament::setTenant(Tenant)
Fil-->>MW: tenant set
MW->>Req: next($request)
Note over Req,Comp: component lifecycle
Req->>Comp: mount()
Comp->>Fil: filament()->getTenant()
Fil-->>Comp: Tenant
Comp->>Comp: compute view path using tenant->slug
Comp->>Fil: render view with tenant-scoped data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app-modules/events/src/Filament/Events/ParticipantDashboard.php (1)
44-47: Reuse cached tenant instead of callinggetTenant()again.
getViewData()callsfilament()->getTenant()rather than using the already-mounted$this->tenant. This is inconsistent withgetView()and makes an unnecessary additional call.protected function getViewData(): array { - return ['event' => filament()->getTenant()->events()->first()]; + return ['event' => $this->tenant->events()->first()]; }
🧹 Nitpick comments (1)
app/Providers/FilamentServiceProvider.php (1)
25-30: Hardcoded tenant mapping limits extensibility.The match expression maps slugs to a fixed set of values (
'3pontos'and'he4rt'). This works for the current tenants but requires code changes when adding new tenants.Consider externalizing tenant mappings to configuration if tenant additions are expected:
$tenantSlug = collect(config('tenants.mappings', [])) ->first(fn ($_, $pattern) => str_contains($tenantSlug, $pattern), 'he4rt');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app-modules/events/src/Filament/Events/EventLandingPage.php(1 hunks)app-modules/events/src/Filament/Events/ParticipantDashboard.php(2 hunks)app/Http/Middleware/GuestTenantIdentifier.php(1 hunks)app/Providers/Filament/EventPanelProvider.php(1 hunks)app/Providers/Filament/GuestPanelProvider.php(1 hunks)app/Providers/FilamentServiceProvider.php(2 hunks)bootstrap/providers.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/Providers/Filament/EventPanelProvider.php (2)
app-modules/provider/src/Models/Provider.php (1)
model(51-54)app-modules/tenant/src/Models/Tenant.php (1)
Tenant(23-104)
bootstrap/providers.php (1)
app/Providers/Filament/AdminPanelProvider.php (1)
AdminPanelProvider(24-75)
app-modules/events/src/Filament/Events/ParticipantDashboard.php (3)
app-modules/tenant/src/Models/Tenant.php (1)
Tenant(23-104)app-modules/events/src/Filament/Events/EventLandingPage.php (2)
mount(18-21)getView(23-30)app-modules/events/src/Filament/Shared/EventLogin.php (1)
mount(66-76)
app-modules/events/src/Filament/Events/EventLandingPage.php (2)
app-modules/events/src/Models/Talk.php (1)
tenant(66-69)app-modules/events/src/Models/EventModel.php (1)
tenant(140-143)
app/Providers/Filament/GuestPanelProvider.php (1)
app/Providers/Filament/EventPanelProvider.php (1)
panel(35-92)
🔇 Additional comments (4)
bootstrap/providers.php (1)
19-23: LGTM!The provider reordering correctly positions
AdminPanelProviderafterEventPanelProvider, ensuring tenant-aware panel initialization occurs before the default admin panel loads.app/Providers/Filament/GuestPanelProvider.php (1)
30-36: LGTM!The production-aware domain binding using
config('app.domain')is appropriate for the guest panel, which serves non-tenanted public pages. This correctly differentiates from tenant-specific domain handling inEventPanelProvider.app/Providers/FilamentServiceProvider.php (1)
47-48: LGTM!Moving the panel session persistence after tenant determination ensures the panel context is stored consistently with the resolved tenant.
app/Providers/Filament/EventPanelProvider.php (1)
39-43: TheownedTenantsrelationship exists and is properly configured.The relationship is defined in the
InteractsWithTenantstrait (line 41-44 ofapp-modules/tenant/src/Models/Concerns/InteractsWithTenants.php):public function ownedTenants(): HasMany { return $this->hasMany(Tenant::class, 'owner_id'); }The User model uses this trait, so the configuration in the EventPanelProvider is valid. The environment-aware
slugAttributecorrectly aligns production domain-based routing with local slug-based routing, and named parameters improve readability.
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-modules/events/src/Filament/Events/ParticipantDashboard.php (1)
35-42: Add null check before accessing tenant properties.If
filament()->getTenant()returnsnull, accessing$this->tenant->slugon line 37 will throw a null pointer exception before reaching theabort_unlesscheck.public function getView(): string { + abort_unless($this->tenant, 403, 'Forbidden Tenant'); + $view = sprintf('events::components.themes.%s.participant-dashboard', $this->tenant->slug); abort_unless(view()->exists($view), 403, 'Forbidden Tenant'); return $view; }
♻️ Duplicate comments (1)
app-modules/events/src/Filament/Events/ParticipantDashboard.php (1)
13-18: Type the property asTenantfor consistency.The docblock declares
@property Tenant $tenant, but the property is typed as?Model. Use the concrete type for better IDE support and type safety.-/** @property Tenant $tenant */ class ParticipantDashboard extends Page { protected Width|string|null $maxContentWidth = Width::Full; - private ?Model $tenant = null; + private ?Tenant $tenant = null;
🧹 Nitpick comments (2)
app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php (1)
118-118: Add a skip reason or TODO comment explaining why this test is skipped.Skipping tests without documentation makes it difficult to track why they were disabled and when they should be re-enabled. Consider using
->skip('reason')or adding a TODO comment.- })->skip(); + })->skip('TODO: Fix tenant context resolution for this test scenario');app-modules/events/src/Filament/Events/ParticipantDashboard.php (1)
44-47: Use$this->tenantinstead of redundantfilament()->getTenant()call.The tenant is already resolved and stored in
$this->tenantduringmount(). Using the cached property improves consistency and avoids redundant resolution.protected function getViewData(): array { - return ['event' => filament()->getTenant()->events()->first()]; + return ['event' => $this->tenant?->events()->first()]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
app-modules/events/src/Filament/Events/ParticipantDashboard.php(2 hunks)app-modules/events/tests/Feature/Filament/App/Talks/CreateTalkTest.php(1 hunks)app-modules/feedback/tests/Feature/Filament/Admin/Feedback/CreateFeedbackTest.php(1 hunks)app-modules/feedback/tests/Feature/Filament/Admin/Feedback/DeleteFeedbackTest.php(1 hunks)app-modules/feedback/tests/Feature/Filament/Admin/Feedback/EditFeedbackTest.php(2 hunks)app-modules/feedback/tests/Feature/Filament/Admin/Feedback/FeedbackResourceTest.php(1 hunks)app/Providers/Filament/EventPanelProvider.php(1 hunks)app/Providers/Filament/GuestPanelProvider.php(1 hunks)bootstrap/providers.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Providers/Filament/GuestPanelProvider.php
🧰 Additional context used
🧬 Code graph analysis (1)
bootstrap/providers.php (1)
app/Providers/Filament/AdminPanelProvider.php (1)
AdminPanelProvider(24-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Perform Pest Tests / Run
- GitHub Check: Perform Rector Check / Run
🔇 Additional comments (7)
app-modules/feedback/tests/Feature/Filament/Admin/Feedback/FeedbackResourceTest.php (1)
8-12: Initializing the admin panel before asserting resources looks correctSetting
filament()->setCurrentPanel('admin');before callingFilament::getResources()ensures the resource list is evaluated in the correct panel context and aligns with the rest of the PR’s panel-handling changes.app-modules/feedback/tests/Feature/Filament/Admin/Feedback/CreateFeedbackTest.php (1)
13-35: Admin panel initialization is correctly placed at test startCalling
filament()->setCurrentPanel('admin');as the first line in the test ensures subsequent Livewire interactions withCreateFeedbackrun in the intended admin panel context.app-modules/feedback/tests/Feature/Filament/Admin/Feedback/DeleteFeedbackTest.php (1)
12-26: Correctly scoping the delete flow to the admin panelUsing
filament()->setCurrentPanel('admin');before instantiatingEditFeedbackensures the delete action executes within the admin panel context, consistent with the other feedback admin tests.app-modules/feedback/tests/Feature/Filament/Admin/Feedback/EditFeedbackTest.php (2)
11-26: Explicit admin panel context for loading the edit page is appropriateInitializing with
filament()->setCurrentPanel('admin');before creating the feedback and mountingEditFeedbackensures the schema state assertions reflect the admin panel configuration.
28-50: Admin panel initialization before updating feedback is consistent and correctSetting
filament()->setCurrentPanel('admin');at the beginning of the update test guarantees the form fill andsavecall operate within the admin panel context, matching the rest of the feedback admin suite.app/Providers/Filament/EventPanelProvider.php (1)
39-46: LGTM - tenant configuration with environment-aware routing.The named parameters improve readability. The conditional logic correctly distinguishes between production (domain-based routing with empty path) and non-production (slug-based routing with
/eventpath). This aligns with the tenantDomain configuration on line 46.bootstrap/providers.php (1)
18-24: Verify provider ordering impact on panel resolution.The
AdminPanelProvider(which sets->default()on its panel) has been repositioned betweenGuestPanelProviderandEventPanelProvider. Provider registration order can affect how Filament resolves the current panel, especially during domain-based tenant identification.Please confirm this ordering correctly prioritizes panel resolution for the domain autoload fix.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Participant dashboards and event views now respect the active tenant/context so users see event data for the correct organization. * **Refactor** * Tenant resolution and panel/provider initialization streamlined to use Filament's tenant resolver and consistent panel/theme mapping; panel ordering adjusted for predictable behavior. * **Tests** * Filament test setups updated to initialize the admin panel context; one test marked as skipped. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.