Merge pull request #189 from PortableStudios/fix/unique-slugs

Fixes for unique slugs

src/Contracts/HasSlug.php

@@ -11,18 +11,46 @@ protected function slugifyField(): string
         return 'title';
     }
 
+    protected function scopeSlugQuery($query, $slug)
+    {
+        $query = $query->where('slug', $slug);
+        if($this->id) {
+            $query = $query->where('id', '!=', $this->id);
+        }
+        return $query;
+    }
+
     protected static function bootHasSlug()
     {
         static::creating(function ($model) {
-            if ($model->slug === null) {
-                $model->slug = Str::slug($model->title);
-            }
+            $model->slug = $model->getNewSlug();
         });
 
         static::updating(function ($model) {
-            if ($model->slug === null) {
-                $model->slug = Str::slug($model->title);
-            }
+            $model->slug = $model->getNewSlug();
         });
     }
+
+    protected function getNewSlug()
+    {
+        $newSlug = $this->slug ?? Str::slug($this->{$this->slugifyField()});
+
+        // if there is a -clone already, then append 1 or increment
+        $result = $this->scopeSlugQuery(static::withoutGlobalScopes(), $newSlug)->first();
+
+        $count = 1;
+        while ($result != null) {
+            $incrementedSlug = $newSlug . '-' . $count;
+
+            $result = $result = $this->scopeSlugQuery(static::withoutGlobalScopes(), $incrementedSlug)->first();
+            $count++;
+
+            if ($result == null) {
+                $newSlug = $incrementedSlug;
+                break;
+            }
+        }
+
+        return $newSlug;
+    }
 }

src/Filament/Actions/CloneAction.php

@@ -27,7 +27,6 @@ protected function setUp(): void
         $this->action(function (Model $record): void {
             $model = get_class($record);
             $data = $record->toArray();
-            $data['slug'] = $this->getNewSlug($model, $data['slug']);
             $data['title'] = '[CLONE] ' . $data['title'];
             $data['is_draft'] = true;
 
@@ -97,27 +96,4 @@ protected function pickData(Model $data, $fields)
 
         return $newData;
     }
-
-    protected function getNewSlug($model, $slug)
-    {
-        $newSlug = $slug;
-
-        // if there is a -clone already, then append 1 or increment
-        $result = $model::withoutGlobalScopes()->where('slug', $newSlug)->first();
-
-        $count = 1;
-        while ($result != null) {
-            $incrementedSlug = $newSlug . '-' . $count;
-
-            $result = $model::withoutGlobalScopes()->where('slug', $incrementedSlug)->first();
-            $count++;
-
-            if ($result == null) {
-                $newSlug = $incrementedSlug;
-                break;
-            }
-        }
-
-        return $newSlug;
-    }
 }

testbench.yaml

@@ -21,6 +21,7 @@ providers:
   - Laravel\Scout\ScoutServiceProvider
   - Spatie\Health\HealthServiceProvider
   - Lab404\Impersonate\ImpersonateServiceProvider
+  - Schmeits\FilamentCharacterCounter\FilamentCharacterCounterServiceProvider
 
 migrations:
   - workbench/database/migrations

tests/Feature/PageTest.php

@@ -6,9 +6,14 @@
 use Illuminate\Foundation\Testing\WithFaker;
 use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\Str;
+use Livewire\Livewire;
+use Portable\FilaCms\Database\Seeders\RoleAndPermissionSeeder;
+use Portable\FilaCms\Filament\Actions\CloneAction;
+use Portable\FilaCms\Filament\Resources\PageResource\Pages\ListPages;
 use Portable\FilaCms\Models\Author;
 use Portable\FilaCms\Models\Page;
 use Portable\FilaCms\Tests\TestCase;
+use Spatie\Permission\Models\Role;
 
 class PageTest extends TestCase
 {
@@ -20,14 +25,17 @@ class PageTest extends TestCase
     protected function setUp(): void
     {
         parent::setUp();
+        $this->artisan('db:seed', ['--class' => RoleAndPermissionSeeder::class]);
         $this->userModel = config('auth.providers.users.model');
         $user = $this->userModel::create([
             'name'  => 'Jeremy Layson',
             'email' => '[email protected]',
             'password'  => 'password'
         ]);
+        $adminRole = Role::where('name', 'Admin')->first();
+        $user->assignRole($adminRole);
 
-        $this->be($user);
+        $this->actingAs($user);
 
         Author::create([
             'first_name'    => 'Portable',
@@ -102,6 +110,24 @@ public function test_published_status(): void
         $this->assertEquals($page->status, 'Published');
     }
 
+    public function test_clone(): void
+    {
+        $user = $this->userModel::first();
+        $adminRole = Role::where('name', 'Admin')->first();
+        $this->assertNotNull($adminRole);
+        $user->assignRole($adminRole);
+        $this->actingAs($user);
+        $page = Page::factory()->create([
+            'is_draft' => 0,
+            'publish_at' => $this->faker->dateTimeBetween('-1 week', '-1 day'),
+            'expire_at' => $this->faker->dateTimeBetween('+1 day', '+1 week'),
+        ]);
+        Livewire::test(ListPages::class)->callTableAction(CloneAction::class, $page->id);
+
+        $this->assertDatabaseHas('pages', [ 'title' => '[CLONE] ' . $page->title ]);
+        $this->assertDatabaseHas('pages', [ 'slug' => $page->slug . '-1' ]);
+    }
+
     public function test_expired_status(): void
     {
         $author = Author::first();

tests/TestCase.php

@@ -9,6 +9,7 @@
 use Illuminate\Support\Facades\Process;
 use Orchestra\Testbench\Attributes\WithMigration;
 use Orchestra\Testbench\Concerns\WithWorkbench;
+use Portable\FilaCms\Database\Seeders\RoleAndPermissionSeeder;
 
 #[WithMigration]
 abstract class TestCase extends \Orchestra\Testbench\TestCase
@@ -54,6 +55,8 @@ protected function setUp(): void
         File::copy(getcwd() . '/resources/css/filacms.css', resource_path('css/filacms.css'));
         File::copy(getcwd() . '/package.json', resource_path('../package.json'));
         Process::path(app_path())->run('npm run build');
+
+        $this->artisan('db:seed', ['--class' => RoleAndPermissionSeeder::class]);
     }
 
     protected function defineEnvironment($app)

tests/Unit/AbstractContentModelTest.php

@@ -16,6 +16,35 @@ class AbstractContentModelTest extends TestCase
 
     protected $author = null;
 
+    public function test_duplicate_slugs_prevented()
+    {
+        $page = Page::factory()->create();
+        $page2 = Page::factory()->create(['title' => $page->title, 'slug' => $page->slug]);
+
+        $this->assertNotEquals($page->slug, $page2->slug);
+    }
+
+    public function test_can_edit_slugs_dont_change()
+    {
+        $page = Page::factory()->create();
+        $originalSlug = $page->slug;
+        $page->title = $this->faker->sentence;
+        $page->save();
+        $page->refresh();
+
+        $this->assertEquals($originalSlug, $page->slug);
+    }
+
+    public function test_can_supply_slug()
+    {
+        $page = Page::factory()->create();
+        $page->slug = 'test-slug';
+        $page->save();
+        $page->refresh();
+
+        $this->assertEquals('test-slug', $page->slug);
+    }
+
     public function test_with_pending()
     {
         $page = Page::factory()->create();