Allow each domains to have its own routes to avoid duplicate route registration #12462

declum · 2024-04-23T16:27:40Z

Description

if we register the Panel with multiple domains, php artisan optimize fails with Unable to prepare route [login] for serialization. Another route has already been assigned name [filament.dashboard.auth.login].

It is because panel logic is registrying routes with the same name for each domain. This PR tries to fix that.

Closes: #11837

Visual changes

Nil

Functional changes

Code style has been fixed by running the composer cs command.
Changes have been tested to not break existing functionality.
Documentation is up-to-date.

danharrin

Please make it so this is only used on panels with > 1 domain, not > 0 domains

declum · 2024-04-24T07:52:22Z

@danharrin Changed it. Please have look.

danharrin · 2024-04-24T07:53:58Z

I have a feeling you didnt test that, the routes file still needs updating as its adding the extra param for 1 domain. Please manually test this with one domain or write a test for our suite.

declum · 2024-04-24T09:15:16Z

I have a feeling you didnt test that, the routes file still needs updating as its adding the extra param for 1 domain. Please manually test this with one domain or write a test for our suite.

Ya didn't test after changing to < 1 👍🏽

How do I write test for routes? Is there a sample reference in the repo?

danharrin · 2024-04-24T09:52:33Z

The only test we really need is to hit the dashboard and check it renders, right? Set up a new panel provider with domain configuration. Basically to make sure the routing doesnt error out

danharrin · 2024-04-24T09:52:49Z

Maybe have one panel provider for single domain, one for multi domains

declum · 2024-04-24T12:35:56Z

Bellow are the tests performed.

With no domain: Verified that site and admin panel is accessible with domain 127.0.0.1:8000
With single domain: Verified that site is accessible with domain 127.0.0.1:8000, and admin panel is only available at the given domain
With multiple domains: Verified that site is accessible with domain 127.0.0.1:8000, and admin panel is only available at the given domains (first.* and second.*)

Below are the output of php artisan route:list for the above scenarios.

Tested the fix with no domain

% php artisan route:list

GET|HEAD / ...................................................................................................................................................................................................................................................................................................................................................................
POST _ignition/execute-solution ............................................................................................................................................................................................................................................................ ignition.executeSolution › Spatie\LaravelIgnition › ExecuteSolutionController
GET|HEAD _ignition/health-check ........................................................................................................................................................................................................................................................................ ignition.healthCheck › Spatie\LaravelIgnition › HealthCheckController
POST _ignition/update-config ..................................................................................................................................................................................................................................................................... ignition.updateConfig › Spatie\LaravelIgnition › UpdateConfigController
GET|HEAD admin ..................................................................................................................................................................................................................................................................................................... filament.admin.pages.dashboard › Filament\Pages\Dashboard
GET|HEAD admin/login ................................................................................................................................................................................................................................................................................................... filament.admin.auth.login › Filament\Pages\Auth\Login
POST admin/logout ........................................................................................................................................................................................................................................................................... filament.admin.auth.logout › Filament\Http\Controllers\Auth\LogoutController
GET|HEAD filament/exports/{export}/download ............................................................................................................................................................................................................................................. filament.exports.download › Filament\Actions\Exports\Http\Controllers\DownloadExport
GET|HEAD filament/imports/{import}/failed-rows/download ........................................................................................................................................................................................................... filament.imports.failed-rows.download › Filament\Actions\Imports\Http\Controllers\DownloadImportFailureCsv
GET|HEAD livewire/livewire.js .................................................................................................................................................................................................................................................................................... Livewire\Mechanisms › FrontendAssets@returnJavaScriptAsFile
GET|HEAD livewire/livewire.min.js.map .............................................................................................................................................................................................................................................................................................. Livewire\Mechanisms › FrontendAssets@maps
GET|HEAD livewire/preview-file/{filename} ........................................................................................................................................................................................................................................................... livewire.preview-file › Livewire\Features › FilePreviewController@handle
POST livewire/update ................................................................................................................................................................................................................................................................................. livewire.update › Livewire\Mechanisms › HandleRequests@handleUpdate
POST livewire/upload-file ......................................................................................................................................................................................................................................................................... livewire.upload-file › Livewire\Features › FileUploadController@handle
GET|HEAD up ..................................................................................................................................................................................................................................................................................................................................................................

With Single Domain ->domain('single.127-0-0-1.sslip.io')

`% php artisan route:list

GET|HEAD / ...................................................................................................................................................................................................................................................................................................................................................................
POST _ignition/execute-solution ............................................................................................................................................................................................................................................................ ignition.executeSolution › Spatie\LaravelIgnition › ExecuteSolutionController
GET|HEAD _ignition/health-check ........................................................................................................................................................................................................................................................................ ignition.healthCheck › Spatie\LaravelIgnition › HealthCheckController
POST _ignition/update-config ..................................................................................................................................................................................................................................................................... ignition.updateConfig › Spatie\LaravelIgnition › UpdateConfigController
GET|HEAD admin ..................................................................................................................................................................................................................................................................................................... filament.admin.pages.dashboard › Filament\Pages\Dashboard
GET|HEAD admin/login ................................................................................................................................................................................................................................................................................................... filament.admin.auth.login › Filament\Pages\Auth\Login
POST admin/logout ........................................................................................................................................................................................................................................................................... filament.admin.auth.logout › Filament\Http\Controllers\Auth\LogoutController
GET|HEAD filament/exports/{export}/download ............................................................................................................................................................................................................................................. filament.exports.download › Filament\Actions\Exports\Http\Controllers\DownloadExport
GET|HEAD filament/imports/{import}/failed-rows/download ........................................................................................................................................................................................................... filament.imports.failed-rows.download › Filament\Actions\Imports\Http\Controllers\DownloadImportFailureCsv
GET|HEAD livewire/livewire.js .................................................................................................................................................................................................................................................................................... Livewire\Mechanisms › FrontendAssets@returnJavaScriptAsFile
GET|HEAD livewire/livewire.min.js.map .............................................................................................................................................................................................................................................................................................. Livewire\Mechanisms › FrontendAssets@maps
GET|HEAD livewire/preview-file/{filename} ........................................................................................................................................................................................................................................................... livewire.preview-file › Livewire\Features › FilePreviewController@handle
POST livewire/update ................................................................................................................................................................................................................................................................................. livewire.update › Livewire\Mechanisms › HandleRequests@handleUpdate
POST livewire/upload-file ......................................................................................................................................................................................................................................................................... livewire.upload-file › Livewire\Features › FileUploadController@handle
GET|HEAD up ..................................................................................................................................................................................................................................................................................................................................................................
`

With multiple domains ->domains(['first.127-0-0-1.sslip.io', 'second.127-0-0-1.sslip.io'])

`% php artisan route:list

GET|HEAD / ...................................................................................................................................................................................................................................................................................................................................................................
POST _ignition/execute-solution ............................................................................................................................................................................................................................................................ ignition.executeSolution › Spatie\LaravelIgnition › ExecuteSolutionController
GET|HEAD _ignition/health-check ........................................................................................................................................................................................................................................................................ ignition.healthCheck › Spatie\LaravelIgnition › HealthCheckController
POST _ignition/update-config ..................................................................................................................................................................................................................................................................... ignition.updateConfig › Spatie\LaravelIgnition › UpdateConfigController
GET|HEAD first.127-0-0-1.sslip.io/admin ................................................................................................................................................................................................................................................... filament.admin.first.127-0-0-1.sslip.io.pages.dashboard › Filament\Pages\Dashboard
GET|HEAD second.127-0-0-1.sslip.io/admin ................................................................................................................................................................................................................................................. filament.admin.second.127-0-0-1.sslip.io.pages.dashboard › Filament\Pages\Dashboard
GET|HEAD first.127-0-0-1.sslip.io/admin/login ................................................................................................................................................................................................................................................. filament.admin.first.127-0-0-1.sslip.io.auth.login › Filament\Pages\Auth\Login
GET|HEAD second.127-0-0-1.sslip.io/admin/login ............................................................................................................................................................................................................................................... filament.admin.second.127-0-0-1.sslip.io.auth.login › Filament\Pages\Auth\Login
POST first.127-0-0-1.sslip.io/admin/logout ......................................................................................................................................................................................................................... filament.admin.first.127-0-0-1.sslip.io.auth.logout › Filament\Http\Controllers\Auth\LogoutController
POST second.127-0-0-1.sslip.io/admin/logout ....................................................................................................................................................................................................................... filament.admin.second.127-0-0-1.sslip.io.auth.logout › Filament\Http\Controllers\Auth\LogoutController
GET|HEAD filament/exports/{export}/download ............................................................................................................................................................................................................................................. filament.exports.download › Filament\Actions\Exports\Http\Controllers\DownloadExport
GET|HEAD filament/imports/{import}/failed-rows/download ........................................................................................................................................................................................................... filament.imports.failed-rows.download › Filament\Actions\Imports\Http\Controllers\DownloadImportFailureCsv
GET|HEAD livewire/livewire.js .................................................................................................................................................................................................................................................................................... Livewire\Mechanisms › FrontendAssets@returnJavaScriptAsFile
GET|HEAD livewire/livewire.min.js.map .............................................................................................................................................................................................................................................................................................. Livewire\Mechanisms › FrontendAssets@maps
GET|HEAD livewire/preview-file/{filename} ........................................................................................................................................................................................................................................................... livewire.preview-file › Livewire\Features › FilePreviewController@handle
POST livewire/update ................................................................................................................................................................................................................................................................................. livewire.update › Livewire\Mechanisms › HandleRequests@handleUpdate
POST livewire/upload-file ......................................................................................................................................................................................................................................................................... livewire.upload-file › Livewire\Features › FileUploadController@handle
GET|HEAD up ..................................................................................................................................................................................................................................................................................................................................................................
`

declum · 2024-04-24T13:12:31Z

@danharrin Please check the pest tests. if it looks good. My first pest test. Manually i have verified that the app is working for NoDomain, SingleDomain and MultiDomain.

Only missing part in unit tests is how HasRoute handles the route resolution in QueueWorker and ArtisanCommand as triggers. (like calling routes for putting links in emails or notificaitons). For now, if the request doesnt have host, it fall back to first domain (assuming that to be the primary domain, hope app_url might fix)

% ./vendor/bin/pest tests/src/Panels/Routes/SingleAndMultiDomainTest.php

PASS Tests\src\Panels\Routes\SingleAndMultiDomainTest
✓ it admin panel without any domain should resolve to default routes 0.08s
✓ it admin panel with single domain should resolve to default routes 0.01s
✓ it admin panel with multi domain should resolve to their respective routes 0.01s

Tests: 3 passed (6 assertions)
Duration: 0.14s

declum · 2024-04-24T13:52:10Z

If i run the tests in isolation all 3 new tests passes. But if i run alll tests with composer test, it fails, all of are with error Route [filament.multi-domain.localhost.auth.logout] not defined.

declum · 2024-04-24T14:08:45Z

Fixed the unit tests by modifying HasRoutes to decide based on app()->runningInConsole().

Also, I have set the current Panel to admin for AuthTests (to avoid picking a random Panel, in my case, LoginTest picked multi-domain panel though it is not intended)

danharrin · 2024-04-28T08:02:43Z

Refactored to only use the first domain while testing, since it could be dangerous to assume the current domain in jobs etc, which now needs to be manually specified

declum · 2024-04-29T01:01:49Z

Refactored to only use the first domain while testing, since it could be dangerous to assume the current domain in jobs etc, which now needs to be manually specified

@danharrin Instead of statically setting, shall we use primaryDomain as a field in each Panel itself with Closure?

danharrin · 2024-04-29T08:45:02Z

If you want to make an additional PR, sure, but I don't mind the existing solution

declum added 2 commits April 23, 2024 21:53

Update web.php

22a2142

Update HasRoutes.php

d948a3a

danharrin requested changes Apr 24, 2024

View reviewed changes

danharrin added the bug Something isn't working label Apr 24, 2024

danharrin added this to the v3 milestone Apr 24, 2024

danharrin self-assigned this Apr 24, 2024

Update HasRoutes.php

f95742e

declum closed this Apr 24, 2024

declum reopened this Apr 24, 2024

declum added 2 commits April 24, 2024 15:45

Update web.php

9212ea8

Fix the route.php

58eef27

declum added 2 commits April 24, 2024 18:25

Fix the phpstan

704a937

unit tests

2d65568

unit tests

e10e79f

declum added 2 commits April 24, 2024 19:32

unit tests

8474bce

fixes for unitest failures

80e24da

composer cs

4f7e125

declum requested a review from danharrin April 24, 2024 14:17

danharrin added 2 commits April 28, 2024 08:17

Update LoginTest.php

bd87b2f

refactor

bab4f34

danharrin merged commit 5a7df73 into filamentphp:3.x Apr 28, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow each domains to have its own routes to avoid duplicate route registration #12462

Allow each domains to have its own routes to avoid duplicate route registration #12462

declum commented Apr 23, 2024 •

edited

Loading

danharrin left a comment •

edited

Loading

declum commented Apr 24, 2024

danharrin commented Apr 24, 2024

declum commented Apr 24, 2024 •

edited

Loading

danharrin commented Apr 24, 2024

danharrin commented Apr 24, 2024

declum commented Apr 24, 2024 •

edited

Loading

declum commented Apr 24, 2024 •

edited

Loading

declum commented Apr 24, 2024

declum commented Apr 24, 2024 •

edited

Loading

danharrin commented Apr 28, 2024

declum commented Apr 29, 2024

danharrin commented Apr 29, 2024

Allow each domains to have its own routes to avoid duplicate route registration #12462

Allow each domains to have its own routes to avoid duplicate route registration #12462

Conversation

declum commented Apr 23, 2024 • edited Loading

Description

Visual changes

Functional changes

danharrin left a comment • edited Loading

Choose a reason for hiding this comment

declum commented Apr 24, 2024

danharrin commented Apr 24, 2024

declum commented Apr 24, 2024 • edited Loading

danharrin commented Apr 24, 2024

danharrin commented Apr 24, 2024

declum commented Apr 24, 2024 • edited Loading

declum commented Apr 24, 2024 • edited Loading

declum commented Apr 24, 2024

declum commented Apr 24, 2024 • edited Loading

danharrin commented Apr 28, 2024

declum commented Apr 29, 2024

danharrin commented Apr 29, 2024

declum commented Apr 23, 2024 •

edited

Loading

danharrin left a comment •

edited

Loading

declum commented Apr 24, 2024 •

edited

Loading

declum commented Apr 24, 2024 •

edited

Loading

declum commented Apr 24, 2024 •

edited

Loading

declum commented Apr 24, 2024 •

edited

Loading