Skip to content

Commit

Permalink
Merge pull request #15112 from marcusmoore/livewire-importer-improvem…
Browse files Browse the repository at this point in the history 
…ents

Improved handling attempted access of deleted files in importer
  • Loading branch information
@snipe
snipe authored Aug 20, 2024
2 parents 886514a + fa76566 commit f8c72fb
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 125 deletions.
204 changes: 103 additions & 101 deletions app/Livewire/Importer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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'
];

Expand All @@ -68,15 +60,13 @@ public function generate_field_map()
{
$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) {
Expand Down Expand Up @@ -115,76 +105,66 @@ private function getColumns($type)
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.
}
// 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
}
// 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
}
// 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'),
Expand Down Expand Up @@ -307,7 +287,7 @@ public function mount()
'manufacturer' => trans('general.manufacturer'),
];

$this->users_fields = [
$this->users_fields = [
'id' => trans('general.id'),
'company' => trans('general.company'),
'location' => trans('general.location'),
Expand All @@ -332,12 +312,12 @@ public function mount()
'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'),
Expand Down Expand Up @@ -510,19 +490,16 @@ public function mount()
];

$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');
Expand All @@ -531,37 +508,51 @@ public function selectFile($id)
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;

}

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->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';
}
}
$this->authorize('import');

$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()
Expand All @@ -570,11 +561,22 @@ public function clearMessage()
$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');
}
}
5 changes: 5 additions & 0 deletions database/factories/UserFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,11 @@ public function canViewReports()
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) {
Expand Down
1 change: 1 addition & 0 deletions resources/lang/en-US/admin/hardware/message.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
'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',
],
Expand Down
Loading

0 comments on commit f8c72fb

Please sign in to comment.