refactor validation

app/Http/Controllers/PlayerController.php

@@ -17,6 +17,7 @@ use Blessing\Filter;
 use Blessing\Rejection;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Http\Request;
+use Illuminate\Validation\Rule;
 
 class PlayerController extends Controller
 {
@@ -78,6 +79,7 @@ class PlayerController extends Controller
                 new Rules\PlayerName(),
                 'min:'.option('player_name_length_min'),
                 'max:'.option('player_name_length_max'),
+                'unique:players',
             ],
         ])['name'];
         $name = $filter->apply('new_player_name', $name);
@@ -89,10 +91,6 @@ class PlayerController extends Controller
             return json($can->getReason(), 1);
         }
 
-        if (Player::where('name', $name)->count() > 0) {
-            return json(trans('user.player.add.repeated'), 6);
-        }
-
         if ($user->score < (int) option('score_per_player')) {
             return json(trans('user.player.add.lack-score'), 7);
         }
@@ -162,6 +160,7 @@ class PlayerController extends Controller
                 new Rules\PlayerName(),
                 'min:'.option('player_name_length_min'),
                 'max:'.option('player_name_length_max'),
+                Rule::unique('players')->ignore($pid),
             ],
         ])['name'];
         $name = $filter->apply('new_player_name', $name);
@@ -174,10 +173,6 @@ class PlayerController extends Controller
             return json($can->getReason(), 1);
         }
 
-        if (Player::where('name', $name)->count() > 0) {
-            return json(trans('user.player.rename.repeated'), 6);
-        }
-
         $old = $player->replicate();
         $player->name = $name;
         $player->save();

tests/HttpTest/ControllersTest/PlayerControllerTest.php

@@ -60,8 +60,13 @@ class PlayerControllerTest extends TestCase
             ['name' => 'yjsnpi']
         )->assertJsonValidationErrors('name');
 
-        // Lack of score
+        // with an existed player name
         option(['player_name_rule' => 'official']);
+        $existed = factory(Player::class)->create();
+        $this->postJson('/user/player/add', ['name' => $existed->name])
+            ->assertJsonValidationErrors('name');
+
+        // Lack of score
         $user = factory(User::class)->create(['score' => 0]);
         $this->actingAs($user)->postJson(
             '/user/player/add',
@@ -140,16 +145,6 @@ class PlayerControllerTest extends TestCase
             User::find($user->uid)->score
         );
 
-        // Add a existed player
-        Event::fake();
-        $this->postJson('/user/player/add', ['name' => '角色名'])
-            ->assertJson([
-                'code' => 6,
-                'message' => trans('user.player.add.repeated'),
-            ]);
-        Event::assertNotDispatched('player.adding');
-        Event::assertNotDispatched('player.added');
-
         // Single player
         option(['single_player' => true]);
         $this->postJson('/user/player/add', ['name' => 'abc'])
@@ -260,19 +255,10 @@ class PlayerControllerTest extends TestCase
         $this->postJson('/user/player/rename/'.$player->pid, ['name' => '\\'])
             ->assertJsonValidationErrors('name');
 
-        // Use a duplicated player name
+        // with an existed player name
-        $name = factory(Player::class)->create()->name;
+        $existed = factory(Player::class)->create();
-        $this->postJson('/user/player/rename/'.$player->pid, ['name' => $name])
+        $this->postJson('/user/player/rename/'.$player->pid, ['name' => $existed->name])
-            ->assertJson([
+            ->assertJsonValidationErrors('name');
-                'code' => 6,
-                'message' => trans('user.player.rename.repeated'),
-            ]);
-        Event::assertDispatched('player.renaming');
-        $filter->assertApplied('new_player_name', function ($newName) use ($name) {
-            $this->assertEquals($name, $newName);
-
-            return true;
-        });
 
         // Rejected by filter
         $filter = Fakes\Filter::fake();
@@ -315,6 +301,11 @@ class PlayerControllerTest extends TestCase
 
             return true;
         });
+        $filter->assertApplied('new_player_name', function ($name) {
+            $this->assertEquals('new_name', $name);
+
+            return true;
+        });
 
         // Single player
         option(['single_player' => true]);