Z3d0X / filament-fabricator

Block-Based Page Builder Skeleton for your Filament Apps
https://filamentphp.com/plugins/fabricator
MIT License
251 stars 48 forks source link

Add exception for runningInConsole in FilamentFabricatorServiceProvid… #160

Open yolanmees opened 1 month ago

yolanmees commented 1 month ago

Hello,

I've encountered an issue with this package, related to using models directly within Block files. The problem arises when these models are loaded from a ServiceProvider. This becomes evident during a fresh migration process, where the error "table not exists" is thrown because the system attempts to access the table while it is being migrated.

To address this issue, I've made modifications to the FilamentFabricatorServiceProvider. My approach involves wrapping certain parts of the service provider logic within an if statement that checks if the application is running in console mode. This ensures that the code within the block doesn't execute when commands are running, thereby preventing the Block files from being called prematurely.

Here's the change I've implemented:

- Route::bind('filamentFabricatorPage', function ($value) {
-     $pageModel = FilamentFabricator::getPageModel();
-     $pageUrls = FilamentFabricator::getPageUrls();
-     $value = Str::start($value, '/');
-     $pageId = array_search($value, $pageUrls);
-     return $pageModel::query()
-         ->where('id', $pageId)
-         ->firstOrFail();
- });
- $this->registerComponentsFromDirectory(
-     Layout::class,
-     config('filament-fabricator.layouts.register'),
-     config('filament-fabricator.layouts.path'),
-     config('filament-fabricator.layouts.namespace')
- );
- $this->registerComponentsFromDirectory(
-     PageBlock::class,
-     config('filament-fabricator.page-blocks.register'),
-     config('filament-fabricator.page-blocks.path'),
-     config('filament-fabricator.page-blocks.namespace')
- );

+ if (! $this->app->runningInConsole()) {
+     Route::bind('filamentFabricatorPage', function ($value) {
+         $pageModel = FilamentFabricator::getPageModel();
+         $pageUrls = FilamentFabricator::getPageUrls();
+         $value = Str::start($value, '/');
+         $pageId = array_search($value, $pageUrls);
+         return $pageModel::query()
+             ->where('id', $pageId)
+             ->firstOrFail();
+     });
+     $this->registerComponentsFromDirectory(
+         Layout::class,
+         config('filament-fabricator.layouts.register'),
+         config('filament-fabricator.layouts.path'),
+         config('filament-fabricator.layouts.namespace')
+     );
+     $this->registerComponentsFromDirectory(
+         PageBlock::class,
+         config('filament-fabricator.page-blocks.register'),
+         config('filament-fabricator.page-blocks.path'),
+         config('filament-fabricator.page-blocks.namespace')
+     );
+ }

With this adjustment, the service provider skips this code when Laravel commands are executed, effectively resolving the migration error without impacting the operational functionality of the package.

I hope this fix can be helpful for others facing the same issue and I believe it could be a valuable addition to the package.

Best regards, Yolan Mees

what-the-diff[bot] commented 1 month ago

PR Summary

matthewmnewman commented 4 weeks ago

You're absolutely right. Incorporating the Page Model into any Fabricator Block can lead to failure during a fresh installation because of the absence of the pages table, which cannot be executed due to the Service Provider limitation.

It's unclear whether this fix has been thoroughly tested, but addressing it would be beneficial as it can create complications when initiating a new project with pre-existing blocks.