diff --git a/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub b/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub new file mode 100644 index 0000000..64f2bc6 --- /dev/null +++ b/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub @@ -0,0 +1,70 @@ + + */ + private array $columns = ['name', 'slug', 'external_id']; + + public function up(): void + { + if (! $this->shouldScopeUniqueIndexes()) { + return; + } + + $tenantColumn = TenantHelper::getTenantColumnName(); + $drop = []; + $add = []; + + foreach ($this->columns as $column) { + if (Schema::hasIndex('lms_courses', [$column], 'unique')) { + $drop[] = $column; + } + + if (! Schema::hasIndex('lms_courses', [$tenantColumn, $column], 'unique')) { + $add[] = $column; + } + } + + if ($drop !== []) { + Schema::table('lms_courses', function (Blueprint $table) use ($drop): void { + foreach ($drop as $column) { + $table->dropUnique([$column]); + } + }); + } + + if ($add !== []) { + Schema::table('lms_courses', function (Blueprint $table) use ($add, $tenantColumn): void { + foreach ($add as $column) { + $table->unique([$tenantColumn, $column]); + } + }); + } + } + + public function down(): void + { + // Do not restore global unique indexes. After this migration runs, + // two tenants may share a name, slug, or external_id; rolling back + // would drop tenant-scoped constraints and can fail to recreate globals. + } + + private function shouldScopeUniqueIndexes(): bool + { + if (! config('filament-lms.tenancy.enabled')) { + return false; + } + + $tenantColumn = TenantHelper::getTenantColumnName(); + + return Schema::hasTable('lms_courses') + && Schema::hasColumn('lms_courses', $tenantColumn); + } +}; diff --git a/src/FilamentLmsServiceProvider.php b/src/FilamentLmsServiceProvider.php index 52f235a..017874e 100644 --- a/src/FilamentLmsServiceProvider.php +++ b/src/FilamentLmsServiceProvider.php @@ -72,6 +72,7 @@ public function configurePackage(Package $package): void 'create_lms_course_user_group_table', 'create_lms_user_group_memberships_table', 'add_is_explicitly_assigned_to_lms_course_user_table', + 'scope_lms_courses_unique_indexes_to_tenant', ]) ->hasCommand(BackfillCourseCompletedAt::class) ->hasCommand(BackfillEmbeddedPlayerCourses::class) diff --git a/src/Resources/CourseResource.php b/src/Resources/CourseResource.php index 5861d54..2bd9c5e 100644 --- a/src/Resources/CourseResource.php +++ b/src/Resources/CourseResource.php @@ -83,13 +83,13 @@ public static function form(Schema $schema): Schema $set('external_id', Str::slug($state ?? '', '_')); } }) - ->unique(ignoreRecord: true) + ->scopedUnique(ignoreRecord: true) ->required(), TextInput::make('external_id') ->label('External ID') ->helperText('Used for external integrations like HubSpot. Updating this will cause a new property to be added to the integration.') ->required() - ->unique(ignoreRecord: true) + ->scopedUnique(ignoreRecord: true) ->rules([ 'regex:/^[a-z][a-z0-9_]*$/', 'max:100', @@ -100,7 +100,7 @@ public static function form(Schema $schema): Schema ]), TextInput::make('slug') ->helperText('Used for urls.') - ->unique(ignoreRecord: true) + ->scopedUnique(ignoreRecord: true) ->required(), SpatieMediaLibraryFileUpload::make('image') ->helperText('Upload a course image.') diff --git a/tests/Feature/ScopeLmsCoursesUniqueIndexesToTenantTest.php b/tests/Feature/ScopeLmsCoursesUniqueIndexesToTenantTest.php new file mode 100644 index 0000000..78405f4 --- /dev/null +++ b/tests/Feature/ScopeLmsCoursesUniqueIndexesToTenantTest.php @@ -0,0 +1,102 @@ +id(); + $table->timestamps(); + }); + + Schema::table('lms_courses', function (Blueprint $table) { + $table->foreignId('company_id')->nullable()->constrained('companies'); + }); + + config([ + 'filament-lms.tenancy.enabled' => true, + 'filament-lms.tenancy.column' => 'company_id', + ]); + + $migration = require dirname(__DIR__, 2).'/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub'; + + $migration->up(); + $migration->up(); + + expect(Schema::hasIndex('lms_courses', ['name'], 'unique'))->toBeFalse() + ->and(Schema::hasIndex('lms_courses', ['slug'], 'unique'))->toBeFalse() + ->and(Schema::hasIndex('lms_courses', ['external_id'], 'unique'))->toBeFalse() + ->and(Schema::hasIndex('lms_courses', ['company_id', 'name'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['company_id', 'slug'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['company_id', 'external_id'], 'unique'))->toBeTrue(); +}); + +test('does not change course unique indexes when tenancy is disabled', function () { + $migration = require dirname(__DIR__, 2).'/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub'; + + $migration->up(); + + expect(Schema::hasIndex('lms_courses', ['name'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['slug'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['external_id'], 'unique'))->toBeTrue(); +}); + +test('does not replace tenant unique indexes with global uniques on rollback', function () { + Schema::create('companies', function (Blueprint $table) { + $table->id(); + $table->timestamps(); + }); + + Schema::table('lms_courses', function (Blueprint $table) { + $table->foreignId('company_id')->nullable()->constrained('companies'); + }); + + config([ + 'filament-lms.tenancy.enabled' => true, + 'filament-lms.tenancy.column' => 'company_id', + ]); + + $migration = require dirname(__DIR__, 2).'/database/migrations/scope_lms_courses_unique_indexes_to_tenant.php.stub'; + + $migration->up(); + + $alphaId = DB::table('companies')->insertGetId([ + 'created_at' => now(), + 'updated_at' => now(), + ]); + $betaId = DB::table('companies')->insertGetId([ + 'created_at' => now(), + 'updated_at' => now(), + ]); + + DB::table('lms_courses')->insert([ + [ + 'company_id' => $alphaId, + 'name' => 'Shared Course Name', + 'slug' => 'shared-course-slug', + 'external_id' => 'shared-external-id', + 'created_at' => now(), + 'updated_at' => now(), + ], + [ + 'company_id' => $betaId, + 'name' => 'Shared Course Name', + 'slug' => 'shared-course-slug', + 'external_id' => 'shared-external-id', + 'created_at' => now(), + 'updated_at' => now(), + ], + ]); + + $migration->down(); + + expect(Schema::hasIndex('lms_courses', ['company_id', 'name'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['company_id', 'slug'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['company_id', 'external_id'], 'unique'))->toBeTrue() + ->and(Schema::hasIndex('lms_courses', ['name'], 'unique'))->toBeFalse() + ->and(Schema::hasIndex('lms_courses', ['slug'], 'unique'))->toBeFalse() + ->and(Schema::hasIndex('lms_courses', ['external_id'], 'unique'))->toBeFalse(); +});