feat: check csv validity before processing it (#1086)

app/Exceptions/MalformedCSVException.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+class MalformedCSVException extends Exception
+{
+}

app/Services/Company/Adminland/Employee/ImportEmployeesFromCSV.php

@@ -13,6 +13,7 @@
 use App\Services\DispatchableService;
 use App\Models\Company\ImportJobReport;
 use Illuminate\Support\Facades\Storage;
+use App\Exceptions\MalformedCSVException;
 use Illuminate\Support\Facades\Validator;
 use Spatie\SimpleExcel\SimpleExcelReader;
 
@@ -109,6 +110,8 @@ private function readFile(): void
 
             Storage::disk('local')->put($filePath, $response->body());
 
+            $this->checkHeaderValidity($filePath);
+
             $reader = SimpleExcelReader::create(Storage::disk('local')->path($filePath))
                 ->trimHeaderRow()
                 ->headersToSnakeCase();
@@ -127,6 +130,32 @@ private function readFile(): void
         }
     }
 
+    /**
+     * Make sure the headers in the file are valid.
+     *
+     * @param string $filePath
+     */
+    private function checkHeaderValidity(string $filePath): void
+    {
+        $actualHeadersInCsv = SimpleExcelReader::create(Storage::disk('local')->path($filePath))->getHeaders();
+
+        if (count($actualHeadersInCsv) != 3) {
+            throw new MalformedCSVException();
+        }
+
+        if (Str::lower($actualHeadersInCsv[0]) != 'first name') {
+            throw new MalformedCSVException();
+        }
+
+        if (Str::lower($actualHeadersInCsv[1]) != 'last name') {
+            throw new MalformedCSVException();
+        }
+
+        if (Str::lower($actualHeadersInCsv[2]) != 'email') {
+            throw new MalformedCSVException();
+        }
+    }
+
     private function handleRow(array $rowProperties): void
     {
         $skipReason = '';

resources/js/Pages/Adminland/Employee/Archives/Show.vue

@@ -62,7 +62,7 @@
       <!-- BODY -->
       <div class="mw7 center br3 mb5 bg-white box restricted relative z-1">
         <div class="mt5">
-          <div class="pa3 bb bb-gray">
+          <div class="pa3">
             <!-- Title when import is waiting processing -->
             <h2 v-if="localReport.status === 'created'" class="tc normal mb4">
               {{ $t('account.import_employees_show_title_created') }}
@@ -91,6 +91,14 @@
               <help :url="$page.props.help_links.import_employees" :top="'1px'" />
             </h2>
 
+            <!-- Title when import is in error -->
+            <h2 v-else-if="localReport.status === 'failed'" class="tc normal mb4">
+              {{ $t('account.import_employees_show_title_failed') }}
+
+              <help :url="$page.props.help_links.import_employees" :top="'1px'" />
+            </h2>
+
+            <!-- Summary of the different import steps -->
             <div v-if="localReport.status !== 'created' && localReport.status !== 'started'" class="flex justify-around items-center mb4">
               <div class="">
                 <span class="db gray mb2 f6">
@@ -303,7 +311,8 @@ export default {
       if (this.$page.component === 'Adminland/Employee/Archives/Show'
         && this.loadingState === null
         && this.localReport.status !== 'imported'
-        && this.localReport.status !== 'uploaded') {
+        && this.localReport.status !== 'uploaded'
+        && this.localReport.status !== 'failed') {
         axios.get(route('account.employees.upload.archive.show', {
           company: this.$page.props.auth.company.id,
           archive: this.localReport.id,

resources/lang/en/account.php

@@ -598,6 +598,7 @@
     'import_employees_show_title_started' => 'Import started on {date}…',
     'import_employees_show_title_uploaded' => 'You are about to import new employees',
     'import_employees_show_title_imported' => 'Entries you have imported on {date}',
+    'import_employees_show_title_failed' => 'This import has failed',
     'import_employees_show_title_number_entries' => 'Number of entries in the file',
     'import_employees_show_title_number_entries_errors' => 'Entries in errors',
     'import_employees_show_title_number_entries_import' => 'Entries we can import',

tests/Fixtures/Services/Adminland/Employee/Imports/working.csv

@@ -1,16 +1,16 @@
-First Name,Last Name,Email,Permission Level, Send email
-Eric,Ramzy,eric.ramzy,200,true
-Henri,Troyat,henri@troyat.com,100,false
-Cata,Strophe,,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
-Al,Berri,al@berri.com,200,true
-Regis,Alexis,regis@berri,200,true
+First Name,Last Name,Email
+Eric,Ramzy,eric.ramzy
+Henri,Troyat,henri@troyat.com
+Cata,Strophe
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri
+Al,Berri,al@berri.com
+Regis,Alexis,regis@berri

tests/Unit/Services/Company/Adminland/Employee/ImportEmployeesFromCSVTest.php

@@ -9,6 +9,7 @@
 use Illuminate\Support\Facades\Http;
 use App\Models\Company\ImportJobReport;
 use Illuminate\Support\Facades\Storage;
+use App\Exceptions\MalformedCSVException;
 use Illuminate\Http\Client\RequestException;
 use Illuminate\Validation\ValidationException;
 use App\Exceptions\NotEnoughPermissionException;
@@ -78,7 +79,7 @@ public function normal_employees_cant_execute_the_service(): void
     }
 
     /** @test */
-    public function it_fails_if_file_not_exist(): void
+    public function it_fails_if_file_does_not_exist(): void
     {
         $michael = $this->createHR();
         $importJob = ImportJob::factory()->create([
@@ -110,6 +111,40 @@ public function it_fails_if_file_not_exist(): void
         (new ImportEmployeesFromCSV($request))->handle();
     }
 
+    /** @test */
+    public function it_fails_if_csv_does_not_have_the_right_header(): void
+    {
+        $michael = $this->createHR();
+        $importJob = ImportJob::factory()->create([
+            'company_id' => $michael->company_id,
+        ]);
+        $report = ImportJobReport::factory()->create([
+            'import_job_id' => $importJob->id,
+        ]);
+        $file = File::factory()->create([
+            'company_id' => $michael->company_id,
+            'cdn_url' => 'http://url.com/',
+            'name' => 'malformed.csv',
+        ]);
+
+        Storage::fake('local');
+
+        $body = file_get_contents(base_path('tests/Fixtures/Services/Adminland/Employee/Imports/malformed.csv'));
+        Http::fake([
+            'url.com/malformed.csv' => Http::response($body, 200),
+        ]);
+
+        $request = [
+            'company_id' => $michael->company_id,
+            'author_id' => $michael->id,
+            'import_job_id' => $importJob->id,
+            'file_id' => $file->id,
+        ];
+
+        $this->expectException(MalformedCSVException::class);
+        (new ImportEmployeesFromCSV($request))->handle();
+    }
+
     /** @test */
     public function it_fails_if_wrong_parameters_are_given(): void
     {
@@ -125,7 +160,7 @@ public function it_fails_if_wrong_parameters_are_given(): void
     }
 
     /** @test */
-    public function it_save_state_when_failing(): void
+    public function it_saves_state_when_failing(): void
     {
         $michael = $this->createEmployee();
         $importJob = ImportJob::factory()->create([