DRCIT/snipe-it
1
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2024-12-25 05:34:06 -08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #15112 from marcusmoore/livewire-importer-improvements

Browse source 
Improved handling attempted access of deleted files in importer
This commit is contained in:
snipe 2024-08-20 09:57:03 +01:00 committed by GitHub
parent 886514a25f fa765667f2
commit f8c72fb0ac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 157 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

108
app/Livewire/Importer.php
View file

@ -3,30 +3,25 @@
namespace App\Livewire; namespace App\Livewire;
use App\Models\CustomField; use App\Models\CustomField;
use Livewire\Component;
use App\Models\Import; use App\Models\Import;
use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Storage;
use Livewire\Attributes\Computed;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Component;
class Importer extends Component class Importer extends Component
{ {
use AuthorizesRequests; public $progress = -1; //upload progress - '-1' means don't show
public $files;
public $progress; //upload progress - '-1' means don't show
public $progress_message; public $progress_message;
public $progress_bar_class; public $progress_bar_class = 'progress-bar-warning';
public $message; //status/error message? public $message; //status/error message?
public $message_type; //success/error? public $message_type; //success/error?
//originally from ImporterFile //originally from ImporterFile
public $import_errors; // public $import_errors; //
public ?Import $activeFile = null; public $activeFileId;
public $headerRow = [];
public $typeOfImport;
public $importTypes; public $importTypes;
public $columnOptions; public $columnOptions;
public $statusType; public $statusType;
@ -35,7 +30,6 @@ class Importer extends Component
public $send_welcome; public $send_welcome;
public $run_backup; 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 $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) // Make these variables public - we set the properties in the constructor so we can localize them (versus the old static arrays)
public $accessories_fields; public $accessories_fields;
@ -51,10 +45,8 @@ class Importer extends Component
'files.*.file_path' => 'required|string', 'files.*.file_path' => 'required|string',
'files.*.created_at' => 'required|string', 'files.*.created_at' => 'required|string',
'files.*.filesize' => 'required|integer', 'files.*.filesize' => 'required|integer',
'activeFile' => 'Import', 'headerRow' => 'array',
'activeFile.import_type' => 'string', 'typeOfImport' => 'string',
'activeFile.field_map' => 'array',
'activeFile.header_row' => 'array',
'field_map' => 'array' 'field_map' => 'array'
]; ];
@ -68,15 +60,13 @@ class Importer extends Component
{ {
$tmp = array(); $tmp = array();
if ($this->activeFile) { 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); $tmp = array_filter($tmp);
} }
return json_encode($tmp); return json_encode($tmp);
} }
private function getColumns($type) private function getColumns($type)
{ {
switch ($type) { switch ($type) {
@ -115,17 +105,15 @@ class Importer extends Component
return $results; 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. // go through each header, find a matching field to try and map it to.
foreach ($this->activeFile->header_row as $i => $header) { foreach ($this->headerRow as $i => $header) {
// do we have something mapped already? // do we have something mapped already?
if (array_key_exists($i, $this->field_map)) { if (array_key_exists($i, $this->field_map)) {
// yes, we do. Is it valid for this type of import? // yes, we do. Is it valid for this type of import?
// (e.g. the import type might have been changed...?) // (e.g. the import type might have been changed...?)
if (array_key_exists($this->field_map[$i], $this->columnOptions[$new_import_type])) { if (array_key_exists($this->field_map[$i], $this->columnOptions[$type])) {
//yes, this key *is* valid. Continue on to the next field. //yes, this key *is* valid. Continue on to the next field.
continue; continue;
} else { } else {
@ -135,9 +123,9 @@ class Importer extends Component
} // TODO - strictly speaking, this isn't necessary here I don't think. } // TODO - strictly speaking, this isn't necessary here I don't think.
} }
// first, check for exact matches // first, check for exact matches
foreach ($this->columnOptions[$new_import_type] as $value => $text) { foreach ($this->columnOptions[$type] as $v => $text) {
if (strcasecmp($text, $header) === 0) { // case-INSENSITIVe on purpose! if (strcasecmp($text, $header) === 0) { // case-INSENSITIVe on purpose!
$this->field_map[$i] = $value; $this->field_map[$i] = $v;
continue 2; //don't bother with the alias check, go to the next header continue 2; //don't bother with the alias check, go to the next header
} }
} }
@ -148,7 +136,7 @@ class Importer extends Component
// Make *absolutely* sure that this key actually _exists_ in this import type - // 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 // you can trigger this by importing accessories with a 'Warranty' column (which don't exist
// in "Accessories"!) // in "Accessories"!)
if (array_key_exists($key, $this->columnOptions[$new_import_type])) { if (array_key_exists($key, $this->columnOptions[$type])) {
$this->field_map[$i] = $key; $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 continue 3; // bust out of both of these loops; as well as the surrounding one - e.g. move on to the next header
} }
@ -159,18 +147,10 @@ class Importer extends Component
$this->field_map[$i] = null; // Booooo :( $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() public function mount()
{ {
$this->authorize('import'); $this->authorize('import');
$this->progress = -1; // '-1' means 'don't show the progressbar'
$this->progress_bar_class = 'progress-bar-warning';
$this->importTypes = [ $this->importTypes = [
'asset' => trans('general.assets'), 'asset' => trans('general.assets'),
'accessory' => trans('general.accessories'), 'accessory' => trans('general.accessories'),
@ -510,19 +490,16 @@ class Importer extends Component
]; ];
$this->columnOptions[''] = $this->getColumns(''); //blank mode? I don't know what this is supposed to mean $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); $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) public function selectFile($id)
{ {
$this->clearMessage(); $this->clearMessage();
$this->activeFile = Import::find($id); $this->activeFileId = $id;
if (!$this->activeFile) { if (!$this->activeFile) {
$this->message = trans('admin/hardware/message.import.file_missing'); $this->message = trans('admin/hardware/message.import.file_missing');
@ -531,15 +508,17 @@ class Importer extends Component
return; return;
} }
$this->headerRow = $this->activeFile->header_row;
$this->typeOfImport = $this->activeFile->import_type;
$this->field_map = null; $this->field_map = null;
foreach($this->activeFile->header_row as $element) { foreach ($this->headerRow as $element) {
if (isset($this->activeFile->field_map[$element])) { if (isset($this->activeFile->field_map[$element])) {
$this->field_map[] = $this->activeFile->field_map[$element]; $this->field_map[] = $this->activeFile->field_map[$element];
} else { } else {
$this->field_map[] = null; // re-inject the 'nulls' if a file was imported with some 'Do Not Import' settings $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->import_errors = null;
$this->statusText = null; $this->statusText = null;
@ -547,22 +526,34 @@ class Importer extends Component
public function destroy($id) public function destroy($id)
{ {
// TODO: why don't we just do File::find($id)? This seems dumb. $this->authorize('import');
foreach($this->files as $file) {
if ($id == $file->id) {
if (Storage::delete('private_uploads/imports/'.$file->file_path)) {
$file->delete();
$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 = trans('admin/hardware/message.import.file_delete_success');
$this->message_type = 'success'; $this->message_type = 'success';
unset($this->files);
return; return;
} else { }
$this->message = trans('admin/hardware/message.import.file_delete_error'); $this->message = trans('admin/hardware/message.import.file_delete_error');
$this->message_type = 'danger'; $this->message_type = 'danger';
} }
}
}
}
public function clearMessage() public function clearMessage()
{ {
@ -570,9 +561,20 @@ class Importer extends Component
$this->message_type = null; $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() public function render()
{ {
$this->files = Import::orderBy('id','desc')->get(); //HACK - slows down renders.
return view('livewire.importer') return view('livewire.importer')
->extends('layouts.default') ->extends('layouts.default')
->section('content'); ->section('content');

5
database/factories/UserFactory.php
View file

@ -296,6 +296,11 @@ class UserFactory extends Factory
return $this->appendPermission(['reports.view' => '1']); return $this->appendPermission(['reports.view' => '1']);
} }
public function canImport()
{
return $this->appendPermission(['import' => '1']);
}
private function appendPermission(array $permission) private function appendPermission(array $permission)
{ {
return $this->state(function ($currentState) use ($permission) { return $this->state(function ($currentState) use ($permission) {

1
resources/lang/en-US/admin/hardware/message.php
View file

@ -58,6 +58,7 @@ return [
'file_delete_success' => 'Your file has been been successfully deleted', 'file_delete_success' => 'Your file has been been successfully deleted',
'file_delete_error' => 'The file was unable to be deleted', 'file_delete_error' => 'The file was unable to be deleted',
'file_missing' => 'The file selected is missing', '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', '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', 'content_row_has_malformed_characters' => 'One or more attributes in the first row of content contain malformed UTF-8 characters',
], ],

48
resources/views/livewire/importer.blade.php
View file

@ -119,9 +119,9 @@
</th> </th>
</tr> </tr>
@foreach($files as $currentFile) @foreach($this->files as $currentFile)
<tr style="{{ ($activeFile && ($currentFile->id == $activeFile->id)) ? 'font-weight: bold' : '' }}" class="{{ ($activeFile && ($currentFile->id == $activeFile->id)) ? 'warning' : '' }}"> <tr style="{{ ($this->activeFile && ($currentFile->id == $this->activeFile->id)) ? 'font-weight: bold' : '' }}" class="{{ ($this->activeFile && ($currentFile->id == $this->activeFile->id)) ? 'warning' : '' }}">
<td class="col-md-6">{{ $currentFile->file_path }}</td> <td class="col-md-6">{{ $currentFile->file_path }}</td>
<td class="col-md-3">{{ Helper::getFormattedDateObject($currentFile->created_at, 'datetime', false) }}</td> <td class="col-md-3">{{ Helper::getFormattedDateObject($currentFile->created_at, 'datetime', false) }}</td>
<td class="col-md-1">{{ Helper::formatFilesizeUnits($currentFile->filesize) }}</td> <td class="col-md-1">{{ Helper::formatFilesizeUnits($currentFile->filesize) }}</td>
@ -130,25 +130,25 @@
<i class="fa-solid fa-list-check" aria-hidden="true"></i> <i class="fa-solid fa-list-check" aria-hidden="true"></i>
<span class="sr-only">{{ trans('general.import') }}</span> <span class="sr-only">{{ trans('general.import') }}</span>
</button> </button>
<a href="#" wire:click.prevent="$set('activeFile',null)"> <a href="#" wire:click.prevent="$set('activeFileId',null)">
<button class="btn btn-sm btn-danger" wire:click="destroy({{ $currentFile->id }})"> <button class="btn btn-sm btn-danger" wire:click="destroy({{ $currentFile->id }})">
<i class="fas fa-trash icon-white" aria-hidden="true"></i><span class="sr-only"></span></button> <i class="fas fa-trash icon-white" aria-hidden="true"></i><span class="sr-only"></span></button>
</a> </a>
</td> </td>
</tr> </tr>
@if( $currentFile && $activeFile && ($currentFile->id == $activeFile->id)) @if( $currentFile && $this->activeFile && ($currentFile->id == $this->activeFile->id))
<tr class="warning"> <tr class="warning">
<td colspan="4"> <td colspan="4">
<div class="form-group"> <div class="form-group">
<label for="activeFile.import_type" class="col-md-3 col-xs-12"> <label for="typeOfImport" class="col-md-3 col-xs-12">
{{ trans('general.import_type') }} {{ trans('general.import_type') }}
</label> </label>
<div class="col-md-9 col-xs-12" wire:ignore> <div class="col-md-9 col-xs-12" wire:ignore>
{{ Form::select('activeFile.import_type', $importTypes, $activeFile->import_type, [ {{ Form::select('typeOfImport', $importTypes, $typeOfImport, [
'id' => 'import_type', 'id' => 'import_type',
'class' => 'livewire-select2', 'class' => 'livewire-select2',
'style' => 'min-width: 350px', 'style' => 'min-width: 350px',
@ -157,7 +157,7 @@
'data-minimum-results-for-search' => '-1', // Remove this if the list gets long enough that we need to search 'data-minimum-results-for-search' => '-1', // Remove this if the list gets long enough that we need to search
'data-livewire-component' => $this->getId() 'data-livewire-component' => $this->getId()
]) }} ]) }}
@if ($activeFile->import_type === 'asset' && $snipeSettings->auto_increment_assets == 0) @if ($typeOfImport === 'asset' && $snipeSettings->auto_increment_assets == 0)
<p class="help-block"> <p class="help-block">
{{ trans('general.auto_incrementing_asset_tags_disabled_so_tags_required') }} {{ trans('general.auto_incrementing_asset_tags_disabled_so_tags_required') }}
</p> </p>
@ -170,7 +170,7 @@
<input type="checkbox" name="update" data-livewire-component="{{ $this->getId() }}" wire:model.live="update"> <input type="checkbox" name="update" data-livewire-component="{{ $this->getId() }}" wire:model.live="update">
{{ trans('general.update_existing_values') }} {{ trans('general.update_existing_values') }}
</label> </label>
@if ($activeFile->import_type === 'asset' && $snipeSettings->auto_increment_assets == 1 && $update) @if ($typeOfImport === 'asset' && $snipeSettings->auto_increment_assets == 1 && $update)
<p class="help-block"> <p class="help-block">
{{ trans('general.auto_incrementing_asset_tags_enabled_so_now_assets_will_be_created') }} {{ trans('general.auto_incrementing_asset_tags_enabled_so_now_assets_will_be_created') }}
</p> </p>
@ -196,13 +196,13 @@
@endif @endif
@if ($activeFile->import_type) @if ($typeOfImport)
<div class="form-group col-md-12"> <div class="form-group col-md-12">
<hr style="border-top: 1px solid lightgray"> <hr style="border-top: 1px solid lightgray">
<h3> <h3>
<i class="{{ Helper::iconTypeByItem($activeFile->import_type) }}"> <i class="{{ Helper::iconTypeByItem($typeOfImport) }}">
</i> </i>
{{ trans('general.map_fields', ['item_type' => ucwords($activeFile->import_type)]) }} {{ trans('general.map_fields', ['item_type' => ucwords($typeOfImport)]) }}
</h3> </h3>
<hr style="border-top: 1px solid lightgray"> <hr style="border-top: 1px solid lightgray">
</div> </div>
@ -218,16 +218,16 @@
</div> </div>
</div><!-- /div row --> </div><!-- /div row -->
@if($activeFile->header_row) @if(! empty($headerRow))
@foreach($activeFile->header_row as $index => $header) @foreach($headerRow as $index => $header)
<div class="form-group col-md-12" wire:key="header-row-{{ $index }}"> <div class="form-group col-md-12" wire:key="header-row-{{ $index }}">
<label for="field_map.{{ $index }}" class="col-md-3 control-label text-right">{{ $header }}</label> <label for="field_map.{{ $index }}" class="col-md-3 control-label text-right">{{ $header }}</label>
<div class="col-md-4" wire:ignore> <div class="col-md-4" wire:ignore>
{{ 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', 'class' => 'mappings livewire-select2',
'placeholder' => trans('general.importer.do_not_import'), 'placeholder' => trans('general.importer.do_not_import'),
@ -238,9 +238,9 @@
]) ])
}} }}
</div> </div>
@if (($activeFile->first_row) && (array_key_exists($index, $activeFile->first_row))) @if (($this->activeFile->first_row) && (array_key_exists($index, $this->activeFile->first_row)))
<div class="col-md-5"> <div class="col-md-5">
<p class="form-control-static">{{ str_limit($activeFile->first_row[$index], 50, '...') }}</p> <p class="form-control-static">{{ str_limit($this->activeFile->first_row[$index], 50, '...') }}</p>
</div> </div>
@else @else
@php @php
@ -256,7 +256,7 @@
<div class="form-group col-md-12"> <div class="form-group col-md-12">
<div class="col-md-3 text-left"> <div class="col-md-3 text-left">
<a href="#" wire:click.prevent="$set('activeFile',null)">{{ trans('general.cancel') }}</a> <a href="#" wire:click.prevent="$set('activeFileId',null)">{{ trans('general.cancel') }}</a>
</div> </div>
<div class="col-md-9"> <div class="col-md-9">
<button type="submit" class="btn btn-primary col-md-5" id="import">{{ trans('admin/hardware/message.import.import_button') }}</button> <button type="submit" class="btn btn-primary col-md-5" id="import">{{ trans('admin/hardware/message.import.import_button') }}</button>
@ -272,10 +272,10 @@
@else @else
<div class="form-group col-md-10"> <div class="form-group col-md-10">
<div class="col-md-3 text-left"> <div class="col-md-3 text-left">
<a href="#" wire:click.prevent="$set('activeFile',null)">{{ trans('general.cancel') }}</a> <a href="#" wire:click.prevent="$set('activeFileId',null)">{{ trans('general.cancel') }}</a>
</div> </div>
</div> </div>
@endif {{-- end of if ... activeFile->import_type --}} @endif {{-- end of if ... $typeOfImport --}}
</div><!-- /div v-show --> </td> </div><!-- /div v-show --> </td>
</tr> </tr>
@ -333,7 +333,7 @@
// we have to hook up to the `<tr id='importer-file'>` at the root of this display, // we have to hook up to the `<tr id='importer-file'>` at the root of this display,
// because the #import button isn't visible until you click an import_type // because the #import button isn't visible until you click an import_type
$('#upload-table').on('click', '#import', function () { $('#upload-table').on('click', '#import', function () {
if (!$wire.$get('activeFile.import_type')) { if (!$wire.$get('typeOfImport')) {
$wire.$set('statusType', 'error'); $wire.$set('statusType', 'error');
$wire.$set('statusText', "An import type is required... "); //TODO: translate? $wire.$set('statusText', "An import type is required... "); //TODO: translate?
return; return;
@ -345,15 +345,15 @@
// console.warn("Here is the mappings:") // console.warn("Here is the mappings:")
// console.dir(mappings) // console.dir(mappings)
// console.warn("Uh, active file id is, I guess: "+$wire.$get('activeFile.id')) // 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({ $.post({
{{-- I want to do something like: route('api.imports.importFile', $activeFile->id) }} --}} {{-- 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', contentType: 'application/json',
data: JSON.stringify({ data: JSON.stringify({
'import-update': !!$wire.$get('update'), 'import-update': !!$wire.$get('update'),
'send-welcome': !!$wire.$get('send_welcome'), 'send-welcome': !!$wire.$get('send_welcome'),
'import-type': $wire.$get('activeFile.import_type'), 'import-type': $wire.$get('typeOfImport'),
'run-backup': !!$wire.$get('run_backup'), 'run-backup': !!$wire.$get('run_backup'),
'column-mappings': mappings 'column-mappings': mappings
}), }),
@ -393,7 +393,7 @@
} }
} }
$wire.$set('activeFile', null); //$wire.$set('hideDetails') $wire.$set('activeFileId', null); //$wire.$set('hideDetails')
}); });
}) })
return false; return false;

25
tests/Feature/Livewire/ImporterTest.php Normal file
View file

@ -0,0 +1,25 @@
<?php
namespace Tests\Feature\Livewire;
use App\Livewire\Importer;
use App\Models\User;
use Livewire\Livewire;
use Tests\TestCase;
class ImporterTest extends TestCase
{
public function testRendersSuccessfully()
{
Livewire::actingAs(User::factory()->canImport()->create())
->test(Importer::class)
->assertStatus(200);
}
public function testRequiresPermission()
{
Livewire::actingAs(User::factory()->create())
->test(Importer::class)
->assertStatus(403);
}
}