From ec6b95d8ac36d506b60541cf23b57efb018c2bd9 Mon Sep 17 00:00:00 2001 From: Sam Maosa Date: Sat, 13 Jun 2026 17:37:14 +0300 Subject: [PATCH 1/4] test: add v5 regression coverage and fix widget access recursion Cover nwidart v13 facade usage, module macros, Resource PHP 8.4 compatibility, and CanAccessTrait behavior, and stop delegating to package parent classes that re-enter the trait with the wrong module name. Co-authored-by: Cursor --- src/Traits/CanAccessTrait.php | 12 ++- tests/Support/CreatesTestModules.php | 85 +++++++++++++++++++ tests/TestCase.php | 18 +++- tests/Unit/CanAccessTraitTest.php | 45 ++++++++++ tests/Unit/ModuleMacrosTest.php | 32 +++++++ tests/Unit/ModulesServiceProviderBootTest.php | 55 ++++++++++++ tests/Unit/ResourceClassTest.php | 18 ++++ 7 files changed, 258 insertions(+), 7 deletions(-) create mode 100644 tests/Support/CreatesTestModules.php create mode 100644 tests/Unit/CanAccessTraitTest.php create mode 100644 tests/Unit/ModuleMacrosTest.php create mode 100644 tests/Unit/ModulesServiceProviderBootTest.php create mode 100644 tests/Unit/ResourceClassTest.php diff --git a/src/Traits/CanAccessTrait.php b/src/Traits/CanAccessTrait.php index 862e722e..8143ae73 100644 --- a/src/Traits/CanAccessTrait.php +++ b/src/Traits/CanAccessTrait.php @@ -21,9 +21,15 @@ public static function canAccess(): bool static::getCurrentModuleName() )->isEnabled(); $parentClass = get_parent_class(static::class); - $parentAccess = is_string($parentClass) && method_exists($parentClass, 'canAccess') - ? $parentClass::canAccess() - : true; + $parentAccess = true; + + if ( + is_string($parentClass) + && str_starts_with($parentClass, 'Filament\\') + && method_exists($parentClass, 'canAccess') + ) { + $parentAccess = $parentClass::canAccess(); + } if ($isModuleEnabled && $parentAccess) { return true; diff --git a/tests/Support/CreatesTestModules.php b/tests/Support/CreatesTestModules.php new file mode 100644 index 00000000..e0d2266a --- /dev/null +++ b/tests/Support/CreatesTestModules.php @@ -0,0 +1,85 @@ +workbenchPath('Modules'); + } + + protected function resetModulesDirectory(): void + { + $modulesPath = $this->modulesPath(); + + if (is_dir($modulesPath)) { + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($modulesPath, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST, + ); + + foreach ($iterator as $file) { + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + } else { + mkdir($modulesPath, 0755, true); + } + + $statusesFile = $this->workbenchPath('modules_statuses.json'); + + if (file_exists($statusesFile)) { + unlink($statusesFile); + } + + $this->clearModuleRepositoryCache(); + } + + protected function createTestModule(string $name = 'Blog', bool $enabled = true): LaravelModule + { + $modulePath = $this->modulesPath() . DIRECTORY_SEPARATOR . $name; + + if (! is_dir($modulePath)) { + mkdir($modulePath . DIRECTORY_SEPARATOR . 'app', 0755, true); + file_put_contents($modulePath . DIRECTORY_SEPARATOR . 'module.json', json_encode([ + 'name' => $name, + 'alias' => strtolower($name), + 'description' => '', + 'keywords' => [], + 'priority' => 0, + 'providers' => [], + 'files' => [], + ], JSON_THROW_ON_ERROR)); + } + + $this->clearModuleRepositoryCache(); + + $module = Module::findOrFail($name); + + $enabled ? $module->enable() : $module->disable(); + + return $module; + } + + protected function clearModuleRepositoryCache(): void + { + $reflection = new \ReflectionClass(FileRepository::class); + + if ($reflection->hasProperty('modules')) { + $property = $reflection->getProperty('modules'); + $property->setAccessible(true); + $property->setValue(null, []); + } + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 5f8eb9f1..f42cc7a6 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -5,6 +5,7 @@ use BladeUI\Heroicons\BladeHeroiconsServiceProvider; use BladeUI\Icons\BladeIconsServiceProvider; use Coolsam\Modules\ModulesServiceProvider; +use Coolsam\Modules\Tests\Support\CreatesTestModules; use Filament\Actions\ActionsServiceProvider; use Filament\FilamentServiceProvider; use Filament\Forms\FormsServiceProvider; @@ -22,10 +23,13 @@ class TestCase extends Orchestra { + use CreatesTestModules; use WithWorkbench; protected function setUp(): void { + $this->resetModulesDirectory(); + parent::setUp(); Factory::guessFactoryNamesUsing( @@ -57,9 +61,15 @@ public function getEnvironmentSetUp($app) { config()->set('database.default', 'testing'); - /* - $migration = include __DIR__.'/../database/migrations/create_modules_table.php.stub'; - $migration->up(); - */ + $modulesPath = $app->basePath('Modules'); + + if (! is_dir($modulesPath)) { + mkdir($modulesPath, 0755, true); + } + + config()->set('modules.paths.modules', $modulesPath); + config()->set('modules.namespace', 'Modules'); + config()->set('modules.paths.app_folder', 'app'); + config()->set('modules.activators.file.statuses-file', $app->basePath('modules_statuses.json')); } } diff --git a/tests/Unit/CanAccessTraitTest.php b/tests/Unit/CanAccessTraitTest.php new file mode 100644 index 00000000..0e67dabd --- /dev/null +++ b/tests/Unit/CanAccessTraitTest.php @@ -0,0 +1,45 @@ +createTestModule('Blog', enabled: true); + + expect(TestChartWidget::canAccess())->toBeTrue(); + expect(TestChartWidget::canView())->toBeTrue(); +}); + +test('can access trait denies widget access when module is disabled', function () { + $this->createTestModule('Blog', enabled: false); + + expect(TestChartWidget::canAccess())->toBeFalse(); + expect(TestChartWidget::canView())->toBeFalse(); +}); + +test('can access trait does not call parent can access when parent lacks the method', function () { + $this->createTestModule('Blog', enabled: true); + + expect(method_exists(ChartWidget::class, 'canAccess'))->toBeTrue(); + expect(method_exists(Filament\Widgets\ChartWidget::class, 'canAccess'))->toBeFalse(); +}); + +test('can access trait resolves the module name from the class namespace', function () { + expect(TestChartWidget::getCurrentModuleName())->toBe('blog'); +}); diff --git a/tests/Unit/ModuleMacrosTest.php b/tests/Unit/ModuleMacrosTest.php new file mode 100644 index 00000000..44e408b6 --- /dev/null +++ b/tests/Unit/ModuleMacrosTest.php @@ -0,0 +1,32 @@ +createTestModule('Blog'); + + expect($module->namespace(''))->toBe('Modules\\Blog\\'); + expect($module->getTitle())->toBe('Blog'); + expect($module->appNamespace('Filament\\Resources'))->toBe('Modules\\Blog\\Filament\\Resources'); + expect($module->appPath('Filament'))->toEndWith('Blog' . DIRECTORY_SEPARATOR . 'app' . DIRECTORY_SEPARATOR . 'Filament'); + expect($module->databasePath('migrations'))->toEndWith('Blog' . DIRECTORY_SEPARATOR . 'database' . DIRECTORY_SEPARATOR . 'migrations'); + expect($module->resourcesPath('views'))->toEndWith('Blog' . DIRECTORY_SEPARATOR . 'resources' . DIRECTORY_SEPARATOR . 'views'); +}); + +test('module facade can resolve a scanned module without the global alias', function () { + expect(class_exists(\Module::class, false))->toBeFalse(); + + $this->createTestModule('Blog'); + + expect(Module::find('Blog'))->not->toBeNull(); + expect(Module::isEnabled('Blog'))->toBeTrue(); +}); + +test('filament modules helper can resolve module panels path via macros', function () { + $this->createTestModule('Blog'); + + $panels = FilamentModules::getModulePanels('Blog'); + + expect($panels)->toBeArray(); +}); diff --git a/tests/Unit/ModulesServiceProviderBootTest.php b/tests/Unit/ModulesServiceProviderBootTest.php new file mode 100644 index 00000000..7d9d7918 --- /dev/null +++ b/tests/Unit/ModulesServiceProviderBootTest.php @@ -0,0 +1,55 @@ +toBeFalse(); + expect($this->app->getProvider(ModulesServiceProvider::class))->toBeInstanceOf(ModulesServiceProvider::class); +}); + +test('modules service provider can register enabled module providers discovered on disk', function () { + $module = $this->createTestModule('Blog', enabled: true); + + $providerPath = $module->appPath('Providers' . DIRECTORY_SEPARATOR . 'BlogServiceProvider.php'); + $providerDir = dirname($providerPath); + + if (! is_dir($providerDir)) { + mkdir($providerDir, 0755, true); + } + + file_put_contents($providerPath, <<<'PHP' + toBe('Modules\\Blog\\Providers\\BlogServiceProvider'); + expect(Module::isEnabled('Blog'))->toBeTrue(); + + $this->app->register($namespace); + + expect(collect($this->app->getProviders($namespace)))->not->toBeEmpty(); +}); + +test('modules service provider condition matches module-owned provider class names', function () { + $module = $this->createTestModule('Blog', enabled: true); + + expect(str('BlogServiceProvider')->startsWith('Blog'))->toBeTrue(); + expect(Module::isEnabled('Blog'))->toBeTrue(); + expect(Module::isEnabled('blog'))->toBeTrue(); +}); diff --git a/tests/Unit/ResourceClassTest.php b/tests/Unit/ResourceClassTest.php new file mode 100644 index 00000000..05b382eb --- /dev/null +++ b/tests/Unit/ResourceClassTest.php @@ -0,0 +1,18 @@ +toBeTrue(); + expect(is_subclass_of(Resource::class, FilamentResource::class))->toBeTrue(); + + $source = file_get_contents(dirname(__DIR__, 2) . '/src/Resource.php'); + + expect($source)->toContain('use Filament\Resources\Resource as FilamentResource'); + expect($source)->not->toContain("use Filament\Resources\Resource;\n"); +}); + +test('resource class can be loaded without redeclaration errors', function () { + expect(new ReflectionClass(Resource::class)->isAbstract())->toBeTrue(); +}); From 7d495506f7db43f0c83fe3d97d3a6b8c35bcf935 Mon Sep 17 00:00:00 2001 From: Sam Maosa Date: Sat, 13 Jun 2026 17:38:53 +0300 Subject: [PATCH 2/4] fix: wrap new expression for PHP 8.3 method chaining Co-authored-by: Cursor --- tests/Unit/ResourceClassTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/ResourceClassTest.php b/tests/Unit/ResourceClassTest.php index 05b382eb..068d18f0 100644 --- a/tests/Unit/ResourceClassTest.php +++ b/tests/Unit/ResourceClassTest.php @@ -14,5 +14,5 @@ }); test('resource class can be loaded without redeclaration errors', function () { - expect(new ReflectionClass(Resource::class)->isAbstract())->toBeTrue(); + expect((new ReflectionClass(Resource::class))->isAbstract())->toBeTrue(); }); From e40fe05e80e4a7396eecab035f545b8589d0576f Mon Sep 17 00:00:00 2001 From: Sam Maosa Date: Sat, 13 Jun 2026 17:39:29 +0300 Subject: [PATCH 3/4] fix: avoid chained new expression in ResourceClassTest Co-authored-by: Cursor --- tests/Unit/ResourceClassTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Unit/ResourceClassTest.php b/tests/Unit/ResourceClassTest.php index 068d18f0..c732d9f0 100644 --- a/tests/Unit/ResourceClassTest.php +++ b/tests/Unit/ResourceClassTest.php @@ -14,5 +14,7 @@ }); test('resource class can be loaded without redeclaration errors', function () { - expect((new ReflectionClass(Resource::class))->isAbstract())->toBeTrue(); + $reflection = new ReflectionClass(Resource::class); + + expect($reflection->isAbstract())->toBeTrue(); }); From 95a2078c7321fb497febf36167e2d62cc50613dd Mon Sep 17 00:00:00 2001 From: Sam Maosa Date: Sat, 13 Jun 2026 17:43:12 +0300 Subject: [PATCH 4/4] fix: normalize module paths on Windows for macros and namespace conversion Normalize mixed directory separators in path macros and convertPathToNamespace so Windows CI and runtime resolve module provider namespaces correctly. Co-authored-by: Cursor --- src/Modules.php | 16 +++++++++++----- src/ModulesServiceProvider.php | 28 ++++++++++++++++++++++------ tests/Unit/ModulesSingletonTest.php | 8 ++++++++ 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/Modules.php b/src/Modules.php index efc12eb3..6f6a7963 100644 --- a/src/Modules.php +++ b/src/Modules.php @@ -67,14 +67,20 @@ public function getModuleClusters(string $moduleName) public function convertPathToNamespace(string $fullPath): string { + $normalizedPath = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $fullPath); $appFolder = trim(config('modules.paths.app_folder', 'app'), '/\\'); - $appPath = $appFolder . DIRECTORY_SEPARATOR; - $base = str(trim(config('modules.paths.modules', base_path('Modules')), '/\\')); - $replacementPath = str_replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, '/', DIRECTORY_SEPARATOR . $appPath); - $relative = str($fullPath)->afterLast($base)->replaceFirst($replacementPath, DIRECTORY_SEPARATOR); + $base = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, trim(config('modules.paths.modules', base_path('Modules')), '/\\')); + $appSegment = $appFolder . DIRECTORY_SEPARATOR; + + $relative = str($normalizedPath)->afterLast($base)->ltrim(DIRECTORY_SEPARATOR); + + if (str($relative)->startsWith($appSegment)) { + $relative = str($relative)->after($appSegment); + } else { + $relative = str($relative)->replace(DIRECTORY_SEPARATOR . $appSegment, DIRECTORY_SEPARATOR); + } return str($relative) - ->ltrim('/\\') ->prepend(DIRECTORY_SEPARATOR) ->prepend(config('modules.namespace', 'Modules')) ->replace(DIRECTORY_SEPARATOR, '\\') diff --git a/src/ModulesServiceProvider.php b/src/ModulesServiceProvider.php index 23dd348a..12cc1b1f 100644 --- a/src/ModulesServiceProvider.php +++ b/src/ModulesServiceProvider.php @@ -233,39 +233,55 @@ protected function registerModuleMacros(): void NwidartModule::macro('appPath', function (string $relativePath = '') { $appPath = $this->getExtraPath(config('modules.paths.app_folder', 'app')); - return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : ''))->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); NwidartModule::macro('databasePath', function (string $relativePath = '') { $appPath = $this->getExtraPath('database'); - return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : ''))->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); NwidartModule::macro('resourcesPath', function (string $relativePath = '') { $appPath = $this->getExtraPath('resources'); return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) - ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); NwidartModule::macro('migrationsPath', function (string $relativePath = '') { $appPath = $this->databasePath('migrations'); return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) - ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); NwidartModule::macro('seedersPath', function (string $relativePath = '') { $appPath = $this->databasePath('seeders'); - return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : ''))->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); NwidartModule::macro('factoriesPath', function (string $relativePath = '') { $appPath = $this->databasePath('factories'); - return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : ''))->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR)->toString(); + return str($appPath . ($relativePath ? DIRECTORY_SEPARATOR . $relativePath : '')) + ->replace(['/', '\\'], DIRECTORY_SEPARATOR) + ->replace(DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR) + ->toString(); }); } } diff --git a/tests/Unit/ModulesSingletonTest.php b/tests/Unit/ModulesSingletonTest.php index d1d6cb0e..6f1aea9e 100644 --- a/tests/Unit/ModulesSingletonTest.php +++ b/tests/Unit/ModulesSingletonTest.php @@ -7,3 +7,11 @@ $namespace = FilamentModules::convertPathToNamespace($path); expect($namespace)->toBe($expected = 'Modules\\Providers\\TestServiceProvider', "Expected $expected Instead got " . $namespace); }); + +test('can convert windows style module paths to namespaces', function () { + $base = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, config('modules.paths.modules')); + $path = $base . DIRECTORY_SEPARATOR . 'Blog' . DIRECTORY_SEPARATOR . 'app' . DIRECTORY_SEPARATOR . 'Providers' . DIRECTORY_SEPARATOR . 'BlogServiceProvider.php'; + $namespace = FilamentModules::convertPathToNamespace($path); + + expect($namespace)->toBe('Modules\\Blog\\Providers\\BlogServiceProvider'); +});