diff --git a/app/Livewire/Importer.php b/app/Livewire/Importer.php index 32dd7912f..3c6f7990e 100644 --- a/app/Livewire/Importer.php +++ b/app/Livewire/Importer.php @@ -3,30 +3,25 @@ namespace App\Livewire; use App\Models\CustomField; -use Livewire\Component; - use App\Models\Import; use Illuminate\Support\Facades\Storage; - -use Illuminate\Foundation\Auth\Access\AuthorizesRequests; - +use Livewire\Attributes\Computed; +use Livewire\Component; class Importer extends Component { - use AuthorizesRequests; - - public $files; - - public $progress; //upload progress - '-1' means don't show + public $progress = -1; //upload progress - '-1' means don't show public $progress_message; - public $progress_bar_class; + public $progress_bar_class = 'progress-bar-warning'; public $message; //status/error message? public $message_type; //success/error? //originally from ImporterFile public $import_errors; // - public ?Import $activeFile = null; + public $activeFileId; + public $headerRow = []; + public $typeOfImport; public $importTypes; public $columnOptions; public $statusType; @@ -35,7 +30,6 @@ class Importer extends Component public $send_welcome; public $run_backup; public $field_map; // we need a separate variable for the field-mapping, because the keys in the normal array are too complicated for Livewire to understand - public $file_id; // TODO: I can't figure out *why* we need this, but it really seems like we do. I can't seem to pull the id from the activeFile for some reason? // Make these variables public - we set the properties in the constructor so we can localize them (versus the old static arrays) public $accessories_fields; @@ -51,10 +45,8 @@ class Importer extends Component 'files.*.file_path' => 'required|string', 'files.*.created_at' => 'required|string', 'files.*.filesize' => 'required|integer', - 'activeFile' => 'Import', - 'activeFile.import_type' => 'string', - 'activeFile.field_map' => 'array', - 'activeFile.header_row' => 'array', + 'headerRow' => 'array', + 'typeOfImport' => 'string', 'field_map' => 'array' ]; @@ -68,15 +60,13 @@ class Importer extends Component { $tmp = array(); if ($this->activeFile) { - $tmp = array_combine($this->activeFile->header_row, $this->field_map); + $tmp = array_combine($this->headerRow, $this->field_map); $tmp = array_filter($tmp); } return json_encode($tmp); } - - private function getColumns($type) { switch ($type) { @@ -115,76 +105,66 @@ class Importer extends Component return $results; } - public function updating($name, $new_import_type) + public function updatingTypeOfImport($type) { - if ($name == "activeFile.import_type") { - - // go through each header, find a matching field to try and map it to. - foreach ($this->activeFile->header_row as $i => $header) { - // do we have something mapped already? - if (array_key_exists($i, $this->field_map)) { - // yes, we do. Is it valid for this type of import? - // (e.g. the import type might have been changed...?) - if (array_key_exists($this->field_map[$i], $this->columnOptions[$new_import_type])) { - //yes, this key *is* valid. Continue on to the next field. - continue; - } else { - //no, this key is *INVALID* for this import type. Better set it to null - // and we'll hope that the $aliases_fields or something else picks it up. - $this->field_map[$i] = null; // fingers crossed! But it's not likely, tbh. - } // TODO - strictly speaking, this isn't necessary here I don't think. + // go through each header, find a matching field to try and map it to. + foreach ($this->headerRow as $i => $header) { + // do we have something mapped already? + if (array_key_exists($i, $this->field_map)) { + // yes, we do. Is it valid for this type of import? + // (e.g. the import type might have been changed...?) + if (array_key_exists($this->field_map[$i], $this->columnOptions[$type])) { + //yes, this key *is* valid. Continue on to the next field. + continue; + } else { + //no, this key is *INVALID* for this import type. Better set it to null + // and we'll hope that the $aliases_fields or something else picks it up. + $this->field_map[$i] = null; // fingers crossed! But it's not likely, tbh. + } // TODO - strictly speaking, this isn't necessary here I don't think. + } + // first, check for exact matches + foreach ($this->columnOptions[$type] as $v => $text) { + if (strcasecmp($text, $header) === 0) { // case-INSENSITIVe on purpose! + $this->field_map[$i] = $v; + continue 2; //don't bother with the alias check, go to the next header } - // first, check for exact matches - foreach ($this->columnOptions[$new_import_type] as $value => $text) { - if (strcasecmp($text, $header) === 0) { // case-INSENSITIVe on purpose! - $this->field_map[$i] = $value; - continue 2; //don't bother with the alias check, go to the next header - } - } - // if you got here, we didn't find a match. Try the $aliases_fields - foreach ($this->aliases_fields as $key => $alias_values) { - foreach ($alias_values as $alias_value) { - if (strcasecmp($alias_value, $header) === 0) { // aLsO CaSe-INSENSitiVE! - // Make *absolutely* sure that this key actually _exists_ in this import type - - // you can trigger this by importing accessories with a 'Warranty' column (which don't exist - // in "Accessories"!) - if (array_key_exists($key, $this->columnOptions[$new_import_type])) { - $this->field_map[$i] = $key; - continue 3; // bust out of both of these loops; as well as the surrounding one - e.g. move on to the next header - } + } + // if you got here, we didn't find a match. Try the $aliases_fields + foreach ($this->aliases_fields as $key => $alias_values) { + foreach ($alias_values as $alias_value) { + if (strcasecmp($alias_value, $header) === 0) { // aLsO CaSe-INSENSitiVE! + // Make *absolutely* sure that this key actually _exists_ in this import type - + // you can trigger this by importing accessories with a 'Warranty' column (which don't exist + // in "Accessories"!) + if (array_key_exists($key, $this->columnOptions[$type])) { + $this->field_map[$i] = $key; + continue 3; // bust out of both of these loops; as well as the surrounding one - e.g. move on to the next header } } } - // and if you got here, we got nothing. Let's recommend 'null' - $this->field_map[$i] = null; // Booooo :( } + // and if you got here, we got nothing. Let's recommend 'null' + $this->field_map[$i] = null; // Booooo :( } } - public function boot() { // FIXME - delete or undelete. - ///////$this->activeFile = null; // I do *not* understand why I have to do this, but, well, whatever. - } - - public function mount() { $this->authorize('import'); - $this->progress = -1; // '-1' means 'don't show the progressbar' - $this->progress_bar_class = 'progress-bar-warning'; $this->importTypes = [ - 'asset' => trans('general.assets'), - 'accessory' => trans('general.accessories'), + 'asset' => trans('general.assets'), + 'accessory' => trans('general.accessories'), 'consumable' => trans('general.consumables'), - 'component' => trans('general.components'), - 'license' => trans('general.licenses'), - 'user' => trans('general.users'), - 'location' => trans('general.locations'), + 'component' => trans('general.components'), + 'license' => trans('general.licenses'), + 'user' => trans('general.users'), + 'location' => trans('general.locations'), ]; /** * These are the item-type specific columns */ - $this->accessories_fields = [ + $this->accessories_fields = [ 'company' => trans('general.company'), 'location' => trans('general.location'), 'quantity' => trans('general.qty'), @@ -307,7 +287,7 @@ class Importer extends Component 'manufacturer' => trans('general.manufacturer'), ]; - $this->users_fields = [ + $this->users_fields = [ 'id' => trans('general.id'), 'company' => trans('general.company'), 'location' => trans('general.location'), @@ -332,12 +312,12 @@ class Importer extends Component 'website' => trans('general.website'), 'avatar' => trans('general.image'), 'gravatar' => trans('general.importer.gravatar'), - 'start_date' => trans('general.start_date'), - 'end_date' => trans('general.end_date'), - 'employee_num' => trans('general.employee_number'), + 'start_date' => trans('general.start_date'), + 'end_date' => trans('general.end_date'), + 'employee_num' => trans('general.employee_number'), ]; - $this->locations_fields = [ + $this->locations_fields = [ 'name' => trans('general.item_name_var', ['item' => trans('general.location')]), 'address' => trans('general.address'), 'address2' => trans('general.importer.address2'), @@ -510,19 +490,16 @@ class Importer extends Component ]; $this->columnOptions[''] = $this->getColumns(''); //blank mode? I don't know what this is supposed to mean - foreach($this->importTypes AS $type => $name) { + foreach ($this->importTypes as $type => $name) { $this->columnOptions[$type] = $this->getColumns($type); } - if ($this->activeFile) { - $this->field_map = $this->activeFile->field_map ? array_values($this->activeFile->field_map) : []; - } } public function selectFile($id) { $this->clearMessage(); - $this->activeFile = Import::find($id); + $this->activeFileId = $id; if (!$this->activeFile) { $this->message = trans('admin/hardware/message.import.file_missing'); @@ -531,15 +508,17 @@ class Importer extends Component return; } + $this->headerRow = $this->activeFile->header_row; + $this->typeOfImport = $this->activeFile->import_type; + $this->field_map = null; - foreach($this->activeFile->header_row as $element) { - if(isset($this->activeFile->field_map[$element])) { + foreach ($this->headerRow as $element) { + if (isset($this->activeFile->field_map[$element])) { $this->field_map[] = $this->activeFile->field_map[$element]; } else { $this->field_map[] = null; // re-inject the 'nulls' if a file was imported with some 'Do Not Import' settings } } - $this->file_id = $id; $this->import_errors = null; $this->statusText = null; @@ -547,21 +526,33 @@ class Importer extends Component public function destroy($id) { - // TODO: why don't we just do File::find($id)? This seems dumb. - foreach($this->files as $file) { - if ($id == $file->id) { - if (Storage::delete('private_uploads/imports/'.$file->file_path)) { - $file->delete(); + $this->authorize('import'); - $this->message = trans('admin/hardware/message.import.file_delete_success'); - $this->message_type = 'success'; - return; - } else { - $this->message = trans('admin/hardware/message.import.file_delete_error'); - $this->message_type = 'danger'; - } - } + $import = Import::find($id); + + // Check that the import wasn't deleted after while page was already loaded... + // @todo: next up...handle the file being missing for other interactions... + // for example having an import open in two tabs, deleting it, and then changing + // the import type in the other tab. The error message below wouldn't display in that case. + if (!$import) { + $this->message = trans('admin/hardware/message.import.file_already_deleted'); + $this->message_type = 'danger'; + + return; } + + if (Storage::delete('private_uploads/imports/' . $import->file_path)) { + $import->delete(); + $this->message = trans('admin/hardware/message.import.file_delete_success'); + $this->message_type = 'success'; + + unset($this->files); + + return; + } + + $this->message = trans('admin/hardware/message.import.file_delete_error'); + $this->message_type = 'danger'; } public function clearMessage() @@ -570,11 +561,22 @@ class Importer extends Component $this->message_type = null; } + #[Computed] + public function files() + { + return Import::orderBy('id', 'desc')->get(); + } + + #[Computed] + public function activeFile() + { + return Import::find($this->activeFileId); + } + public function render() { - $this->files = Import::orderBy('id','desc')->get(); //HACK - slows down renders. return view('livewire.importer') - ->extends('layouts.default') - ->section('content'); + ->extends('layouts.default') + ->section('content'); } } diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index 656fc8672..151c11431 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -296,6 +296,11 @@ class UserFactory extends Factory return $this->appendPermission(['reports.view' => '1']); } + public function canImport() + { + return $this->appendPermission(['import' => '1']); + } + private function appendPermission(array $permission) { return $this->state(function ($currentState) use ($permission) { diff --git a/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index d06bf4a0e..041d32f56 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -58,6 +58,7 @@ return [ 'file_delete_success' => 'Your file has been been successfully deleted', 'file_delete_error' => 'The file was unable to be deleted', 'file_missing' => 'The file selected is missing', + 'file_already_deleted' => 'The file selected was already deleted', 'header_row_has_malformed_characters' => 'One or more attributes in the header row contain malformed UTF-8 characters', 'content_row_has_malformed_characters' => 'One or more attributes in the first row of content contain malformed UTF-8 characters', ], diff --git a/resources/views/livewire/importer.blade.php b/resources/views/livewire/importer.blade.php index 92dfd128a..e69a38b18 100644 --- a/resources/views/livewire/importer.blade.php +++ b/resources/views/livewire/importer.blade.php @@ -119,9 +119,9 @@ - @foreach($files as $currentFile) + @foreach($this->files as $currentFile) - + {{ $currentFile->file_path }} {{ Helper::getFormattedDateObject($currentFile->created_at, 'datetime', false) }} {{ Helper::formatFilesizeUnits($currentFile->filesize) }} @@ -130,25 +130,25 @@ {{ trans('general.import') }} - + - @if( $currentFile && $activeFile && ($currentFile->id == $activeFile->id)) + @if( $currentFile && $this->activeFile && ($currentFile->id == $this->activeFile->id))
-
- @if($activeFile->header_row) + @if(! empty($headerRow)) - @foreach($activeFile->header_row as $index => $header) + @foreach($headerRow as $index => $header)
- {{ Form::select('field_map.'.$index, $columnOptions[$activeFile->import_type], @$field_map[$index], + {{ Form::select('field_map.'.$index, $columnOptions[$typeOfImport], @$field_map[$index], [ 'class' => 'mappings livewire-select2', 'placeholder' => trans('general.importer.do_not_import'), @@ -238,9 +238,9 @@ ]) }}
- @if (($activeFile->first_row) && (array_key_exists($index, $activeFile->first_row))) + @if (($this->activeFile->first_row) && (array_key_exists($index, $this->activeFile->first_row)))
-

{{ str_limit($activeFile->first_row[$index], 50, '...') }}

+

{{ str_limit($this->activeFile->first_row[$index], 50, '...') }}

@else @php @@ -256,7 +256,7 @@
- {{ trans('general.cancel') }} + {{ trans('general.cancel') }}
@@ -272,10 +272,10 @@ @else
- {{ trans('general.cancel') }} + {{ trans('general.cancel') }}
- @endif {{-- end of if ... activeFile->import_type --}} + @endif {{-- end of if ... $typeOfImport --}}
@@ -333,7 +333,7 @@ // we have to hook up to the `` at the root of this display, // because the #import button isn't visible until you click an import_type $('#upload-table').on('click', '#import', function () { - if (!$wire.$get('activeFile.import_type')) { + if (!$wire.$get('typeOfImport')) { $wire.$set('statusType', 'error'); $wire.$set('statusText', "An import type is required... "); //TODO: translate? return; @@ -345,15 +345,15 @@ // console.warn("Here is the mappings:") // console.dir(mappings) // console.warn("Uh, active file id is, I guess: "+$wire.$get('activeFile.id')) - var this_file = $wire.$get('file_id'); // okay, I actually don't know what I'm doing here. + var file_id = $wire.$get('activeFileId'); $.post({ {{-- I want to do something like: route('api.imports.importFile', $activeFile->id) }} --}} - url: "api/v1/imports/process/"+this_file, // maybe? Good a guess as any..FIXME. HARDCODED DUMB FILE + url: "api/v1/imports/process/"+file_id, // maybe? Good a guess as any..FIXME. HARDCODED DUMB FILE contentType: 'application/json', data: JSON.stringify({ 'import-update': !!$wire.$get('update'), 'send-welcome': !!$wire.$get('send_welcome'), - 'import-type': $wire.$get('activeFile.import_type'), + 'import-type': $wire.$get('typeOfImport'), 'run-backup': !!$wire.$get('run_backup'), 'column-mappings': mappings }), @@ -393,7 +393,7 @@ } } - $wire.$set('activeFile', null); //$wire.$set('hideDetails') + $wire.$set('activeFileId', null); //$wire.$set('hideDetails') }); }) return false; diff --git a/tests/Feature/Livewire/ImporterTest.php b/tests/Feature/Livewire/ImporterTest.php new file mode 100644 index 000000000..b8af8f9bc --- /dev/null +++ b/tests/Feature/Livewire/ImporterTest.php @@ -0,0 +1,25 @@ +canImport()->create()) + ->test(Importer::class) + ->assertStatus(200); + } + + public function testRequiresPermission() + { + Livewire::actingAs(User::factory()->create()) + ->test(Importer::class) + ->assertStatus(403); + } +}