From 8f47cf83867f242673a5790b75d3165080d6b1ac Mon Sep 17 00:00:00 2001 From: Regis Freyd Date: Wed, 16 Jun 2021 07:39:22 -0400 Subject: [PATCH] feat: check csv validity before processing it (#1086) --- app/Exceptions/MalformedCSVException.php | 9 +++++ .../Employee/ImportEmployeesFromCSV.php | 29 ++++++++++++++ .../Adminland/Employee/Archives/Show.vue | 13 ++++++- resources/lang/en/account.php | 1 + .../Adminland/Employee/Imports/working.csv | 32 +++++++-------- .../Employee/ImportEmployeesFromCSVTest.php | 39 ++++++++++++++++++- 6 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 app/Exceptions/MalformedCSVException.php diff --git a/app/Exceptions/MalformedCSVException.php b/app/Exceptions/MalformedCSVException.php new file mode 100644 index 000000000..9a5e47f6b --- /dev/null +++ b/app/Exceptions/MalformedCSVException.php @@ -0,0 +1,9 @@ +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 = ''; diff --git a/resources/js/Pages/Adminland/Employee/Archives/Show.vue b/resources/js/Pages/Adminland/Employee/Archives/Show.vue index 98dd06330..d734caad4 100644 --- a/resources/js/Pages/Adminland/Employee/Archives/Show.vue +++ b/resources/js/Pages/Adminland/Employee/Archives/Show.vue @@ -62,7 +62,7 @@
-
+

{{ $t('account.import_employees_show_title_created') }} @@ -91,6 +91,14 @@

+ +

+ {{ $t('account.import_employees_show_title_failed') }} + + +

+ +
@@ -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, diff --git a/resources/lang/en/account.php b/resources/lang/en/account.php index 2b5fe0e16..1c295e06e 100644 --- a/resources/lang/en/account.php +++ b/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', diff --git a/tests/Fixtures/Services/Adminland/Employee/Imports/working.csv b/tests/Fixtures/Services/Adminland/Employee/Imports/working.csv index fd11e7cc3..e20068d29 100644 --- a/tests/Fixtures/Services/Adminland/Employee/Imports/working.csv +++ b/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 diff --git a/tests/Unit/Services/Company/Adminland/Employee/ImportEmployeesFromCSVTest.php b/tests/Unit/Services/Company/Adminland/Employee/ImportEmployeesFromCSVTest.php index a024d88b8..a9155aece 100644 --- a/tests/Unit/Services/Company/Adminland/Employee/ImportEmployeesFromCSVTest.php +++ b/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([