diff --git a/src/IlluminateTag.php b/src/IlluminateTag.php index b01d10c..8c66ef3 100644 --- a/src/IlluminateTag.php +++ b/src/IlluminateTag.php @@ -25,6 +25,10 @@ use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; +/** @property string $name */ +/** @property string $namespace */ +/** @property string $slug */ +/** @property int $count */ class IlluminateTag extends Model { /** diff --git a/src/TaggableTrait.php b/src/TaggableTrait.php index 0b2046c..ce601f3 100644 --- a/src/TaggableTrait.php +++ b/src/TaggableTrait.php @@ -20,10 +20,12 @@ namespace Cartalyst\Tags; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\MorphToMany; +/** @property Collection $tags */ trait TaggableTrait { /** @@ -172,10 +174,19 @@ public function tag($tags): bool */ public function untag($tags = null): bool { - $tags = $tags ?: $this->tags->pluck('name')->all(); - - foreach ($this->prepareTags($tags) as $tag) { - $this->removeTag($tag); + if (empty($tags)) { + $tags = $this->tags; + if (!empty($tags)) { + $this->tags()->detach(); + foreach ($tags as $tag) { + $tag->update(['count' => $tag->count - 1]); + } + $this->tags = new Collection(); + } + } else { + foreach ($this->prepareTags($tags) as $tag) { + $this->removeTag($tag); + } } return true; @@ -214,24 +225,24 @@ public function setTags($tags, string $type = 'name'): bool */ public function addTag(string $name): void { - $tag = $this->createTagsModel()->firstOrNew([ + /** @var IlluminateTag $tag */ + $tag = $this->createTagsModel()->firstOrCreate([ 'slug' => $this->generateTagSlug($name), 'namespace' => $this->getEntityClassName(), + ], [ + 'count' => 1, + 'name' => $name, ]); - if (! $tag->exists) { - $tag->name = $name; - - $tag->save(); - } - - if (! $this->tags()->get()->contains($tag->id)) { - $tag->update(['count' => $tag->count + 1]); - + if ($tag->wasRecentlyCreated || !$this->tags()->whereKey($tag)->exists()) { + if (!$tag->wasRecentlyCreated) { + $tag->update(['count' => $tag->count + 1]); + } $this->tags()->attach($tag); + if ($this->relationLoaded('tags')) { + $this->tags->add($tag); + } } - - $this->load('tags'); } /** @@ -243,8 +254,8 @@ public function removeTag(string $name): void $namespace = $this->getEntityClassName(); - $tag = $this - ->createTagsModel() + /** @var IlluminateTag $tag */ + $tag = $this->tags() ->whereNamespace($namespace) ->where(function ($query) use ($name, $slug) { $query @@ -255,13 +266,19 @@ public function removeTag(string $name): void ->first() ; - if ($tag && $this->tags()->get()->contains($tag->id)) { + if ($tag) { $tag->update(['count' => $tag->count - 1]); - $this->tags()->detach($tag); - } - $this->load('tags'); + if ($this->relationLoaded('tags')) { + foreach($this->tags as $key => $loadedTag) { + if ($loadedTag->getKey() === $tag->getKey()) { + unset($this->tags[$key]); + break; + } + } + } + } } /** diff --git a/tests/FunctionalTestCase.php b/tests/FunctionalTestCase.php index 8326eba..864e12e 100644 --- a/tests/FunctionalTestCase.php +++ b/tests/FunctionalTestCase.php @@ -21,6 +21,7 @@ namespace Cartalyst\Tags\Tests; use Cartalyst\Tags\Tests\Stubs\Post; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; class FunctionalTestCase extends \Orchestra\Testbench\TestCase @@ -77,4 +78,14 @@ protected function createPost() { return Post::create(['title' => 'My Test Post']); } + + protected function withQueryCount($cb): int + { + DB::enableQueryLog(); + call_user_func($cb); + $log = DB::getQueryLog(); + DB::flushQueryLog(); + DB::disableQueryLog(); + return count($log); + } } diff --git a/tests/IlluminateTagTest.php b/tests/IlluminateTagTest.php index d7922df..b3385e1 100644 --- a/tests/IlluminateTagTest.php +++ b/tests/IlluminateTagTest.php @@ -24,10 +24,11 @@ use Cartalyst\Tags\IlluminateTagged; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; +use PHPUnit\Framework\Attributes\Test; class IlluminateTagTest extends FunctionalTestCase { - /** @test */ + #[Test] public function it_can_delete_a_tag_and_its_tagged_relations() { $post = $this->createPost(); @@ -45,7 +46,7 @@ public function it_can_delete_a_tag_and_its_tagged_relations() $this->assertCount(1, $post->tags); } - /** @test */ + #[Test] public function it_has_a_taggable_relationship() { $tag = new IlluminateTag(); @@ -53,7 +54,7 @@ public function it_has_a_taggable_relationship() $this->assertInstanceOf(MorphTo::class, $tag->taggable()); } - /** @test */ + #[Test] public function it_has_a_tag_relationship() { $tag = new IlluminateTag(); @@ -61,7 +62,7 @@ public function it_has_a_tag_relationship() $this->assertInstanceOf(HasMany::class, $tag->tagged()); } - /** @test */ + #[Test] public function it_has_a_name_scope() { IlluminateTag::create(['name' => 'Foo', 'slug' => 'foo', 'namespace' => 'foo']); @@ -69,7 +70,7 @@ public function it_has_a_name_scope() $this->assertCount(1, IlluminateTag::name('Foo')->get()); } - /** @test */ + #[Test] public function it_has_a_slug_scope() { IlluminateTag::create(['name' => 'Foo', 'slug' => 'foo', 'namespace' => 'foo']); @@ -77,7 +78,7 @@ public function it_has_a_slug_scope() $this->assertCount(1, IlluminateTag::slug('foo')->get()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tagged_model() { $tag = new IlluminateTag(); diff --git a/tests/TaggableTraitTest.php b/tests/TaggableTraitTest.php index 5c2d026..0d081d6 100644 --- a/tests/TaggableTraitTest.php +++ b/tests/TaggableTraitTest.php @@ -23,10 +23,12 @@ use Cartalyst\Tags\IlluminateTag; use Cartalyst\Tags\Tests\Stubs\Post; use Cartalyst\Tags\Tests\Stubs\Post2; +use Illuminate\Support\Facades\DB; +use PHPUnit\Framework\Attributes\Test; class TaggableTraitTest extends FunctionalTestCase { - /** @test */ + #[Test] public function it_can_add_a_single_tag() { $post1 = $this->createPost(); @@ -39,23 +41,26 @@ public function it_can_add_a_single_tag() $this->assertSame(['foo'], $post2->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_add_multiple_tags() { $post1 = $this->createPost(); $post2 = $this->createPost(); $post3 = $this->createPost(); - $post1->tag('foo, bar'); - $post2->tag(['foo', 'bar']); - $post3->tag(null); - + $count = $this->withQueryCount(function () use ($post1, $post2, $post3) { + $post1->tag('foo, bar'); + $post2->tag(['foo', 'bar']); + $post3->tag(null); + }); + $this->assertLessThanOrEqual(14, $count); // Previously 22 queries $this->assertSame(['foo', 'bar'], $post1->tags->pluck('slug')->toArray()); $this->assertSame(['foo', 'bar'], $post2->tags->pluck('slug')->toArray()); $this->assertEmpty($post3->tags->pluck('slug')->toArray()); + } - /** @test */ + #[Test] public function it_can_untag() { $post = $this->createPost(); @@ -70,21 +75,23 @@ public function it_can_untag() $this->assertEmpty($post->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_remove_all_tags() { $post = $this->createPost(); - $post->tag('foo, bar, baz'); + $queryCount = $this->withQueryCount(fn () => $post->tag('foo, bar, baz')); $this->assertCount(3, $post->tags); + $this->assertLessThanOrEqual(9, $queryCount); - $post->untag(); + $queryCount = $this->withQueryCount(fn () => $post->untag()); $this->assertCount(0, $post->tags); + $this->assertLessThanOrEqual(4, $queryCount); } - /** @test */ + #[Test] public function it_can_set_tags() { $post = $this->createPost(); @@ -96,7 +103,7 @@ public function it_can_set_tags() $this->assertSame(['foo', 'bar'], $post->tags->pluck('slug')->toArray()); } - /** @test */ + #[Test] public function it_can_retrieve_tags() { $post = $this->createPost(); @@ -106,7 +113,7 @@ public function it_can_retrieve_tags() $this->assertCount(3, $post->tags); } - /** @test */ + #[Test] public function it_can_retrieve_all_tags() { $post1 = $this->createPost(); @@ -119,7 +126,7 @@ public function it_can_retrieve_all_tags() $this->assertCount(0, Post2::allTags()->get()); } - /** @test */ + #[Test] public function it_can_retrieve_by_the_given_tags() { $post1 = $this->createPost(); @@ -135,7 +142,7 @@ public function it_can_retrieve_by_the_given_tags() $this->assertcount(1, Post::withTag('bat')->get()); } - /** @test */ + #[Test] public function it_can_retrieve_without_the_given_tags() { $post1 = $this->createPost(); @@ -151,7 +158,7 @@ public function it_can_retrieve_without_the_given_tags() $this->assertcount(1, Post::withoutTag('bat')->get()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tags_delimiter() { $post = new Post(); @@ -161,7 +168,7 @@ public function it_can_get_and_set_the_tags_delimiter() $this->assertSame(',', $post->getTagsDelimiter()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_tags_model() { $post = new Post(); @@ -171,7 +178,7 @@ public function it_can_get_and_set_the_tags_model() $this->assertSame(IlluminateTag::class, $post->getTagsModel()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_slug_generator_as_a_string() { $post = new Post(); @@ -181,7 +188,7 @@ public function it_can_get_and_set_the_slug_generator_as_a_string() $this->assertSame('Illuminate\Support\Str::slug', $post->getSlugGenerator()); } - /** @test */ + #[Test] public function it_can_get_and_set_the_slug_generator_as_a_closure() { $post = new Post();