mirror of
https://github.com/snipe/snipe-it.git
synced 2026-08-18 11:15:42 +00:00
Merge pull request #19338 from grokability/purge-tweaks
Data Purge: Fixed #17580, code refinements from #19333
This commit is contained in:
@ -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<int, array{0: string, 1: int}>
|
||||
*/
|
||||
@ -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());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -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'));
|
||||
|
||||
@ -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')));
|
||||
|
||||
@ -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'));
|
||||
|
||||
@ -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++;
|
||||
}
|
||||
|
||||
@ -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')
|
||||
|
||||
@ -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'));
|
||||
}
|
||||
|
||||
@ -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'));
|
||||
|
||||
@ -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'));
|
||||
|
||||
@ -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');
|
||||
}
|
||||
}
|
||||
|
||||
@ -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()
|
||||
|
||||
@ -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
|
||||
|
||||
Reference in New Issue
Block a user