Some cleanups and improvements around tag groups

Author: rtconner

migrations/2014_01_07_073615_create_tags_table.php

@@ -13,6 +13,7 @@ public function up()
 			$table->string('name', 125);
 			$table->boolean('suggest')->default(false);
 			$table->integer('count')->unsigned()->default(0); // count of how many times this tag was used
+            $table->integer('tag_group_id')->unsigned()->nullable();
 		});
 	}
 

migrations/2016_06_29_073615_update_tags_table.php

@@ -8,7 +8,6 @@ class UpdateTagsTable extends Migration {
 	public function up()
 	{
 		Schema::table('tagging_tags', function ($table) {
-			$table->integer('tag_group_id')->unsigned()->nullable()->after('id');
 			$table->foreign('tag_group_id')->references('id')->on('tagging_tag_groups');
 		});
 
@@ -20,7 +19,6 @@ public function down()
         Schema::disableForeignKeyConstraints();
 		Schema::table('tagging_tags', function ($table) {
 			$table->dropForeign('tagging_tags_tag_group_id_foreign');
-			$table->dropColumn('tag_group_id');
 		});
 	}
 }

src/Model/Tag.php

@@ -13,9 +13,10 @@
  * @property string slug
  * @property bool suggest
  * @property integer count
+ * @property integer tag_group_id
  * @property TagGroup group
  * @method static suggested()
- * @method static inGroup(string $groupName)
+ * @method static inGroup(string $group)
  */
 class Tag extends Model
 {
@@ -49,13 +50,13 @@ public function save(array $options = [])
 
     /**
      * Tag group setter
-     * @param string $groupName
+     * @param string $group
      * @return Tag
      */
-    public function setGroup($groupName)
+    public function setGroup(string $group)
     {
         $tagGroup = TagGroup::query()
-            ->where('slug', TaggingUtility::normalize($groupName))
+            ->where('slug', TaggingUtility::normalize($group))
             ->first();
 
         if ($tagGroup) {
@@ -64,27 +65,20 @@ public function setGroup($groupName)
 
             return $this;
         } else {
-            throw new \RuntimeException('No Tag Group found: '. $groupName);
+            throw new \RuntimeException('No Tag Group found: '. $group);
         }
     }
 
     /**
      * Tag group remove
-     * @param string $groupName
      * @return Tag
      */
-    public function removeGroup(string $groupName)
+    public function removeGroup()
     {
-        $tagGroup = TagGroup::query()->where('slug', TaggingUtility::normalize($groupName))->first();
-
-        if ($tagGroup) {
-            $this->group()->dissociate($tagGroup);
-            $this->save();
+        $this->group()->dissociate();
+        $this->save();
 
-            return $this;
-        } else {
-            throw new \RuntimeException('No Tag Group found: '. $groupName);
-        }
+        return $this;
     }
 
     /**

tests/TagGroupTest.php

@@ -5,12 +5,16 @@
 use Illuminate\Database\Eloquent\Model as Eloquent;
 use Conner\Tagging\Model\Tag;
 use Conner\Tagging\Model\TagGroup;
+use Illuminate\Foundation\Testing\WithFaker;
 
 class TagGroupTest extends TestCase
 {
+    use WithFaker;
+
     public function setUp(): void
     {
         parent::setUp();
+        $this->setUpFaker();
 
         Eloquent::unguard();
     }
@@ -24,73 +28,87 @@ public function test_create_group()
 
     public function test_tag_group_tags_list()
     {
-        $tag_group = $this->createTagGroup('MyTagGroup');
+        $tagGroup = $this->createTagGroup('MyTagGroup');
 
         $tag = $this->createTag();
 
-        $tag->setGroup($tag_group->name);
+        $tag->setGroup($tagGroup->name);
 
-        $this->assertEquals(1, $tag_group->tags()->count());
+        $this->assertEquals(1, $tagGroup->tags()->count());
+        $this->assertEquals($tagGroup->id, $tag->tag_group_id);
     }
 
     public function test_add_group_to_tag()
     {
-        $tag_group = $this->createTagGroup('MyTagGroup');
+        $tagGroup = $this->createTagGroup('MyTagGroup');
 
         $tag = $this->createTag();
 
-        $tag->setGroup($tag_group->name);
+        $tag->setGroup($tagGroup->name);
 
-        $this->assertCount(1, Tag::inGroup($tag_group->name)->get());
+        $this->assertCount(1, Tag::inGroup($tagGroup->name)->get());
 
         $this->assertTrue($tag->isInGroup('MyTagGroup'));
     }
 
     public function test_delete_group_from_tag()
     {
-        $tag_group = $this->createTagGroup('MyTagGroup');
+        $tagGroup = $this->createTagGroup('MyTagGroup');
 
         $tag = $this->createTag();
 
-        $tag->setGroup($tag_group->name);
+        $tag->setGroup($tagGroup->name);
 
-        $this->assertCount(1, Tag::inGroup($tag_group->name)->get());
+        $this->assertCount(1, Tag::inGroup($tagGroup->name)->get());
+        $this->assertEquals($tagGroup->id, $tag->tag_group_id);
 
-        $tag->removeGroup($tag_group->name);
+        $tag->removeGroup();
 
-        $this->assertCount(0, Tag::inGroup($tag_group->name)->get());
+        $this->assertCount(0, Tag::inGroup($tagGroup->name)->get());
 
         $this->assertFalse($tag->isInGroup('MyTagGroup'));
     }
 
+    public function test_removeGroup_with_no_group()
+    {
+        $tag = $this->createTag();
+
+        $tag->removeGroup();
+
+        $this->assertTrue(true); // no exceptions thrown
+    }
+
     public function test_delete_group_tag()
     {
-        $tag_group = $this->createTagGroup('MyTagGroup');
+        $tagGroup = $this->createTagGroup('MyTagGroup');
 
         $tag = $this->createTag();
 
-        $tag->setGroup($tag_group->name);
+        $tag->setGroup($tagGroup->name);
 
-        $tag_group->delete();
+        $tagGroup->delete();
 
         // unless you refresh the tag, it will still think there is a relationship
         $tag = $tag->fresh();
 
-        $this->assertFalse($tag_group->exists);
+        $this->assertFalse($tagGroup->exists);
 
         $this->assertNull($tag->group, 'The group should not exist on the tag after it is deleted');
 
         $this->assertFalse($tag->isInGroup('MyTagGroup'), 'The tag should not belong to a deleted group');
     }
 
-    private function createTagGroup($group_name = 'MyTagGroup')
+    private function createTagGroup($name = null): TagGroup
     {
+        if(is_null($name)) {
+            $name = $this->faker->name;
+        }
         return TagGroup::create([
-            'name' => $group_name
+            'name' => $name
         ]);
     }
 
-    private function createTag($name = 'Test Tag')
+    private function createTag($name = 'Test Tag'): Tag
     {
         $tag = new Tag();
         $tag->name = $name;