Merge pull request ppy#12059 from notbakaneko/feature/beatmap-tags-ruleset

Add per ruleset tag support

Author: nanaya

app/Http/Controllers/BeatmapTagsController.php

@@ -7,6 +7,7 @@
 
 namespace App\Http\Controllers;
 
+use App\Exceptions\InvariantException;
 use App\Models\Beatmap;
 use App\Models\BeatmapTag;
 use App\Models\Tag;
@@ -42,6 +43,10 @@ public function update($beatmapId, $tagId)
 
         $tag = Tag::findOrFail($tagId);
 
+        if ($tag->ruleset_id !== null && $tag->ruleset_id !== $beatmap->playmode) {
+            throw new InvariantException(osu_trans('beatmap_tags.update.invalid_ruleset'));
+        }
+
         $tag
             ->beatmapTags()
             ->firstOrCreate(['beatmap_id' => $beatmapId, 'user_id' => \Auth::user()->getKey()]);

app/Models/Tag.php

@@ -13,6 +13,7 @@
 /**
  * @property int $id
  * @property string $name
+ * @property ?int $ruleset_id
  * @property string $description
  * @property-read Collection<BeatmapTag> $beatmapTags
  */

database/factories/TagFactory.php

@@ -16,8 +16,8 @@ class TagFactory extends Factory
     public function definition(): array
     {
         return [
-            'name' => fn () => "Tag {$this->faker->unique()->word}",
             'description' => fn () => $this->faker->sentence,
+            'name' => fn () => "Tag {$this->faker->unique()->word}",
         ];
     }
 }

database/migrations/2025_03_28_000001_add_ruleset_id_to_tags.php

@@ -0,0 +1,29 @@
+<?php
+
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the GNU Affero General Public License v3.0.
+// See the LICENCE file in the repository root for full licence text.
+
+declare(strict_types=1);
+
+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('tags', function (Blueprint $table) {
+            $table->smallInteger('ruleset_id')->nullable()->unsigned()->after('name');
+            $table->dropIndex('tags_name_unique');
+        });
+    }
+
+    public function down(): void
+    {
+        Schema::table('tags', function (Blueprint $table) {
+            $table->unique('name', 'tags_name_unique');
+            $table->dropColumn('ruleset_id');
+        });
+    }
+};

resources/lang/en/beatmap_tags.php

@@ -0,0 +1,10 @@
+<?php
+
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the GNU Affero General Public License v3.0.
+// See the LICENCE file in the repository root for full licence text.
+
+return [
+    'update' => [
+        'invalid_ruleset' => 'This tag is not valid with this Beatmap.',
+    ],
+];

tests/Controllers/BeatmapTagsControllerTest.php

@@ -16,53 +16,61 @@
 
 class BeatmapTagsControllerTest extends TestCase
 {
-    private Tag $tag;
-    private Beatmap $beatmap;
-    private BeatmapTag $beatmapTag;
+    public static function dataProviderForUpdate(): array
+    {
+        return [
+            [0, null, true],
+            [0, 0, true],
+            [0, 1, false],
+        ];
+    }
 
-    public function testStore(): void
+    public function testDestroy(): void
     {
-        $user = User::factory()
-            ->has(Score::factory()->state(['beatmap_id' => $this->beatmap]), 'soloScores')
-            ->create();
+        $beatmapTag = BeatmapTag::factory()->create();
 
-        $this->expectCountChange(fn () => BeatmapTag::count(), 1);
+        $this->expectCountChange(fn () => BeatmapTag::count(), -1);
 
-        $this->actAsScopedUser($user);
+        $this->actAsScopedUser($beatmapTag->user);
         $this
-            ->put(route('api.beatmaps.tags.update', ['beatmap' => $this->beatmap->getKey(), 'tag' => $this->tag->getKey()]))
+            ->delete(route('api.beatmaps.tags.destroy', ['beatmap' => $beatmapTag->beatmap_id, 'tag' => $beatmapTag->tag_id]))
             ->assertSuccessful();
     }
 
-    public function testStoreNoScore(): void
+     /**
+      * @dataProvider dataProviderForUpdate
+      */
+    public function testUpdate(int $beatmapRulesetId, ?int $tagRulesetId, bool $successful): void
     {
-        $this->expectCountChange(fn () => BeatmapTag::count(), 0);
+        $tag = Tag::factory()->state(['ruleset_id' => $tagRulesetId])->create();
+        $beatmap = Beatmap::factory()->state(['playmode' => $beatmapRulesetId])->create();
 
-        $this->actAsScopedUser(User::factory()->create());
-        $this
-            ->put(route('api.beatmaps.tags.update', ['beatmap' => $this->beatmap->getKey(), 'tag' => $this->tag->getKey()]))
-            ->assertForbidden();
-    }
+        $user = User::factory()
+            ->has(Score::factory()->state(['beatmap_id' => $beatmap]), 'soloScores')
+            ->create();
 
-    public function testDestroy(): void
-    {
-        $this->expectCountChange(fn () => BeatmapTag::count(), -1);
+        $this->expectCountChange(fn () => BeatmapTag::count(), $successful ? 1 : 0);
 
-        $this->actAsScopedUser($this->beatmapTag->user);
-        $this
-            ->delete(route('api.beatmaps.tags.destroy', ['beatmap' => $this->beatmap->getKey(), 'tag' => $this->tag->getKey()]))
-            ->assertSuccessful();
+        $this->actAsScopedUser($user);
+        $request = $this->put(route('api.beatmaps.tags.update', ['beatmap' => $beatmap->getKey(), 'tag' => $tag->getKey()]));
+
+        if ($successful) {
+            $request->assertSuccessful();
+        } else {
+            $request->assertStatus(422);
+        }
     }
 
-    protected function setUp(): void
+    public function testUpdateNoScore(): void
     {
-        parent::setUp();
-
-        $this->tag = Tag::factory()->create();
-        $this->beatmap = Beatmap::factory()->create();
-        $this->beatmapTag = BeatmapTag::factory()->create([
-            'tag_id' => $this->tag,
-            'beatmap_id' => $this->beatmap,
-        ]);
+        $tag = Tag::factory()->create();
+        $beatmap = Beatmap::factory()->create();
+
+        $this->expectCountChange(fn () => BeatmapTag::count(), 0);
+
+        $this->actAsScopedUser(User::factory()->create());
+        $this
+            ->put(route('api.beatmaps.tags.update', ['beatmap' => $beatmap->getKey(), 'tag' => $tag->getKey()]))
+            ->assertForbidden();
     }
 }