fix: fix employees can be added with an email adress already used (#742)

officelifehq · Apr 10, 2021 · e71eaab · e71eaab
1 parent 679e31a
commit e71eaab
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 57 deletions.
diff --git a/app/Http/Controllers/Company/Adminland/AdminEmployeeController.php b/app/Http/Controllers/Company/Adminland/AdminEmployeeController.php
@@ -11,6 +11,7 @@
 use App\Helpers\NotificationHelper;
 use App\Http\Controllers\Controller;
 use Illuminate\Support\Facades\View;
+use App\Exceptions\EmailAlreadyUsedException;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use App\Services\Company\Adminland\Employee\LockEmployee;
 use App\Http\ViewHelpers\Adminland\AdminEmployeeViewHelper;
@@ -144,7 +145,13 @@ public function store(Request $request, int $companyId): JsonResponse
             'send_invitation' => $request->input('send_invitation'),
         ];
 
-        (new AddEmployeeToCompany)->execute($data);
+        try {
+            (new AddEmployeeToCompany)->execute($data);
+        } catch (EmailAlreadyUsedException $e) {
+            return response()->json([
+                'message' => trans('app.error_email_already_taken'),
+            ], 500);
+        }
 
         return response()->json([
             'company_id' => $companyId,

diff --git a/app/Services/Company/Adminland/Employee/AddEmployeeToCompany.php b/app/Services/Company/Adminland/Employee/AddEmployeeToCompany.php
@@ -10,11 +10,14 @@
 use App\Jobs\LogEmployeeAudit;
 use App\Models\Company\Company;
 use App\Models\Company\Employee;
+use App\Exceptions\EmailAlreadyUsedException;
 
 class AddEmployeeToCompany extends BaseService
 {
     private Employee $employee;
 
+    private array $data;
+
     /**
      * Get the validation rules that apply to the service.
      *
@@ -42,24 +45,20 @@ public function rules(): array
      */
     public function execute(array $data): Employee
     {
-        $this->validateRules($data);
-
-        $this->author($data['author_id'])
-            ->inCompany($data['company_id'])
-            ->asAtLeastHR()
-            ->canExecuteService();
+        $this->data = $data;
+        $this->validate();
 
-        $this->createEmployee($data);
+        $this->makeSureEmailIsUniqueInCompany();
+        $this->createEmployee();
+        $this->addHolidays();
 
-        $this->addHolidays($data);
+        $this->notifyEmployee();
 
-        $this->notifyEmployee($data);
+        $this->logAccountAudit();
 
-        $this->logAccountAudit($data);
-
-        if ($data['send_invitation']) {
+        if ($this->data['send_invitation']) {
             (new InviteEmployeeToBecomeUser)->execute([
-                'company_id' => $data['company_id'],
+                'company_id' => $this->data['company_id'],
                 'author_id' => $this->author->id,
                 'employee_id' => $this->employee->id,
             ]);
@@ -68,24 +67,38 @@ public function execute(array $data): Employee
         return $this->employee;
     }
 
-    /**
-     * Create the employee.
-     *
-     * @param array $data
-     *
-     * @return Employee
-     */
-    private function createEmployee(array $data): Employee
+    private function validate(): void
+    {
+        $this->validateRules($this->data);
+
+        $this->author($this->data['author_id'])
+            ->inCompany($this->data['company_id'])
+            ->asAtLeastHR()
+            ->canExecuteService();
+    }
+
+    private function makeSureEmailIsUniqueInCompany(): void
+    {
+        $employee = Employee::where('company_id', $this->data['company_id'])
+            ->where('email', $this->data['email'])
+            ->first();
+
+        if ($employee) {
+            throw new EmailAlreadyUsedException();
+        }
+    }
+
+    private function createEmployee(): Employee
     {
         $uuid = Str::uuid()->toString();
 
         $this->employee = Employee::create([
-            'company_id' => $data['company_id'],
+            'company_id' => $this->data['company_id'],
             'uuid' => $uuid,
-            'email' => $data['email'],
-            'first_name' => $data['first_name'],
-            'last_name' => $data['last_name'],
-            'permission_level' => $data['permission_level'],
+            'email' => $this->data['email'],
+            'first_name' => $this->data['first_name'],
+            'last_name' => $this->data['last_name'],
+            'permission_level' => $this->data['permission_level'],
         ]);
 
         LogEmployeeAudit::dispatch([
@@ -105,12 +118,10 @@ private function createEmployee(array $data): Employee
 
     /**
      * Add the default amount of holidays to this new employee.
-     *
-     * @param array $data
      */
-    private function addHolidays(array $data): void
+    private function addHolidays(): void
     {
-        $company = Company::find($data['company_id']);
+        $company = Company::find($this->data['company_id']);
 
         Employee::where('id', $this->employee->id)->update([
             'amount_of_allowed_holidays' => $company->getCurrentPTOPolicy()->default_amount_of_allowed_holidays,
@@ -119,12 +130,10 @@ private function addHolidays(array $data): void
 
     /**
      * Add a welcome message for the employee.
-     *
-     * @param array $data
      */
-    private function notifyEmployee(array $data): void
+    private function notifyEmployee(): void
     {
-        $company = Company::findOrFail($data['company_id']);
+        $company = Company::findOrFail($this->data['company_id']);
 
         NotifyEmployee::dispatch([
             'employee_id' => $this->employee->id,
@@ -135,24 +144,19 @@ private function notifyEmployee(array $data): void
         ])->onQueue('low');
     }
 
-    /**
-     * Add an audit log entry for this action.
-     *
-     * @param array $data
-     */
-    private function logAccountAudit(array $data): void
+    private function logAccountAudit(): void
     {
         LogAccountAudit::dispatch([
-            'company_id' => $data['company_id'],
+            'company_id' => $this->data['company_id'],
             'action' => 'employee_added_to_company',
             'author_id' => $this->author->id,
             'author_name' => $this->author->name,
             'audited_at' => Carbon::now(),
             'objects' => json_encode([
                 'employee_id' => $this->employee->id,
-                'employee_email' => $data['email'],
-                'employee_first_name' => $data['first_name'],
-                'employee_last_name' => $data['last_name'],
+                'employee_email' => $this->data['email'],
+                'employee_first_name' => $this->data['first_name'],
+                'employee_last_name' => $this->data['last_name'],
                 'employee_name' => $this->employee->name,
             ]),
         ])->onQueue('low');

diff --git a/resources/js/Pages/Adminland/Employee/Create.vue b/resources/js/Pages/Adminland/Employee/Create.vue
@@ -182,7 +182,6 @@ export default {
         errors: [],
       },
       loadingState: '',
-      errorTemplate: Error,
     };
   },
 
@@ -192,7 +191,7 @@ export default {
 
       axios.post('/' + this.$page.props.auth.company.id + '/account/employees', this.form)
         .then(response => {
-          localStorage.success = 'The employee has been added';
+          localStorage.success = this.$t('company.employee_new_success');
           this.$inertia.visit('/' + response.data.company_id + '/account/employees');
         })
         .catch(error => {

diff --git a/resources/lang/en/account.php b/resources/lang/en/account.php
@@ -70,6 +70,7 @@
     'employee_new_user_desc' => 'Can see all teams and employees, but can not manage the account or read private information.',
     'employee_new_send_email' => 'Send an email to this person with a link to setup the account.',
     'employee_new_send_email_optional' => 'This is optional - you will be able to invite this person later.',
+    'employee_new_success' => 'The employee has been added.',
 
     'teams_title' => 'All the teams listed in {company}',
     'teams_number_teams' => '{company} has one team. | {company} has {count} teams.',

diff --git a/resources/lang/en/app.php b/resources/lang/en/app.php
@@ -49,6 +49,7 @@
     'error_project_code_already_exists' => 'This project code is already used. Pick a new one.',
     'error_more' => 'See more...',
     'error_exception' => 'Exception:',
+    'error_email_already_taken' => 'This email is already taken.',
 
     'previous' => 'Previous',
     'next' => 'Next',

diff --git a/tests/Unit/Services/Company/Adminland/Employee/AddEmployeeToCompanyTest.php b/tests/Unit/Services/Company/Adminland/Employee/AddEmployeeToCompanyTest.php
@@ -4,6 +4,7 @@
 
 use Carbon\Carbon;
 use Tests\TestCase;
+use Faker\Factory as Faker;
 use App\Helpers\ImageHelper;
 use App\Jobs\NotifyEmployee;
 use App\Jobs\LogAccountAudit;
@@ -12,6 +13,7 @@
 use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Facades\Queue;
 use App\Models\Company\CompanyPTOPolicy;
+use App\Exceptions\EmailAlreadyUsedException;
 use Illuminate\Validation\ValidationException;
 use App\Exceptions\NotEnoughPermissionException;
 use App\Mail\Company\InviteEmployeeToBecomeUserMail;
@@ -26,16 +28,16 @@ class AddEmployeeToCompanyTest extends TestCase
     public function it_adds_an_employee_to_a_company_as_administrator(): void
     {
         $michael = $this->createAdministrator();
-        $this->executeService($michael, false);
-        $this->executeService($michael, true);
+        $this->executeService($michael, false, Faker::create()->email);
+        $this->executeService($michael, true, Faker::create()->email);
     }
 
     /** @test */
     public function it_adds_an_employee_to_a_company_as_hr(): void
     {
         $michael = $this->createHR();
-        $this->executeService($michael, false);
-        $this->executeService($michael, true);
+        $this->executeService($michael, false, Faker::create()->email);
+        $this->executeService($michael, true, Faker::create()->email);
     }
 
     /** @test */
@@ -44,8 +46,20 @@ public function normal_user_cant_execute_the_service(): void
         $michael = $this->createEmployee();
 
         $this->expectException(NotEnoughPermissionException::class);
-        $this->executeService($michael, true);
-        $this->executeService($michael, false);
+        $this->executeService($michael, true, Faker::create()->email);
+        $this->executeService($michael, false, Faker::create()->email);
+    }
+
+    /** @test */
+    public function it_cant_add_an_employee_with_an_email_address_already_used(): void
+    {
+        $michael = Employee::factory()->create([
+            'email' => 'dwight@dundermifflin.com',
+            'permission_level' => 100,
+        ]);
+
+        $this->expectException(EmailAlreadyUsedException::class);
+        $this->executeService($michael, false, 'dwight@dundermifflin.com');
     }
 
     /** @test */
@@ -63,7 +77,7 @@ public function it_fails_if_wrong_parameters_are_given(): void
         (new AddEmployeeToCompany)->execute($request);
     }
 
-    private function executeService(Employee $michael, bool $sendEmail): void
+    private function executeService(Employee $michael, bool $sendEmail, string $email): void
     {
         Queue::fake();
         Mail::fake();
@@ -78,7 +92,7 @@ private function executeService(Employee $michael, bool $sendEmail): void
         $request = [
             'company_id' => $michael->company_id,
             'author_id' => $michael->id,
-            'email' => 'dwight@dundermifflin.com',
+            'email' => $email,
             'first_name' => 'Dwight',
             'last_name' => 'Schrute',
             'permission_level' => config('officelife.permission_level.user'),
@@ -98,17 +112,18 @@ private function executeService(Employee $michael, bool $sendEmail): void
             'company_id' => $michael->company_id,
             'first_name' => 'Dwight',
             'last_name' => 'Schrute',
+            'email' => $email,
             'amount_of_allowed_holidays' => 30,
         ]);
 
         $this->assertNotNull(ImageHelper::getAvatar($dwight));
 
-        Queue::assertPushed(LogAccountAudit::class, function ($job) use ($michael, $dwight) {
+        Queue::assertPushed(LogAccountAudit::class, function ($job) use ($michael, $dwight, $email) {
             return $job->auditLog['action'] === 'employee_added_to_company' &&
                 $job->auditLog['author_id'] === $michael->id &&
                 $job->auditLog['objects'] === json_encode([
                     'employee_id' => $dwight->id,
-                    'employee_email' => 'dwight@dundermifflin.com',
+                    'employee_email' => $email,
                     'employee_first_name' => 'Dwight',
                     'employee_last_name' => 'Schrute',
                     'employee_name' => 'Dwight Schrute',