diff --git a/app/Couple.php b/app/Couple.php index e20b942..0a2f40a 100644 --- a/app/Couple.php +++ b/app/Couple.php @@ -23,12 +23,12 @@ class Couple extends Model public function husband() { - return $this->belongsTo(User::class); + return $this->belongsTo(User::class)->withDefault(['name' => 'N/A']); } public function wife() { - return $this->belongsTo(User::class); + return $this->belongsTo(User::class)->withDefault(['name' => 'N/A']); } public function childs() diff --git a/app/Http/Controllers/UsersController.php b/app/Http/Controllers/UsersController.php index 5619a86..2b2df4e 100644 --- a/app/Http/Controllers/UsersController.php +++ b/app/Http/Controllers/UsersController.php @@ -2,12 +2,12 @@ namespace App\Http\Controllers; -use DB; -use Storage; -use App\User; use App\Couple; -use Illuminate\Http\Request; use App\Http\Requests\Users\UpdateRequest; +use App\Jobs\Users\DeleteAndReplaceUser; +use App\User; +use Illuminate\Http\Request; +use Storage; class UsersController extends Controller { @@ -146,11 +146,7 @@ class UsersController extends Controller 'replacement_user_id.required' => __('validation.user.replacement_user_id.required'), ]); - DB::beginTransaction(); - $this->replaceUserOnUsersTable($user->id, $attributes['replacement_user_id']); - $this->replaceUserOnCouplesTable($user->id, $attributes['replacement_user_id']); - $user->delete(); - DB::commit(); + $this->dispatchNow(new DeleteAndReplaceUser($user, $attributes['replacement_user_id'])); return redirect()->route('users.show', $attributes['replacement_user_id']); } @@ -190,39 +186,6 @@ class UsersController extends Controller } /** - * Replace User Ids on users table. - * - * @param string $oldUserId - * @param string $replacementUserId - * @return void - */ - private function replaceUserOnUsersTable($oldUserId, $replacementUserId) - { - foreach (['father_id', 'mother_id', 'manager_id'] as $field) { - DB::table('users')->where($field, $oldUserId)->update([ - $field => $replacementUserId, - ]); - } - } - - /** - * Replace User Ids on couples table. - * - * @param string $oldUserId - * @param string $replacementUserId - * - * @return void - */ - private function replaceUserOnCouplesTable($oldUserId, $replacementUserId) - { - foreach (['husband_id', 'wife_id', 'manager_id'] as $field) { - DB::table('couples')->where($field, $oldUserId)->update([ - $field => $replacementUserId, - ]); - } - } - - /** * Get User list based on gender. * * @param int $genderId diff --git a/app/Jobs/Users/DeleteAndReplaceUser.php b/app/Jobs/Users/DeleteAndReplaceUser.php new file mode 100644 index 0000000..0f775b6 --- /dev/null +++ b/app/Jobs/Users/DeleteAndReplaceUser.php @@ -0,0 +1,95 @@ +user = $user; + $this->replacementUserId = $replacementUserId; + } + + public function handle() + { + $user = $this->user; + $replacementUserId = $this->replacementUserId; + + DB::beginTransaction(); + $this->replaceUserOnUsersTable($user->id, $replacementUserId); + $this->removeDuplicatedCouples($user->id, $replacementUserId); + $this->replaceUserOnCouplesTable($user->id, $replacementUserId); + $user->delete(); + DB::commit(); + } + + private function replaceUserOnCouplesTable($oldUserId, $replacementUserId) + { + DB::table('couples')->where('husband_id', $oldUserId)->update([ + 'husband_id' => $replacementUserId, + ]); + DB::table('couples')->where('wife_id', $oldUserId)->update([ + 'wife_id' => $replacementUserId, + ]); + DB::table('couples')->where('manager_id', $oldUserId)->update([ + 'manager_id' => $replacementUserId, + ]); + } + + private function removeDuplicatedCouples(string $oldUserId, string $replacementUserId) + { + $oldUser = User::find($oldUserId); + $replacementUser = User::find($replacementUserId); + + if ($replacementUser->gender_id == 1) { + $replacementUserCouples = Couple::where('husband_id', $replacementUserId)->get(); + } else { + $replacementUserCouples = Couple::where('wife_id', $replacementUserId)->get(); + } + if ($oldUser->gender_id == 1) { + $oldUserCouples = Couple::where('husband_id', $oldUserId)->get(); + } else { + $oldUserCouples = Couple::where('wife_id', $oldUserId)->get(); + } + + $couplesArray = []; + foreach ($replacementUserCouples as $replacementUserCouple) { + $couplesArray[$replacementUserCouple->id] = $replacementUserCouple->husband_id.'_'.$replacementUserCouple->wife_id; + } + foreach ($oldUserCouples as $oldUserCouple) { + $couplesArray[$oldUserCouple->id] = $oldUserCouple->husband_id.'_'.$oldUserCouple->wife_id; + } + $couplesArray = collect($couplesArray); + $deletableCouples = []; + if ($oldUser->gender_id == 1) { + foreach ($oldUserCouples as $oldUserCouple) { + $deletableCouples[] = $couplesArray->search($replacementUserId.'_'.$oldUserCouple->wife_id); + } + } else { + foreach ($oldUserCouples as $oldUserCouple) { + $deletableCouples[] = $couplesArray->search($oldUserCouple->husband_id.'_'.$replacementUserId); + } + } + + if ($deletableCouples) { + Couple::whereIn('id', $deletableCouples)->delete(); + } + } + + private function replaceUserOnUsersTable(string $oldUserId, string $replacementUserId) + { + foreach (['father_id', 'mother_id', 'manager_id'] as $field) { + DB::table('users')->where($field, $oldUserId)->update([ + $field => $replacementUserId, + ]); + } + } +} diff --git a/tests/Feature/UsersDeletionTest.php b/tests/Feature/UsersDeletionTest.php index bb2fe42..57f9344 100644 --- a/tests/Feature/UsersDeletionTest.php +++ b/tests/Feature/UsersDeletionTest.php @@ -2,10 +2,10 @@ namespace Tests\Feature; -use App\User; use App\Couple; -use Tests\TestCase; +use App\User; use Illuminate\Foundation\Testing\RefreshDatabase; +use Tests\TestCase; class UsersDeletionTest extends TestCase { @@ -253,4 +253,79 @@ class UsersDeletionTest extends TestCase $this->seeElement('option', ['value' => $replacementMaleUser->id]); $this->dontSeeElement('option', ['value' => $femaleUser->id]); } + + /** @test */ + public function bugfix_handle_duplicated_couple_on_user_deletion() + { + $manager = $this->loginAsUser(); + $singleWife = factory(User::class)->states('female')->create(['manager_id' => $manager->id]); + $oldUser = factory(User::class)->states('male')->create(['manager_id' => $manager->id]); + $replacementUser = factory(User::class)->states('male')->create(['manager_id' => $manager->id]); + $oldUserCouple = factory(Couple::class)->create([ + 'husband_id' => $oldUser->id, + 'wife_id' => $singleWife->id, + ]); + $duplicatedCouple = factory(Couple::class)->create([ + 'husband_id' => $replacementUser->id, + 'wife_id' => $singleWife->id, + ]); + + $this->visit(route('users.edit', [$oldUser, 'action' => 'delete'])); + $this->see(__('user.replace_delete_text')); + + $this->submitForm(__('user.replace_delete_button'), [ + 'replacement_user_id' => $replacementUser->id, + ]); + + $this->dontSeeInDatabase('users', ['id' => $oldUser->id]); + + $this->dontSeeInDatabase('couples', ['husband_id' => $oldUser->id]); + + $this->seeInDatabase('couples', [ + 'id' => $oldUserCouple->id, + 'husband_id' => $replacementUser->id, + 'wife_id' => $singleWife->id, + ]); + } + + /** @test */ + public function bugfix_handle_duplicated_couple_on_user_deletion_with_different_marriages() + { + $manager = $this->loginAsUser(); + $oldUserWife = factory(User::class)->states('female')->create(['manager_id' => $manager->id]); + $replacementUserWife = factory(User::class)->states('female')->create(['manager_id' => $manager->id]); + $oldUser = factory(User::class)->states('male')->create(['manager_id' => $manager->id]); + $replacementUser = factory(User::class)->states('male')->create(['manager_id' => $manager->id]); + $oldUserCouple = factory(Couple::class)->create([ + 'husband_id' => $oldUser->id, + 'wife_id' => $oldUserWife->id, + ]); + $newUserCouple = factory(Couple::class)->create([ + 'husband_id' => $replacementUser->id, + 'wife_id' => $replacementUserWife->id, + ]); + + $this->visit(route('users.edit', [$oldUser, 'action' => 'delete'])); + $this->see(__('user.replace_delete_text')); + + $this->submitForm(__('user.replace_delete_button'), [ + 'replacement_user_id' => $replacementUser->id, + ]); + + $this->dontSeeInDatabase('users', ['id' => $oldUser->id]); + + $this->dontSeeInDatabase('couples', ['husband_id' => $oldUser->id]); + + $this->seeInDatabase('couples', [ + 'id' => $oldUserCouple->id, + 'husband_id' => $replacementUser->id, + 'wife_id' => $oldUserWife->id, + ]); + + $this->seeInDatabase('couples', [ + 'id' => $newUserCouple->id, + 'husband_id' => $replacementUser->id, + 'wife_id' => $replacementUserWife->id, + ]); + } } diff --git a/tests/Unit/CoupleTest.php b/tests/Unit/CoupleTest.php index d8d1967..577ad32 100644 --- a/tests/Unit/CoupleTest.php +++ b/tests/Unit/CoupleTest.php @@ -20,6 +20,16 @@ class CoupleTest extends TestCase } /** @test */ + public function a_couples_husband_or_wife_has_a_default_name() + { + $couple = factory(Couple::class)->create(); + $couple->husband->delete(); + $couple->wife->delete(); + $this->assertEquals($couple->fresh()->husband->name, 'N/A'); + $this->assertEquals($couple->fresh()->wife->name, 'N/A'); + } + + /** @test */ public function a_couple_can_have_many_childs() { $couple = factory(Couple::class)->create();