diff --git a/app/Console/Commands/Purge.php b/app/Console/Commands/Purge.php index 99031d6f55..2b6f4fabda 100644 --- a/app/Console/Commands/Purge.php +++ b/app/Console/Commands/Purge.php @@ -2,13 +2,29 @@ namespace App\Console\Commands; +use App\Enums\ActionType; +use App\Models\Accessory; use App\Models\Asset; +use App\Models\AssetModel; +use App\Models\Category; +use App\Models\CheckoutAcceptance; +use App\Models\Company; +use App\Models\Component; +use App\Models\Consumable; +use App\Models\Department; use App\Models\License; +use App\Models\Location; +use App\Models\Maintenance; +use App\Models\Manufacturer; +use App\Models\Supplier; use App\Models\User; use Illuminate\Console\Command; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Storage; use ReflectionClass; use function Laravel\Prompts\confirm; @@ -27,34 +43,7 @@ class Purge extends Command {--force=false : Skip the confirmation prompt (accepts "true").} {--dry-run : Report what would be purged without deleting anything.}'; - protected $description = 'Purge all soft-deleted records in the database. Walks every model that uses the SoftDeletes trait, DELETEs the trashed rows, and cleans up their polymorphic action_log children. No undo.'; - - /** - * Users are excluded even when soft-deleted if show_in_list='0'. System - * users (LDAP-sync placeholders, etc.) set this to '0' and shouldn't be - * garbage-collected here. - */ - private const PARENT_QUERY_FILTERS = [ - User::class => [['column' => 'show_in_list', 'op' => '!=', 'value' => '0']], - ]; - - /** - * Non-soft-deletable FK children that should be nuked when their parent - * is purged, even if the child itself was not soft-deleted. Auto- - * discovery finds all soft-deletable models, but a trashed License with - * live LicenseSeats or a trashed Asset with live Maintenances would - * leave orphans behind if we only nuked soft-deleted rows. This map - * covers the parent→child relations where the parent "owns" the child - * outright and orphaning it makes no sense. - */ - private const FK_CHILDREN = [ - Asset::class => [ - ['table' => 'maintenances', 'foreign_key' => 'asset_id'], - ], - License::class => [ - ['table' => 'license_seats', 'foreign_key' => 'license_id'], - ], - ]; + protected $description = 'Purge all soft-deleted records in the database. Walks every model that uses the SoftDeletes trait, DELETEs the trashed rows, cleans up their polymorphic action_log children, and removes their uploaded files and image assets from disk. No undo.'; public function handle(): int { @@ -112,7 +101,7 @@ class Purge extends Command * * Dedupe by table so single-table inheritance / subclassed models * (e.g. SCIMUser extends User, same `users` table) don't get processed - * twice — the first pass would run without the subclass-specific + * twice. The first pass would run without the subclass-specific * filters (`show_in_list != 0` for users) and delete the rows the * parent's filter was supposed to preserve. Prefer the base class: * the more-derived class is skipped if a parent for its table was @@ -158,15 +147,10 @@ class Purge extends Command } /** - * Nuke one model's trashed rows: pluck the trashed ids, wipe their - * polymorphic action_log children (via `item_type`/`item_id` and - * `target_type`/`target_id`), then bulk-delete the parents by their - * `deleted_at` index. - * - * Children go per-row DELETE against the composite index on - * action_logs — bulk WHERE IN and JOIN DELETE both benchmarked ~3x - * slower on MariaDB (InnoDB commits small autocommit transactions - * faster than one long one against the composite index). + * Nuke one model's trashed rows: pluck the trashed ids, delete + * on-disk files (images and uploaded files) associated with those + * rows, wipe polymorphic action_log children, wipe FK child tables, + * then bulk-delete the parents by their `deleted_at` index. * * @return array */ @@ -177,8 +161,13 @@ class Purge extends Command $label = class_basename($modelClass); $parentQuery = DB::table($table)->whereNotNull('deleted_at'); - foreach (self::PARENT_QUERY_FILTERS[$modelClass] ?? [] as $f) { - $parentQuery->where($f['column'], $f['op'], $f['value']); + + // show_in_list=0 excludes a user from checkout-target dropdowns + // in the UI. Preserved by the purge (matches the pre-refactor + // behavior) so users with this flag stick around even when + // soft-deleted. + if ($modelClass === User::class) { + $parentQuery->where('show_in_list', '!=', '0'); } $ids = (clone $parentQuery)->pluck('id'); @@ -186,7 +175,15 @@ class Purge extends Command return []; } - $rows = []; + // File cleanup runs before the DB deletes so we can still read + // the image/avatar column off the parent row and correlate + // action_logs to a still-existing parent. Skipped during dry-run + // so `--dry-run` truly writes nothing. + if (! $dryRun) { + $this->deleteImageFiles($modelClass, $table, $ids); + $this->deleteActionLogFiles($modelClass, $ids); + $this->deletePrivateFileColumns($modelClass, $table, $ids); + } // Polymorphic action_log cleanup. Every model referenced by // action_logs uses one of two column pairs. Users use target_*, @@ -194,46 +191,49 @@ class Purge extends Command $itemLogs = 0; $targetLogs = 0; foreach ($ids as $id) { + $itemQuery = DB::table('action_logs') + ->where('item_type', $modelClass) + ->where('item_id', $id); + $targetQuery = DB::table('action_logs') + ->where('target_type', $modelClass) + ->where('target_id', $id); if ($dryRun) { - $itemLogs += DB::table('action_logs') - ->where('item_type', $modelClass) - ->where('item_id', $id) - ->count(); - $targetLogs += DB::table('action_logs') - ->where('target_type', $modelClass) - ->where('target_id', $id) - ->count(); + $itemLogs += $itemQuery->count(); + $targetLogs += $targetQuery->count(); } else { - $itemLogs += DB::table('action_logs') - ->where('item_type', $modelClass) - ->where('item_id', $id) - ->delete(); - $targetLogs += DB::table('action_logs') - ->where('target_type', $modelClass) - ->where('target_id', $id) - ->delete(); + $itemLogs += $itemQuery->delete(); + $targetLogs += $targetQuery->delete(); } } - // FK-child cleanup: rows in other tables that belong to a trashed - // parent by a plain foreign key (see FK_CHILDREN docblock). These - // are nuked whole rather than only-trashed because a live - // LicenseSeat pointing at a purged License is an orphan by - // definition. - $fkChildCounts = []; - foreach (self::FK_CHILDREN[$modelClass] ?? [] as $child) { - $count = 0; - foreach ($ids as $id) { - $q = DB::table($child['table'])->where($child['foreign_key'], $id); - $count += $dryRun ? $q->count() : $q->delete(); - } - if ($count > 0) { - $fkChildCounts[$child['table']] = $count; + // Child-table cleanup: rows in other tables that belong to a + // trashed parent by a plain foreign key. Nuked whole rather than + // only-trashed because a live LicenseSeat pointing at a purged + // License is an orphan by definition. Auto-discovery finds all + // soft-deletable models, but a trashed License with live + // LicenseSeats or a trashed Asset with live Maintenances would + // leave orphans behind if we only nuked soft-deleted rows. + $childTables = [ + Asset::class => ['maintenances' => 'asset_id'], + License::class => ['license_seats' => 'license_id'], + ]; + $childCounts = []; + if (array_key_exists($modelClass, $childTables)) { + foreach ($childTables[$modelClass] as $childTable => $foreignKey) { + $count = 0; + foreach ($ids as $id) { + $q = DB::table($childTable)->where($foreignKey, $id); + $count += $dryRun ? $q->count() : $q->delete(); + } + if ($count > 0) { + $childCounts[$childTable] = $count; + } } } $parentCount = $dryRun ? $ids->count() : $parentQuery->delete(); + $rows = []; $rows[] = [$label, $parentCount]; if ($itemLogs > 0) { $rows[] = [$label.' action_logs (item)', $itemLogs]; @@ -241,10 +241,195 @@ class Purge extends Command if ($targetLogs > 0) { $rows[] = [$label.' action_logs (target)', $targetLogs]; } - foreach ($fkChildCounts as $table => $count) { - $rows[] = [$table, $count]; + foreach ($childCounts as $childTable => $count) { + $rows[] = [$childTable, $count]; } return $rows; } + + /** + * Delete image/avatar files stored on the public disk for the + * trashed rows. Reads the filename off each trashed parent row, + * then unlinks `{subpath}/{filename}` from the public disk. + * + * Only lives here in the purge (not in each controller's destroy + * method) so that soft-deleting a row does NOT delete the image. + * That way a soft-deleted row can be restored with its image intact. + * The image is only permanently removed when the row is permanently + * removed (via this purge). + */ + private function deleteImageFiles(string $modelClass, string $table, Collection $ids): void + { + // Image/avatar files stored on the public disk, keyed by parent + // model. Value is `column_name => public-disk subpath`. Purge + // reads the filename off each trashed parent row and unlinks + // `{subpath}/{column_value}` from the public disk. + $imageFiles = [ + User::class => ['avatar' => 'avatars'], + Asset::class => ['image' => 'assets'], + AssetModel::class => ['image' => 'models'], + Accessory::class => ['image' => 'accessories'], + Category::class => ['image' => 'categories'], + Company::class => ['image' => 'companies'], + Component::class => ['image' => 'components'], + Consumable::class => ['image' => 'consumables'], + Department::class => ['image' => 'departments'], + Location::class => ['image' => 'locations'], + Manufacturer::class => ['image' => 'manufacturers'], + Supplier::class => ['image' => 'suppliers'], + ]; + + if (! array_key_exists($modelClass, $imageFiles)) { + return; + } + + foreach ($imageFiles[$modelClass] as $column => $subpath) { + $filenames = DB::table($table) + ->whereIn('id', $ids) + ->pluck($column) + ->filter() + ->unique(); + + foreach ($filenames as $filename) { + try { + $key = trim($subpath, '/').'/'.basename($filename); + if (Storage::disk('public')->exists($key)) { + Storage::disk('public')->delete($key); + } + } catch (\Exception $e) { + Log::info(sprintf( + 'snipeit:purge - error deleting %s file %s for %s: %s', + $column, $filename, $modelClass, $e->getMessage() + )); + } + } + } + } + + /** + * Delete every file referenced by action_logs whose parent row is + * about to be purged. Covers four categories of file, keyed by + * `action_type` on the log: + * + * - `uploaded` → `private_uploads/{type}/` (Files tab attachments) + * - `audit` → `private_uploads/audits/` + * - `accepted` → `private_uploads/eula-pdfs/` + * - `declined` → `private_uploads/eula-pdfs/` + * + * Plus, independent of action_type, the `accept_signature` column + * can point at a signature file under `private_uploads/signatures/`. + * + * Match rows via BOTH the item_* and target_* column pairs. When + * purging a user, we want signatures/EULAs stored under target_id + * (the accepting user) even though the checkoutable item's + * item_type points at Asset/License/etc. + * + * Failure to unlink is logged but not fatal. + */ + private function deleteActionLogFiles(string $modelClass, Collection $ids): void + { + // "Files" tab attachment roots under private_uploads/, keyed by + // the parent model of the file. These are the contracts, receipts, + // photos, etc. tracked in action_logs with action_type = 'uploaded'. + // Not done at soft-delete time so restoring a soft-deleted row + // brings the files back with it. + $uploadRoots = [ + Accessory::class => 'private_uploads/accessories', + Asset::class => 'private_uploads/assets', + AssetModel::class => 'private_uploads/models', + Company::class => 'private_uploads/companies', + Component::class => 'private_uploads/components', + Consumable::class => 'private_uploads/consumables', + Department::class => 'private_uploads/departments', + License::class => 'private_uploads/licenses', + Location::class => 'private_uploads/locations', + Maintenance::class => 'private_uploads/maintenances', + Supplier::class => 'private_uploads/suppliers', + User::class => 'private_uploads/users', + ]; + + $logs = DB::table('action_logs') + ->select('action_type', 'item_type', 'filename', 'accept_signature') + ->where(function ($outer) use ($modelClass, $ids) { + $outer->where(function ($s) use ($modelClass, $ids) { + $s->where('item_type', $modelClass)->whereIn('item_id', $ids); + })->orWhere(function ($s) use ($modelClass, $ids) { + $s->where('target_type', $modelClass)->whereIn('target_id', $ids); + }); + }) + ->get(); + + $paths = []; + foreach ($logs as $log) { + // Map action_type to disk path for the attached file, or + // skip if the log carries no attachment we know how to route. + // Mirrors Actionlog::uploads_file_path() but works off raw + // query-builder rows (no Eloquent). + if (! empty($log->filename)) { + if ($log->action_type === ActionType::Accepted->value || $log->action_type === ActionType::Declined->value) { + $paths[] = 'private_uploads/eula-pdfs/'.$log->filename; + } elseif ($log->action_type === ActionType::Audit->value) { + $paths[] = 'private_uploads/audits/'.$log->filename; + } elseif ($log->item_type && isset($uploadRoots[$log->item_type])) { + $paths[] = rtrim($uploadRoots[$log->item_type], '/').'/'.$log->filename; + } + } + if (! empty($log->accept_signature)) { + $paths[] = 'private_uploads/signatures/'.$log->accept_signature; + } + } + + foreach (array_unique($paths) as $path) { + $this->tryUnlink($path); + } + } + + /** + * Delete files referenced by columns on the parent row itself + * (as opposed to action_logs). Covers CheckoutAcceptance's + * `signature_filename` and `stored_eula_file`, which store their + * paths inline on the row rather than in a related action_log. + */ + private function deletePrivateFileColumns(string $modelClass, string $table, Collection $ids): void + { + $privateFileColumns = [ + CheckoutAcceptance::class => [ + 'signature_filename' => 'private_uploads/signatures', + 'stored_eula_file' => 'private_uploads/eula-pdfs', + ], + ]; + + if (! array_key_exists($modelClass, $privateFileColumns)) { + return; + } + + foreach ($privateFileColumns[$modelClass] as $column => $subpath) { + $filenames = DB::table($table) + ->whereIn('id', $ids) + ->pluck($column) + ->filter() + ->unique(); + + foreach ($filenames as $filename) { + $this->tryUnlink(rtrim($subpath, '/').'/'.basename($filename)); + } + } + } + + /** + * Storage::delete with a log-and-continue on failure. All private- + * disk unlink calls funnel through here so error handling stays + * uniform. + */ + private function tryUnlink(string $key): void + { + try { + if (Storage::exists($key)) { + Storage::delete($key); + } + } catch (\Exception $e) { + Log::info('snipeit:purge - error deleting '.$key.': '.$e->getMessage()); + } + } } diff --git a/app/Http/Controllers/Accessories/AccessoriesController.php b/app/Http/Controllers/Accessories/AccessoriesController.php index ec7149d9fd..004ebbf053 100755 --- a/app/Http/Controllers/Accessories/AccessoriesController.php +++ b/app/Http/Controllers/Accessories/AccessoriesController.php @@ -9,7 +9,6 @@ use App\Models\Accessory; use App\Models\Company; use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Validator; @@ -218,14 +217,11 @@ class AccessoriesController extends Controller $accessory->loadCount('checkouts as checkouts_count'); if ($accessory->isDeletable()) { - if ($accessory->image) { - try { - Storage::disk('public')->delete('accessories'.'/'.$accessory->image); - } catch (\Exception $e) { - Log::debug($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping + // the file here means a restored soft-deleted row still has + // its image. $accessory->delete(); return redirect()->route('accessories.index')->with('success', trans('admin/accessories/message.delete.success')); diff --git a/app/Http/Controllers/Api/AssetModelsController.php b/app/Http/Controllers/Api/AssetModelsController.php index d5a36b6548..4acc3b67cf 100644 --- a/app/Http/Controllers/Api/AssetModelsController.php +++ b/app/Http/Controllers/Api/AssetModelsController.php @@ -16,7 +16,6 @@ use App\Models\Setting; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Http\Response; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; /** @@ -278,14 +277,13 @@ class AssetModelsController extends Controller return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/models/message.assoc_users'))); } - if ($assetmodel->image) { - try { - Storage::disk('public')->delete('assetmodels/'.$assetmodel->image); - } catch (\Exception $e) { - Log::info($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping the + // file here means a restored soft-deleted row still has its + // image. Also fixes a latent path bug: the old delete used + // `assetmodels/` but handleImages stores under `models/`, so + // the unlink here was silently missing the file anyway. $assetmodel->delete(); return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/models/message.delete.success'))); diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index de63a2cb16..26451c78cc 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -564,14 +564,11 @@ class AssetsController extends Controller ->update(['assigned_to' => null, 'assigned_type' => null]); } - if ($asset->image) { - try { - Storage::disk('public')->delete('assets/'.basename($asset->image)); - } catch (\Exception $e) { - Log::debug($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping the + // file here means a restored soft-deleted row still has its + // image. $asset->delete(); return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.delete.success')); diff --git a/app/Http/Controllers/BulkAccessoriesController.php b/app/Http/Controllers/BulkAccessoriesController.php index 8f153106fb..7f558c8e15 100644 --- a/app/Http/Controllers/BulkAccessoriesController.php +++ b/app/Http/Controllers/BulkAccessoriesController.php @@ -5,8 +5,6 @@ namespace App\Http\Controllers; use App\Models\Accessory; use Illuminate\Http\Request; use Illuminate\Support\Facades\Gate; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; class BulkAccessoriesController extends Controller { @@ -49,14 +47,11 @@ class BulkAccessoriesController extends Controller continue; } - if ($accessory->image) { - try { - Storage::disk('public')->delete('accessories/'.$accessory->image); - } catch (\Exception $e) { - Log::debug($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping + // the file here means a restored soft-deleted row still has + // its image. $accessory->delete(); $success_count++; } diff --git a/app/Http/Controllers/CompaniesController.php b/app/Http/Controllers/CompaniesController.php index ac3a778091..8e3521e4b0 100644 --- a/app/Http/Controllers/CompaniesController.php +++ b/app/Http/Controllers/CompaniesController.php @@ -13,8 +13,6 @@ use App\Models\User; use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; /** * This controller handles all actions related to Companies for @@ -155,14 +153,11 @@ final class CompaniesController extends Controller ->with('error', trans('admin/companies/message.assoc_users')); } - if ($company->image) { - try { - Storage::disk('public')->delete('companies'.'/'.$company->image); - } catch (\Exception $e) { - Log::debug($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping the + // file here means a restored soft-deleted row still has its + // image. $company->delete(); return redirect()->route('companies.index') diff --git a/app/Http/Controllers/Components/ComponentsController.php b/app/Http/Controllers/Components/ComponentsController.php index 90041c1857..f88a9d58db 100644 --- a/app/Http/Controllers/Components/ComponentsController.php +++ b/app/Http/Controllers/Components/ComponentsController.php @@ -11,8 +11,6 @@ use App\Models\Component; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Storage; /** * This class controls all actions related to Components for @@ -151,7 +149,7 @@ class ComponentsController extends Controller public function update(UpdateComponentRequest $request, Component $component) { $this->authorize('update', $component); - + // Update the component data $component->name = $request->input('name'); $component->category_id = $request->input('category_id'); @@ -200,15 +198,11 @@ class ComponentsController extends Controller $this->authorize('delete', $component); - // Remove the image if one exists - if ($component->image && Storage::disk('public')->exists('components/'.$component->image)) { - try { - Storage::disk('public')->delete('components/'.$component->image); - } catch (\Exception $e) { - Log::debug($e); - } - } - + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping the + // file here means a restored soft-deleted row still has its + // image. if ($component->numCheckedOut() > 0) { return redirect()->route('components.index')->with('error', trans('admin/components/message.delete.error_qty')); } diff --git a/app/Http/Controllers/DepartmentsController.php b/app/Http/Controllers/DepartmentsController.php index ad0322df07..567237abcf 100644 --- a/app/Http/Controllers/DepartmentsController.php +++ b/app/Http/Controllers/DepartmentsController.php @@ -8,7 +8,6 @@ use App\Models\Department; use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; class DepartmentsController extends Controller @@ -115,13 +114,11 @@ class DepartmentsController extends Controller return redirect()->to(route('departments.index'))->with('error', trans('admin/departments/message.assoc_users')); } - if ($department->image) { - try { - Storage::disk('public')->delete('departments'.'/'.$department->image); - } catch (\Exception $e) { - Log::debug($e); - } - } + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping the + // file here means a restored soft-deleted row still has its + // image. $department->delete(); return redirect()->back()->with('success', trans('admin/departments/message.delete.success')); diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 1df7a07e2f..b03816fe03 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -243,13 +243,11 @@ class LocationsController extends Controller if ($location->isDeletable()) { - if ($location->image) { - try { - Storage::disk('public')->delete('locations/'.$location->image); - } catch (\Exception $e) { - Log::error($e); - } - } + // Note: the image file is deliberately preserved across this + // soft-delete. Snipe-IT's `snipeit:purge` command permanently + // removes it later when the row is force-deleted. Keeping + // the file here means a restored soft-deleted row still has + // its image. $location->delete(); return redirect()->to(route('locations.index'))->with('success', trans('admin/locations/message.delete.success')); diff --git a/tests/Feature/Assets/Ui/DeleteAssetTest.php b/tests/Feature/Assets/Ui/DeleteAssetTest.php index a086c6238f..c2c606c6c0 100644 --- a/tests/Feature/Assets/Ui/DeleteAssetTest.php +++ b/tests/Feature/Assets/Ui/DeleteAssetTest.php @@ -99,8 +99,13 @@ class DeleteAssetTest extends TestCase Event::assertDispatched(CheckoutableCheckedIn::class); } - public function test_image_is_deleted_when_asset_deleted() + public function test_image_is_preserved_when_asset_soft_deleted() { + // Soft-deleting an asset preserves its image on disk so a + // restored asset still has one. The image is only removed for + // good by `snipeit:purge` when the row is force-deleted. + // Coverage for that permanent-removal path lives in + // `tests/Feature/Console/Commands/PurgeTest.php`. Storage::fake('public'); $asset = Asset::factory()->create(['image' => 'image.jpg']); @@ -112,6 +117,6 @@ class DeleteAssetTest extends TestCase $this->actingAs(User::factory()->deleteAssets()->create()) ->delete(route('hardware.destroy', $asset)); - Storage::disk('public')->assertMissing('assets/image.jpg'); + Storage::disk('public')->assertExists('assets/image.jpg'); } } diff --git a/tests/Feature/Components/Ui/DeleteComponentTest.php b/tests/Feature/Components/Ui/DeleteComponentTest.php index ece7b0d538..59c25510cc 100644 --- a/tests/Feature/Components/Ui/DeleteComponentTest.php +++ b/tests/Feature/Components/Ui/DeleteComponentTest.php @@ -50,8 +50,13 @@ class DeleteComponentTest extends TestCase implements TestsFullMultipleCompanies ->assertRedirect(route('components.index')); } - public function test_deleting_component_removes_component_image() + public function test_deleting_component_preserves_component_image() { + // Soft-deleting a component preserves its image on disk so a + // restored component still has one. The image is only removed + // for good by `snipeit:purge` when the row is force-deleted. + // Coverage for that permanent-removal path lives in + // `tests/Feature/Console/Commands/PurgeTest.php`. Storage::fake('public'); $component = Component::factory()->create(['image' => 'component-image.jpg']); @@ -62,7 +67,7 @@ class DeleteComponentTest extends TestCase implements TestsFullMultipleCompanies $this->actingAs(User::factory()->deleteComponents()->create())->delete(route('components.destroy', $component->id)); - Storage::disk('public')->assertMissing('components/component-image.jpg'); + Storage::disk('public')->assertExists('components/component-image.jpg'); } public function test_deleting_component_is_logged() diff --git a/tests/Feature/Console/Commands/PurgeTest.php b/tests/Feature/Console/Commands/PurgeTest.php index e25e9eec8a..835808a743 100644 --- a/tests/Feature/Console/Commands/PurgeTest.php +++ b/tests/Feature/Console/Commands/PurgeTest.php @@ -5,11 +5,13 @@ namespace Tests\Feature\Console\Commands; use App\Models\Accessory; use App\Models\Actionlog; use App\Models\Asset; +use App\Models\CheckoutAcceptance; use App\Models\License; use App\Models\Location; use App\Models\Maintenance; use App\Models\User; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Storage; use Tests\TestCase; /** @@ -122,16 +124,233 @@ class PurgeTest extends TestCase public function test_soft_deleted_user_with_show_in_list_zero_is_preserved(): void { - // System users (LDAP-sync placeholders, etc.) set show_in_list=0 - // and are excluded from purge even when soft-deleted. This filter - // was in the pre-refactor code and must be preserved. - $systemUser = User::factory()->create(['show_in_list' => 0]); - $systemUser->delete(); + // show_in_list=0 excludes a user from checkout-target dropdowns + // in the UI. Purge preserves these users so they stick around + // even when soft-deleted (matches the pre-refactor behavior). + $nonCheckoutUser = User::factory()->create(['show_in_list' => 0]); + $nonCheckoutUser->delete(); $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); // Row is gone-from-index (soft-deleted) but still in the table. - $this->assertDatabaseHas('users', ['id' => $systemUser->id]); + $this->assertDatabaseHas('users', ['id' => $nonCheckoutUser->id]); + } + + public function test_purge_removes_uploaded_files_for_soft_deleted_users(): void + { + // Regression guard: an intermediate refactor of Purge dropped the + // Storage::delete() step that removes uploaded avatars/documents + // under private_uploads/users/ when a user is purged. Without + // this test, that call could silently disappear again and leave + // orphan files on disk. The old inline code lived in a per-user + // loop; the current implementation batches via a single + // action_logs query, and either shape needs to actually unlink + // the file for the corresponding trashed user. + Storage::fake(); + $user = User::factory()->create(); + $filename = "u{$user->id}-avatar.png"; + Storage::put("private_uploads/users/{$filename}", 'fake image bytes'); + + Actionlog::factory()->create([ + 'item_type' => User::class, + 'item_id' => $user->id, + 'action_type' => 'uploaded', + 'filename' => $filename, + ]); + + $user->delete(); + Storage::assertExists("private_uploads/users/{$filename}"); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing("private_uploads/users/{$filename}"); + $this->assertDatabaseMissing('users', ['id' => $user->id]); + } + + public function test_dry_run_does_not_delete_user_files(): void + { + // Companion guard: --dry-run must be side-effect-free on disk. + Storage::fake(); + $user = User::factory()->create(); + $filename = "u{$user->id}-avatar.png"; + Storage::put("private_uploads/users/{$filename}", 'fake image bytes'); + + Actionlog::factory()->create([ + 'item_type' => User::class, + 'item_id' => $user->id, + 'action_type' => 'uploaded', + 'filename' => $filename, + ]); + + $user->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true', '--dry-run' => true])->assertExitCode(0); + + Storage::assertExists("private_uploads/users/{$filename}"); + } + + public function test_purge_removes_image_files_for_soft_deleted_assets(): void + { + // Image column on the parent row itself. Snipe-IT stores these + // on the public disk under `{plural-type}/{filename}`. Removing + // them at purge time (rather than at soft-delete) means a + // restored soft-deleted asset still has its image intact. + Storage::fake('public'); + $asset = Asset::factory()->create(['image' => 'asset-42.jpg']); + Storage::disk('public')->put('assets/asset-42.jpg', 'fake image bytes'); + + $asset->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::disk('public')->assertMissing('assets/asset-42.jpg'); + $this->assertDatabaseMissing('assets', ['id' => $asset->id]); + } + + public function test_purge_removes_avatar_files_for_soft_deleted_users(): void + { + // Users' avatar column has its own public-disk subpath (`avatars`) + // distinct from every other model's `image` column. Covered + // separately because `UsersController::destroy` used to NOT + // delete the avatar and now (correctly) still doesn't; purge + // is the sole avatar-unlink path. + Storage::fake('public'); + $user = User::factory()->create(['avatar' => 'user-7.jpg']); + Storage::disk('public')->put('avatars/user-7.jpg', 'fake avatar bytes'); + + $user->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::disk('public')->assertMissing('avatars/user-7.jpg'); + $this->assertDatabaseMissing('users', ['id' => $user->id]); + } + + public function test_purge_removes_eula_pdfs_when_action_log_parent_is_purged(): void + { + // Signed-EULA PDFs live under `private_uploads/eula-pdfs/`. + // They're identified by an action_log with action_type of + // `accepted` or `declined`, not by item_type, so the routing + // logic in Purge has to key off action_type first. + Storage::fake(); + $asset = Asset::factory()->create(); + $eula = "eula-{$asset->id}.pdf"; + Storage::put("private_uploads/eula-pdfs/{$eula}", 'fake pdf bytes'); + + Actionlog::factory()->create([ + 'item_type' => Asset::class, + 'item_id' => $asset->id, + 'action_type' => 'accepted', + 'filename' => $eula, + ]); + + $asset->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing("private_uploads/eula-pdfs/{$eula}"); + } + + public function test_purge_removes_signature_files_from_action_logs(): void + { + // Signatures live under `private_uploads/signatures/` and are + // referenced by the `accept_signature` column on action_logs + // (not by the `filename` column and not by any specific + // action_type). Purge must read that column separately. + Storage::fake(); + $asset = Asset::factory()->create(); + $sig = "sig-{$asset->id}.png"; + Storage::put("private_uploads/signatures/{$sig}", 'fake signature bytes'); + + Actionlog::factory()->create([ + 'item_type' => Asset::class, + 'item_id' => $asset->id, + 'action_type' => 'checkout', + 'accept_signature' => $sig, + ]); + + $asset->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing("private_uploads/signatures/{$sig}"); + } + + public function test_purge_removes_audit_files_from_action_logs(): void + { + // Audit files (photos, notes attached during an audit) live + // under `private_uploads/audits/` and are keyed on + // `action_type = 'audit'` in the action_log. + Storage::fake(); + $asset = Asset::factory()->create(); + $auditFile = "audit-{$asset->id}.jpg"; + Storage::put("private_uploads/audits/{$auditFile}", 'fake audit photo'); + + Actionlog::factory()->create([ + 'item_type' => Asset::class, + 'item_id' => $asset->id, + 'action_type' => 'audit', + 'filename' => $auditFile, + ]); + + $asset->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing("private_uploads/audits/{$auditFile}"); + } + + public function test_purge_matches_action_log_files_via_target_columns_too(): void + { + // Signatures/EULAs for checkouts are recorded with the + // checkoutable item under `item_*` and the recipient user under + // `target_*`. Purging the recipient user must clean up their + // signature file, even though the action_log's `item_type` + // points at Asset (not User). + Storage::fake(); + $user = User::factory()->create(); + $sig = "user-{$user->id}-sig.png"; + Storage::put("private_uploads/signatures/{$sig}", 'fake signature bytes'); + + Actionlog::factory()->create([ + 'item_type' => Asset::class, + 'item_id' => Asset::factory()->create()->id, + 'target_type' => User::class, + 'target_id' => $user->id, + 'action_type' => 'checkout', + 'accept_signature' => $sig, + ]); + + $user->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing("private_uploads/signatures/{$sig}"); + } + + public function test_purge_removes_checkout_acceptance_signature_and_eula_files(): void + { + // CheckoutAcceptance stores its signature filename and the + // rendered EULA PDF inline on the row (not via a related + // action_log). Both need to be unlinked when the acceptance is + // itself purged. + Storage::fake(); + $acceptance = CheckoutAcceptance::factory() + ->withoutActionLog() + ->accepted() + ->create([ + 'signature_filename' => 'acceptance-sig.png', + 'stored_eula_file' => 'acceptance-eula.pdf', + ]); + Storage::put('private_uploads/signatures/acceptance-sig.png', 'sig bytes'); + Storage::put('private_uploads/eula-pdfs/acceptance-eula.pdf', 'pdf bytes'); + + $acceptance->delete(); + + $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0); + + Storage::assertMissing('private_uploads/signatures/acceptance-sig.png'); + Storage::assertMissing('private_uploads/eula-pdfs/acceptance-eula.pdf'); } public function test_soft_deleted_location_is_purged(): void