mirror of
https://github.com/snipe/snipe-it.git
synced 2024-11-14 09:34:10 -08:00
Merge pull request #15112 from marcusmoore/livewire-importer-improvements
Improved handling attempted access of deleted files in importer
This commit is contained in:
snipe
GitHub
commit
f8c72fb0ac
|
|
@ -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,17 +105,15 @@ 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) {
|
||||
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[$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.
|
||||
continue;
|
||||
} else {
|
||||
|
|
@ -135,9 +123,9 @@ class Importer extends Component
|
|||
} // 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) {
|
||||
foreach ($this->columnOptions[$type] as $v => $text) {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
|
@ -148,7 +136,7 @@ class Importer extends Component
|
|||
// 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])) {
|
||||
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
|
||||
}
|
||||
|
|
@ -159,18 +147,10 @@ class Importer extends Component
|
|||
$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'),
|
||||
|
|
@ -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,22 +526,34 @@ 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');
|
||||
|
||||
$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;
|
||||
} else {
|
||||
}
|
||||
|
||||
$this->message = trans('admin/hardware/message.import.file_delete_error');
|
||||
$this->message_type = 'danger';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function clearMessage()
|
||||
{
|
||||
|
|
@ -570,9 +561,20 @@ 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');
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
],
|
||||
|
|
|
|||
|
|
@ -119,9 +119,9 @@
|
|||
</th>
|
||||
</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-3">{{ Helper::getFormattedDateObject($currentFile->created_at, 'datetime', false) }}</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>
|
||||
<span class="sr-only">{{ trans('general.import') }}</span>
|
||||
</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 }})">
|
||||
<i class="fas fa-trash icon-white" aria-hidden="true"></i><span class="sr-only"></span></button>
|
||||
</a>
|
||||
</td>
|
||||
</tr>
|
||||
|
||||
@if( $currentFile && $activeFile && ($currentFile->id == $activeFile->id))
|
||||
@if( $currentFile && $this->activeFile && ($currentFile->id == $this->activeFile->id))
|
||||
<tr class="warning">
|
||||
<td colspan="4">
|
||||
|
||||
<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') }}
|
||||
</label>
|
||||
|
||||
<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',
|
||||
'class' => 'livewire-select2',
|
||||
'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-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">
|
||||
{{ trans('general.auto_incrementing_asset_tags_disabled_so_tags_required') }}
|
||||
</p>
|
||||
|
|
@ -170,7 +170,7 @@
|
|||
<input type="checkbox" name="update" data-livewire-component="{{ $this->getId() }}" wire:model.live="update">
|
||||
{{ trans('general.update_existing_values') }}
|
||||
</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">
|
||||
{{ trans('general.auto_incrementing_asset_tags_enabled_so_now_assets_will_be_created') }}
|
||||
</p>
|
||||
|
|
@ -196,13 +196,13 @@
|
|||
@endif
|
||||
|
||||
|
||||
@if ($activeFile->import_type)
|
||||
@if ($typeOfImport)
|
||||
<div class="form-group col-md-12">
|
||||
<hr style="border-top: 1px solid lightgray">
|
||||
<h3>
|
||||
<i class="{{ Helper::iconTypeByItem($activeFile->import_type) }}">
|
||||
<i class="{{ Helper::iconTypeByItem($typeOfImport) }}">
|
||||
</i>
|
||||
{{ trans('general.map_fields', ['item_type' => ucwords($activeFile->import_type)]) }}
|
||||
{{ trans('general.map_fields', ['item_type' => ucwords($typeOfImport)]) }}
|
||||
</h3>
|
||||
<hr style="border-top: 1px solid lightgray">
|
||||
</div>
|
||||
|
|
@ -218,16 +218,16 @@
|
|||
</div>
|
||||
</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 }}">
|
||||
|
||||
<label for="field_map.{{ $index }}" class="col-md-3 control-label text-right">{{ $header }}</label>
|
||||
<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',
|
||||
'placeholder' => trans('general.importer.do_not_import'),
|
||||
|
|
@ -238,9 +238,9 @@
|
|||
])
|
||||
}}
|
||||
</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">
|
||||
<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>
|
||||
@else
|
||||
@php
|
||||
|
|
@ -256,7 +256,7 @@
|
|||
|
||||
<div class="form-group col-md-12">
|
||||
<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 class="col-md-9">
|
||||
<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
|
||||
<div class="form-group col-md-10">
|
||||
<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>
|
||||
@endif {{-- end of if ... activeFile->import_type --}}
|
||||
@endif {{-- end of if ... $typeOfImport --}}
|
||||
|
||||
</div><!-- /div v-show --> </td>
|
||||
</tr>
|
||||
|
|
@ -333,7 +333,7 @@
|
|||
// 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
|
||||
$('#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;
|
||||
|
|
|
|||
25
tests/Feature/Livewire/ImporterTest.php
Normal file
25
tests/Feature/Livewire/ImporterTest.php
Normal 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);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue