From 56a6c749f6ab989d25c74eca5bcdddc58bea2d10 Mon Sep 17 00:00:00 2001 From: bryanlopezinc Date: Thu, 28 Nov 2024 10:33:32 +0100 Subject: [PATCH] imporove import items command --- app/Console/Commands/ObjectImportCommand.php | 104 +++++++++-- app/Http/Requests/ItemImportRequest.php | 10 +- app/Importer/Factory.php | 24 +++ app/Importer/Importer.php | 2 +- app/Importer/MimeTypes.php | 19 ++ app/Importer/Type.php | 31 ++++ .../Console/ImportItemsCommandTest.php | 162 ++++++++++++++++++ .../Importing/Api/GeneralImportTest.php | 18 ++ .../Importing/Api/ImportAssetsTest.php | 5 - 9 files changed, 347 insertions(+), 28 deletions(-) create mode 100644 app/Importer/Factory.php create mode 100644 app/Importer/MimeTypes.php create mode 100644 app/Importer/Type.php create mode 100644 tests/Feature/Console/ImportItemsCommandTest.php diff --git a/app/Console/Commands/ObjectImportCommand.php b/app/Console/Commands/ObjectImportCommand.php index a1202ded8959..817296157686 100644 --- a/app/Console/Commands/ObjectImportCommand.php +++ b/app/Console/Commands/ObjectImportCommand.php @@ -2,10 +2,17 @@ namespace App\Console\Commands; +use App\Importer\Factory; +use App\Importer\MimeTypes; +use App\Importer\Type; +use App\Models\User; use Illuminate\Console\Command; +use Illuminate\Http\File; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Validator; +use Illuminate\Validation\Rule; use Symfony\Component\Console\Helper\ProgressIndicator; ini_set('max_execution_time', env('IMPORT_TIME_LIMIT', 600)); //600 seconds = 10 minutes @@ -52,33 +59,84 @@ public function __construct() */ public function handle() { + if (!$this->validate()) { + return self::FAILURE; + } + $this->progressIndicator = new ProgressIndicator($this->output); $filename = $this->argument('filename'); - $class = title_case($this->option('item-type')); - $classString = "App\\Importer\\{$class}Importer"; - $importer = new $classString($filename); + $importer = Factory::make($filename, $this->option('item-type')); $importer->setCallbacks([$this, 'log'], [$this, 'progress'], [$this, 'errorCallback']) - ->setUserId($this->option('user_id')) - ->setUpdating($this->option('update')) - ->setShouldNotify($this->option('send-welcome')) - ->setUsernameFormat($this->option('username_format')); + ->setUserId((int)$this->option('user_id')) + ->setUpdating($this->option('update')) + ->setShouldNotify($this->option('send-welcome')) + ->setUsernameFormat($this->option('username_format')); // This $logFile/useFiles() bit is currently broken, so commenting it out for now // $logFile = $this->option('logfile'); // Log::useFiles($logFile); - $this->progressIndicator->start('======= Importing Items from '.$filename.' ========='); + $this->progressIndicator->start('======= Importing Items from ' . $filename . ' ========='); $importer->import(); $this->progressIndicator->finish('Import finished.'); } + protected function validate(): bool + { + $filepath = $this->argument('filename'); + $importFile = new File(realpath($filepath), false); + $validator = Validator::make( + array_merge($this->options(), ['file' => $importFile]), + $this->rules(), + $this->messages() + )->stopOnFirstFailure(); + + if (!$importFile->isFile()) { + $this->error("file \"{$filepath}\" not found."); + + return false; + } + + foreach ($validator->errors()->all() as $errors) { + $this->error($errors); + + return false; + } + + return true; + } + + /** + * Get the validation rules for this command. + */ + protected function rules(): array + { + return [ + 'file' => ['file', Rule::file()->types(MimeTypes::VALID)], + 'item-type' => ['sometimes', Rule::in(Type::validTypesForCli())], + 'user_id' => ['sometimes', 'int', 'min:1', Rule::exists(User::class, 'id')->withoutTrashed()], + 'username_format' => ['nullable', 'in:firstname.lastname,firstname,filastname,email'], + 'email_format' => ['nullable', 'in:firstname.lastname,firstname,filastname'], + ]; + } + + /** + * Get custom messages for validator errors. + */ + protected function messages(): array + { + return [ + 'file.mimetypes' => 'The given file type is not supported.' + ]; + } + public function errorCallback($item, $field, $error) { $this->output->write("\x0D\x1B[2K"); - $this->warn('Error: Item: '.$item->name.' failed validation: '.json_encode($error)); + $this->warn('Error: Item: ' . $item->name . ' failed validation: ' . json_encode($error)); } public function progress($importedItemsCount) @@ -132,14 +190,26 @@ protected function getArguments() protected function getOptions() { return [ - ['email_format', null, InputOption::VALUE_REQUIRED, 'The format of the email addresses that should be generated. Options are firstname.lastname, firstname, filastname', null], - ['username_format', null, InputOption::VALUE_REQUIRED, 'The format of the username that should be generated. Options are firstname.lastname, firstname, filastname, email', null], - ['logfile', null, InputOption::VALUE_REQUIRED, 'The path to log output to. storage/logs/importer.log by default', storage_path('logs/importer.log')], - ['item-type', null, InputOption::VALUE_REQUIRED, 'Item Type To import. Valid Options are Asset, Consumable, Accessory, License, or User', 'Asset'], - ['web-importer', null, InputOption::VALUE_NONE, 'Internal: packages output for use with the web importer'], - ['user_id', null, InputOption::VALUE_REQUIRED, 'ID of user creating items', 1], - ['update', null, InputOption::VALUE_NONE, 'If a matching item is found, update item information'], - ['send-welcome', null, InputOption::VALUE_NONE, 'Whether to send a welcome email to any new users that are created.'], + ['email_format', null, InputOption::VALUE_REQUIRED, 'The format of the email addresses that should be generated. Options are firstname.lastname, firstname, filastname', null], + ['username_format', null, InputOption::VALUE_REQUIRED, 'The format of the username that should be generated. Options are firstname.lastname, firstname, filastname, email', null], + ['logfile', null, InputOption::VALUE_REQUIRED, 'The path to log output to. storage/logs/importer.log by default', storage_path('logs/importer.log')], + ['item-type', null, InputOption::VALUE_REQUIRED, "Item Type To import. Valid Options are {$this->getValidImportTypesText()}", 'Asset'], + ['web-importer', null, InputOption::VALUE_NONE, 'Internal: packages output for use with the web importer'], + ['user_id', null, InputOption::VALUE_REQUIRED, 'ID of user creating items', 1], + ['update', null, InputOption::VALUE_NONE, 'If a matching item is found, update item information'], + ['send-welcome', null, InputOption::VALUE_NONE, 'Whether to send a welcome email to any new users that are created.'], ]; } + + private function getValidImportTypesText(): string + { + $importTypes = collect(Type::cases()) + ->pluck('value') + ->map('ucfirst') + ->map(fn (string $type) => "{$type}"); + + $last = $importTypes->pop(); + + return implode(' or ', [$importTypes->implode(', '), $last]); + } } diff --git a/app/Http/Requests/ItemImportRequest.php b/app/Http/Requests/ItemImportRequest.php index a6dc0ad7e5e2..d2985a4350d3 100644 --- a/app/Http/Requests/ItemImportRequest.php +++ b/app/Http/Requests/ItemImportRequest.php @@ -2,10 +2,12 @@ namespace App\Http\Requests; +use App\Importer\Factory; +use App\Importer\Type; use App\Models\Import; use Illuminate\Foundation\Http\FormRequest; -use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; +use Illuminate\Validation\Rule; class ItemImportRequest extends FormRequest { @@ -27,7 +29,7 @@ public function authorize() public function rules() { return [ - 'import-type' => 'required', + 'import-type' => ['required', Rule::in(Type::cases())], ]; } @@ -38,9 +40,7 @@ public function import(Import $import) $filename = config('app.private_uploads').'/imports/'.$import->file_path; $import->import_type = $this->input('import-type'); - $class = title_case($import->import_type); - $classString = "App\\Importer\\{$class}Importer"; - $importer = new $classString($filename); + $importer = Factory::make($filename, $this->input('import-type')); $import->field_map = request('column-mappings'); $import->save(); $fieldMappings = []; diff --git a/app/Importer/Factory.php b/app/Importer/Factory.php new file mode 100644 index 000000000000..0beeac6e7f64 --- /dev/null +++ b/app/Importer/Factory.php @@ -0,0 +1,24 @@ + new AccessoryImporter($filepath), + Type::ASSET => new AssetImporter($filepath), + Type::COMPONENT => new ComponentImporter($filepath), + Type::CONSUMABLE => new ConsumableImporter($filepath), + Type::LICENSE => new LicenseImporter($filepath), + Type::LOCATION => new LocationImporter($filepath), + Type::USER => new UserImporter($filepath), + }; + } +} diff --git a/app/Importer/Importer.php b/app/Importer/Importer.php index 6f2816c7af07..feb2211d6054 100644 --- a/app/Importer/Importer.php +++ b/app/Importer/Importer.php @@ -146,7 +146,7 @@ public function __construct($file) */ public function import() { - $headerRow = $this->csv->fetchOne(); + $headerRow = $this->csv->nth(0); $this->csv->setHeaderOffset(0); //explicitly sets the CSV document header record $this->populateCustomFields($headerRow); diff --git a/app/Importer/MimeTypes.php b/app/Importer/MimeTypes.php new file mode 100644 index 000000000000..da827a12a502 --- /dev/null +++ b/app/Importer/MimeTypes.php @@ -0,0 +1,19 @@ + + */ + public static function validTypesForCli(): array + { + $types = array_column(self::cases(), 'value'); + + return collect($types) + ->map(ucfirst(...)) + ->merge($types) + ->all(); + } +} diff --git a/tests/Feature/Console/ImportItemsCommandTest.php b/tests/Feature/Console/ImportItemsCommandTest.php new file mode 100644 index 000000000000..b7d91f70aaa4 --- /dev/null +++ b/tests/Feature/Console/ImportItemsCommandTest.php @@ -0,0 +1,162 @@ +find(1) === null) { + User::factory()->createOne(['id' => 1]); + } + } + + #[Test] + public function importItems(): void + { + $accessoriesFile = AccessoriesImportFileBuilder::new(); + $assetsFile = AssetsImportFileBuilder::new(); + $componentsFile = ComponentsImportFileBuilder::new(); + $consumablesFile = ConsumablesImportFileBuilder::new(); + $licensesFile = LicensesImportFileBuilder::new(); + $usersFile = UsersImportFileBuilder::new(); + + $this->runCommand(['filename' => $this->getPath($assetsFile->saveToImportsDirectory())])->assertOk(); + $this->runCommand(['filename' => $this->getPath($accessoriesFile->saveToImportsDirectory()), '--item-type' => 'Accessory'])->assertOk(); + $this->runCommand(['filename' => $this->getPath($componentsFile->saveToImportsDirectory()), '--item-type' => 'component'])->assertOk(); + $this->runCommand(['filename' => $this->getPath($consumablesFile->saveToImportsDirectory()), '--item-type' => 'Consumable'])->assertOk(); + $this->runCommand(['filename' => $this->getPath($licensesFile->saveToImportsDirectory()), '--item-type' => 'License'])->assertOk(); + $this->runCommand(['filename' => $this->getPath($usersFile->saveToImportsDirectory()), '--item-type' => 'user'])->assertOk(); + + $this->assertTrue(Accessory::query()->where('name', $accessoriesFile->firstRow()['itemName'])->exists()); + $this->assertTrue(Asset::query()->where('serial', $assetsFile->firstRow()['serialNumber'])->exists()); + $this->assertTrue(Component::query()->where('name', $componentsFile->firstRow()['itemName'])->exists()); + $this->assertTrue(Consumable::query()->where('name', $consumablesFile->firstRow()['itemName'])->exists()); + $this->assertTrue(License::query()->where('serial', $licensesFile->firstRow()['serialNumber'])->exists()); + $this->assertTrue(User::query()->where('username', $usersFile->firstRow()['username'])->exists()); + } + + protected function runCommand(array $parameters = []): PendingCommand + { + return $this->artisan('snipeit:import', $parameters); + } + + #[Test] + public function willReturnFailedWhenFilenameIsNotProvided(): void + { + $this->expectExceptionMessage('Not enough arguments (missing: "filename").'); + + $this->runCommand(); + } + + #[Test] + public function willReturnFailedWhenImportFileDoesNotExits(): void + { + $this->runCommand(['filename' => 'foo.csv']) + ->expectsOutput('file "foo.csv" not found.') + ->assertFailed(); + + $this->runCommand(['filename' => $dir = __DIR__]) + ->expectsOutput("file \"{$dir}\" not found.") + ->assertFailed(); + } + + #[Test] + public function willReturnFailedWhenImportTypeIsInvalid(): void + { + $filename = AssetsImportFileBuilder::times()->saveToImportsDirectory(); + + $this->runCommand(['--item-type' => 'foo', 'filename' => $this->getPath($filename)]) + ->expectsOutput('The selected item-type is invalid.') + ->assertFailed(); + } + + private function getPath(string $filename): string + { + return config('app.private_uploads') . "/imports/{$filename}"; + } + + #[Test] + public function willReturnFailedWhenUserIdIsInvalid(): void + { + $filename = AssetsImportFileBuilder::times()->saveToImportsDirectory(); + + $this->runCommand(['--user_id' => 'foo', 'filename' => $this->getPath($filename)]) + ->expectsOutput('The user id field must be an integer.') + ->assertFailed(); + + $this->runCommand(['--user_id' => '-1', 'filename' => $this->getPath($filename)]) + ->expectsOutput('The user id field must be at least 1.') + ->assertFailed(); + } + + #[Test] + public function willReturnFailedWhenUserDoesNotExists(): void + { + $expectedOutput = 'The selected user id is invalid.'; + $filename = AssetsImportFileBuilder::times()->saveToImportsDirectory(); + [$softDeletedUser, $deletedUser] = User::factory()->createMany(2); + + $softDeletedUser->delete(); + $deletedUser->forceDelete(); + + $this->runCommand(['--user_id' => "{$softDeletedUser->id}", 'filename' => $this->getPath($filename)]) + ->expectsOutput($expectedOutput) + ->assertFailed(); + + $this->runCommand(['--user_id' => "{$deletedUser->id}", 'filename' => $this->getPath($filename)]) + ->expectsOutput($expectedOutput) + ->assertFailed(); + } + + #[Test] + public function willReturnFailedWhenUsernameFormatIsInvalid(): void + { + $filename = AssetsImportFileBuilder::times()->saveToImportsDirectory(); + + $this->runCommand(['--username_format' => 'foo', 'filename' => $this->getPath($filename)]) + ->expectsOutput('The selected username format is invalid.') + ->assertFailed(); + } + + #[Test] + public function willReturnFailedWhenEmailFormatIsInvalid(): void + { + $filename = AssetsImportFileBuilder::times()->saveToImportsDirectory(); + + $this->runCommand(['--email_format' => 'foo', 'filename' => $this->getPath($filename)]) + ->expectsOutput('The selected email format is invalid.') + ->assertFailed(); + } + + #[Test] + public function willReturnFailedWhenFileTypeIsNotSupported(): void + { + $this->runCommand(['filename' => __FILE__])->expectsOutput('The given file type is not supported.'); + } +} diff --git a/tests/Feature/Importing/Api/GeneralImportTest.php b/tests/Feature/Importing/Api/GeneralImportTest.php index 5c38dab7f3e6..02e3c4ccfd54 100644 --- a/tests/Feature/Importing/Api/GeneralImportTest.php +++ b/tests/Feature/Importing/Api/GeneralImportTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Importing\Api; +use App\Models\Import; use App\Models\User; class GeneralImportTest extends ImportDataTestCase @@ -13,4 +14,21 @@ public function testRequiresExistingImport() $this->importFileResponse(['import' => 9999, 'import-type' => 'accessory']) ->assertStatusMessageIs('import-errors'); } + + public function testWillReturnValidationErrorWhenImportTypeIsInvalid() + { + $this->actingAsForApi(User::factory()->canImport()->create()); + + $import = Import::factory()->accessory()->create(); + + $this->importFileResponse(['import-type' => 'foo', 'import' => $import->id]) + ->assertOk() + ->assertJson([ + 'messages' => [ + 'import-type' => [ + 'The selected import-type is invalid.' + ] + ] + ]); + } } diff --git a/tests/Feature/Importing/Api/ImportAssetsTest.php b/tests/Feature/Importing/Api/ImportAssetsTest.php index 0f54b22e92d7..32c4c780810b 100644 --- a/tests/Feature/Importing/Api/ImportAssetsTest.php +++ b/tests/Feature/Importing/Api/ImportAssetsTest.php @@ -2,18 +2,14 @@ namespace Tests\Feature\Importing\Api; -use App\Mail\CheckoutAssetMail; use App\Models\Actionlog as ActionLog; use App\Models\Asset; use App\Models\CustomField; use App\Models\Import; use App\Models\User; -use App\Notifications\CheckoutAssetNotification; use Carbon\Carbon; use Illuminate\Foundation\Testing\WithFaker; use Illuminate\Support\Arr; -use Illuminate\Support\Facades\Mail; -use Illuminate\Support\Facades\Notification; use Illuminate\Support\Str; use Illuminate\Testing\TestResponse; use PHPUnit\Framework\Attributes\Test; @@ -56,7 +52,6 @@ public function userWithImportAssetsPermissionCanImportAssets(): void #[Test] public function importAsset(): void { - $importFileBuilder = ImportFileBuilder::new(); $row = $importFileBuilder->firstRow(); $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]);