Merge pull request #154 from limosa-io/copilot/fix-default-migration-inclusion

Make SCIM user table migrations opt-in instead of auto-loading

Author: arietimmerman

Dockerfile

@@ -169,12 +169,32 @@ class UserFactory extends Factory
         $suffix = $this->faker->unique()->numberBetween(100, 999999);
         $username = $base . $suffix;
 
+        $roleTemplates = [
+            ['value' => 'admin', 'display' => 'Admin', 'type' => 'system'],
+            ['value' => 'developer', 'display' => 'Developer', 'type' => 'system'],
+            ['value' => 'manager', 'display' => 'Manager', 'type' => 'business'],
+            ['value' => 'support', 'display' => 'Support', 'type' => 'business'],
+            ['value' => 'auditor', 'display' => 'Auditor', 'type' => 'governance'],
+        ];
+
+        $selectedRoles = $this->faker->randomElements(
+            $roleTemplates,
+            $this->faker->numberBetween(1, count($roleTemplates))
+        );
+        $selectedRoles = array_values($selectedRoles);
+        $primaryIndex = array_rand($selectedRoles);
+        foreach ($selectedRoles as $index => &$role) {
+            $role['primary'] = $index === $primaryIndex;
+        }
+        unset($role);
+
         return [
             'name' => $username,                 // login username
             'formatted' => $formatted,           // full name
             'email' => "{$username}@example.test",
             'password' => bcrypt('test'),
             'active' => $this->faker->boolean(),
+            'roles' => $selectedRoles,
         ];
     }
 }
@@ -229,6 +249,7 @@ class User extends Authenticatable
         'email',
         'password',
         'active',
+        'roles',
 
     ];
 
@@ -240,6 +261,7 @@ class User extends Authenticatable
     protected $casts = [
         'email_verified_at' => 'datetime',
         'active' => 'boolean',
+        'roles' => 'array',
 
     ];
 
@@ -277,55 +299,10 @@ class DemoSeeder extends Seeder
 }
 EOM
 
-# Add migration to add SCIM fields to users table
-RUN cat > /example/database/migrations/2021_01_01_000003_add_scim_fields_to_users_table.php <<'EOM'
-<?php
-
-use Illuminate\Database\Migrations\Migration;
-use Illuminate\Database\Schema\Blueprint;
-use Illuminate\Support\Facades\Schema;
-
-return new class extends Migration {
-    public function up(): void
-    {
-        Schema::table('users', function (Blueprint $table) {
-            if (!Schema::hasColumn('users', 'formatted')) {
-                $table->string('formatted')->nullable();
-            }
-            if (!Schema::hasColumn('users', 'displayName')) {
-                $table->string('displayName')->nullable();
-            }
-            if (!Schema::hasColumn('users', 'active')) {
-                $table->boolean('active')->default(false);
-            }
-            if (!Schema::hasColumn('users', 'roles')) {
-                $table->json('roles')->nullable();
-            }
-        });
-    }
-
-    public function down(): void
-    {
-        Schema::table('users', function (Blueprint $table) {
-            if (Schema::hasColumn('users', 'formatted')) {
-                $table->dropColumn('formatted');
-            }
-            if (Schema::hasColumn('users', 'displayName')) {
-                $table->dropColumn('displayName');
-            }
-            if (Schema::hasColumn('users', 'active')) {
-                $table->dropColumn('active');
-            }
-            if (Schema::hasColumn('users', 'roles')) {
-                $table->dropColumn('roles');
-            }
-        });
-    }
-};
-EOM
+# Publish SCIM migrations from the package
+RUN php artisan vendor:publish --tag=laravel-scim-migrations
 
 # Run migrations and seed demo data
 RUN php artisan migrate && php artisan db:seed --class=Database\\Seeders\\DemoSeeder
 
 CMD ["php","artisan","serve","--host=0.0.0.0","--port=8000"]
-

README.md

@@ -45,6 +45,15 @@ Optionally publish the config for fine-grained control:
 php artisan vendor:publish --tag=laravel-scim
 ```
 
+If you need to add SCIM-specific columns (`formatted`, `active`, `roles`) to your users table, publish the migrations:
+
+```bash
+php artisan vendor:publish --tag=laravel-scim-migrations
+php artisan migrate
+```
+
+**Note:** These migrations are optional. Only publish them if your SCIM implementation requires these specific fields in your users table.
+
 ## SCIM routes
 
 | Method | Path | Description |

database/migrations/2021_01_01_000003_add_scim_fields_to_users_table.php

@@ -12,9 +12,15 @@ public function up(): void
                 if (!Schema::hasColumn('users', 'formatted')) {
                     $table->string('formatted')->nullable();
                 }
+                if (!Schema::hasColumn('users', 'displayName')) {
+                    $table->string('displayName')->nullable();
+                }
                 if (!Schema::hasColumn('users', 'active')) {
                     $table->boolean('active')->default(false);
                 }
+                if (!Schema::hasColumn('users', 'roles')) {
+                    $table->json('roles')->nullable();
+                }
             });
         }
     }
@@ -26,9 +32,15 @@ public function down(): void
                 if (Schema::hasColumn('users', 'formatted')) {
                     $table->dropColumn('formatted');
                 }
+                if (Schema::hasColumn('users', 'displayName')) {
+                    $table->dropColumn('displayName');
+                }
                 if (Schema::hasColumn('users', 'active')) {
                     $table->dropColumn('active');
                 }
+                if (Schema::hasColumn('users', 'roles')) {
+                    $table->dropColumn('roles');
+                }
             });
         }
     }

src/Attribute/JSONCollection.php

@@ -29,7 +29,13 @@ public function replace($value, Model &$object, ?Path $path = null)
 
     public function doRead(&$object, $attributes = [])
     {
-        return $object->{$this->attribute}?->values()->all();
+        $value = $object->{$this->attribute};
+
+        if ($value === null) {
+            return null;
+        }
+
+        return collect($value)->values()->all();
     }
 
     public function remove($value, Model &$object, Path $path = null)

src/ServiceProvider.php

@@ -12,12 +12,14 @@ class ServiceProvider extends \Illuminate\Support\ServiceProvider
 {
     public function boot(\Illuminate\Routing\Router $router)
     {
-        $this->loadMigrationsFrom(__DIR__ . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . 'database' . DIRECTORY_SEPARATOR . 'migrations');
-
         $this->publishes([
             __DIR__ . '/../config/scim.php' => config_path('scim.php'),
         ], 'laravel-scim');
 
+        $this->publishes([
+            __DIR__ . '/../database/migrations/' => database_path('migrations'),
+        ], 'laravel-scim-migrations');
+
         // Match everything, except the Me routes
         $router->pattern('resourceType', '^((?!Me).)*$');
 

tests/ServiceProviderTest.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace ArieTimmerman\Laravel\SCIMServer\Tests;
+
+use ArieTimmerman\Laravel\SCIMServer\ServiceProvider;
+use Illuminate\Support\Facades\Artisan;
+use Orchestra\Testbench\TestCase as BaseTestCase;
+
+class ServiceProviderTest extends BaseTestCase
+{
+    protected function getPackageProviders($app)
+    {
+        return [ServiceProvider::class];
+    }
+
+    public function testMigrationsCanBePublished()
+    {
+        // Test that the vendor:publish command can find the migration tag
+        $result = Artisan::call('vendor:publish', [
+            '--tag' => 'laravel-scim-migrations',
+            '--provider' => ServiceProvider::class,
+            '--force' => true,
+        ]);
+
+        // The command should succeed (return 0)
+        $this->assertEquals(0, $result, 'Migrations should be publishable via vendor:publish command');
+    }
+
+    public function testConfigCanBePublished()
+    {
+        // Test that the vendor:publish command can find the config tag
+        $result = Artisan::call('vendor:publish', [
+            '--tag' => 'laravel-scim',
+            '--provider' => ServiceProvider::class,
+            '--force' => true,
+        ]);
+
+        // The command should succeed (return 0)
+        $this->assertEquals(0, $result, 'Config should be publishable via vendor:publish command');
+    }
+
+    public function testMigrationFileExistsInPackage()
+    {
+        // Verify the migration file exists in the package
+        $migrationPath = realpath(__DIR__ . '/../database/migrations/2021_01_01_000003_add_scim_fields_to_users_table.php');
+        
+        $this->assertFileExists($migrationPath, 'Migration file should exist in the package');
+    }
+}