Merge pull request #13926 from snipe/bug/sc-24106

Fixed issue where delete then restore could result in duplicate asset tags

app/Http/Controllers/Api/AssetsController.php

@@ -754,34 +754,32 @@ class AssetsController extends Controller
      */
     public function restore(Request $request, $assetId = null)
     {
-        // Get asset information
-        $asset = Asset::withTrashed()->find($assetId);
-        $this->authorize('delete', $asset);
 
-        if (isset($asset->id)) {
+        if ($asset = Asset::withTrashed()->find($assetId)) {
+            $this->authorize('delete', $asset);
 
-            if ($asset->deleted_at=='') {
+            if ($asset->deleted_at == '') {
-               $message = 'Asset was not deleted. No data was changed.';
+                return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.asset')])), 200);
+            }
 
-            } else {
+            if ($asset->restore()) {
-
-                $message = trans('admin/hardware/message.restore.success');
-                // Restore the asset
-                Asset::withTrashed()->where('id', $assetId)->restore();
 
                 $logaction = new Actionlog();
                 $logaction->item_type = Asset::class;
                 $logaction->item_id = $asset->id;
-                $logaction->created_at =  date("Y-m-d H:i:s");
+                $logaction->created_at = date('Y-m-d H:i:s');
                 $logaction->user_id = Auth::user()->id;
                 $logaction->logaction('restored');
+
+                return response()->json(Helper::formatStandardApiResponse('success', trans('admin/hardware/message.restore.success')), 200);
             }
 
-            return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset, $request), $message));
+            // Check validation to make sure we're not restoring an asset with the same asset tag (or unique attribute) as an existing asset
-        
+            return response()->json(Helper::formatStandardApiResponse('error', trans('general.could_not_restore', ['item_type' => trans('general.asset'), 'error' => $asset->getErrors()->first()])), 200);
-
         }
+
         return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200);
+
     }
 
     /**

app/Http/Controllers/Api/ManufacturersController.php

@@ -6,9 +6,11 @@ use App\Helpers\Helper;
 use App\Http\Controllers\Controller;
 use App\Http\Transformers\ManufacturersTransformer;
 use App\Http\Transformers\SelectlistTransformer;
+use App\Models\Actionlog;
 use App\Models\Manufacturer;
 use Illuminate\Http\Request;
 use App\Http\Requests\ImageUploadRequest;
+use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Storage;
 
 class ManufacturersController extends Controller
@@ -159,6 +161,44 @@ class ManufacturersController extends Controller
 
     }
 
+    /**
+     * Restore a given Manufacturer (mark as un-deleted)
+     *
+     * @author [A. Gianotto] [<snipe@snipe.net>]
+     * @since [v6.3.4]
+     * @param int $id
+     * @return \Illuminate\Http\JsonResponse
+     * @throws \Illuminate\Auth\Access\AuthorizationException
+     */
+    public function restore($id)
+    {
+        $this->authorize('delete', Manufacturer::class);
+
+        if ($manufacturer = Manufacturer::withTrashed()->find($id)) {
+
+            if ($manufacturer->deleted_at == '') {
+                return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')])), 200);
+            }
+
+            if ($manufacturer->restore()) {
+
+                $logaction = new Actionlog();
+                $logaction->item_type = Manufacturer::class;
+                $logaction->item_id = $manufacturer->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+                return response()->json(Helper::formatStandardApiResponse('success', trans('admin/manufacturers/message.restore.success')), 200);
+            }
+
+            // Check validation to make sure we're not restoring an item with the same unique attributes as a non-deleted one
+            return response()->json(Helper::formatStandardApiResponse('error', trans('general.could_not_restore', ['item_type' => trans('general.manufacturer'), 'error' => $manufacturer->getErrors()->first()])), 200);
+        }
+
+        return response()->json(Helper::formatStandardApiResponse('error', null,  trans('admin/manufacturers/message.does_not_exist')));
+    }
+
     /**
      * Gets a paginated collection for the select2 menus
      *

app/Http/Controllers/Api/UsersController.php

@@ -11,6 +11,7 @@ use App\Http\Transformers\ConsumablesTransformer;
 use App\Http\Transformers\LicensesTransformer;
 use App\Http\Transformers\SelectlistTransformer;
 use App\Http\Transformers\UsersTransformer;
+use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
 use App\Models\License;
@@ -688,17 +689,31 @@ class UsersController extends Controller
      */
     public function restore($userId = null)
     {
-        // Get asset information
-        $user = User::withTrashed()->find($userId);
-        $this->authorize('delete', $user);
-        if (isset($user->id)) {
-            // Restore the user
-            User::withTrashed()->where('id', $userId)->restore();
 
-            return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.success.restored')));
+        if ($user = User::withTrashed()->find($userId)) {
+            $this->authorize('delete', $user);
+
+            if ($user->deleted_at == '') {
+                return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.user')])), 200);
+            }
+
+            if ($user->restore()) {
+
+                $logaction = new Actionlog();
+                $logaction->item_type = User::class;
+                $logaction->item_id = $user->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+                return response()->json(Helper::formatStandardApiResponse('success', trans('admin/users/message.restore.success')), 200);
+            }
+
+            // Check validation to make sure we're not restoring a user with the same username as an existing user
+            return response()->json(Helper::formatStandardApiResponse('error', trans('general.could_not_restore', ['item_type' => trans('general.user'), 'error' => $user->getErrors()->first()])), 200);
         }
-        
+
-        $id = $userId;
+        return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found')), 200);
-        return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id'))), 200);
+
     }
 }

app/Http/Controllers/AssetModelsController.php

@@ -4,7 +4,10 @@ namespace App\Http\Controllers;
 
 use App\Helpers\Helper;
 use App\Http\Requests\ImageUploadRequest;
+use App\Models\Actionlog;
+use App\Models\Asset;
 use App\Models\AssetModel;
+use App\Models\User;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Input;
@@ -209,7 +212,7 @@ class AssetModelsController extends Controller
         $this->authorize('delete', AssetModel::class);
         // Check if the model exists
         if (is_null($model = AssetModel::find($modelId))) {
-            return redirect()->route('models.index')->with('error', trans('admin/models/message.not_found'));
+            return redirect()->route('models.index')->with('error', trans('admin/models/message.does_not_exist'));
         }
 
         if ($model->assets()->count() > 0) {
@@ -237,22 +240,42 @@ class AssetModelsController extends Controller
      *
      * @author [A. Gianotto] [<snipe@snipe.net>]
      * @since [v1.0]
-     * @param int $modelId
+     * @param int $id
      * @return Redirect
      * @throws \Illuminate\Auth\Access\AuthorizationException
      */
-    public function getRestore($modelId = null)
+    public function getRestore($id)
     {
         $this->authorize('create', AssetModel::class);
-        // Get user information
-        $model = AssetModel::withTrashed()->find($modelId);
 
-        if (isset($model->id)) {
+        if ($model = AssetModel::withTrashed()->find($id)) {
-            $model->restore();
 
-            return redirect()->route('models.index')->with('success', trans('admin/models/message.restore.success'));
+            if ($model->deleted_at == '') {
+                return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.asset_model')]));
+            }
+
+            if ($model->restore()) {
+                $logaction = new Actionlog();
+                $logaction->item_type = User::class;
+                $logaction->item_id = $model->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+
+                // Redirect them to the deleted page if there are more, otherwise the section index
+                $deleted_models = AssetModel::onlyTrashed()->count();
+                if ($deleted_models > 0) {
+                    return redirect()->back()->with('success', trans('admin/models/message.restore.success'));
+                }
+                return redirect()->route('models.index')->with('success', trans('admin/models/message.restore.success'));
+            }
+
+            // Check validation
+            return redirect()->back()->with('error', trans('general.could_not_restore', ['item_type' => trans('general.asset_model'), 'error' => $model->getErrors()->first()]));
         }
-        return redirect()->back()->with('error', trans('admin/models/message.not_found'));
+
+        return redirect()->back()->with('error', trans('admin/models/message.does_not_exist'));
 
     }
 

app/Http/Controllers/Assets/AssetsController.php

@@ -6,6 +6,7 @@ use App\Helpers\Helper;
 use App\Http\Controllers\Controller;
 use App\Http\Requests\ImageUploadRequest;
 use App\Models\Actionlog;
+use App\Models\Manufacturer;
 use Illuminate\Support\Facades\Log;
 use App\Models\Asset;
 use App\Models\AssetModel;
@@ -794,21 +795,31 @@ class AssetsController extends Controller
      */
     public function getRestore($assetId = null)
     {
-        // Get asset information
+        if ($asset = Asset::withTrashed()->find($assetId)) {
-        $asset = Asset::withTrashed()->find($assetId);
+            $this->authorize('delete', $asset);
-        $this->authorize('delete', $asset);
-        if (isset($asset->id)) {
-            // Restore the asset
-            Asset::withTrashed()->where('id', $assetId)->restore();
 
-            $logaction = new Actionlog();
+            if ($asset->deleted_at == '') {
-            $logaction->item_type = Asset::class;
+                return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.asset')]));
-            $logaction->item_id = $asset->id;
+            }
-            $logaction->created_at = date('Y-m-d H:i:s');
-            $logaction->user_id = Auth::user()->id;
-            $logaction->logaction('restored');
 
-            return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.restore.success'));
+            if ($asset->restore()) {
+                $logaction = new Actionlog();
+                $logaction->item_type = Asset::class;
+                $logaction->item_id = $asset->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+                // Redirect them to the deleted page if there are more, otherwise the section index
+                $deleted_assets = Asset::onlyTrashed()->count();
+                if ($deleted_assets > 0) {
+                    return redirect()->back()->with('success', trans('admin/hardware/message.restore.success'));
+                }
+                return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.restore.success'));
+            }
+
+            // Check validation to make sure we're not restoring an asset with the same asset tag (or unique attribute) as an existing asset
+            return redirect()->back()->with('error', trans('general.could_not_restore', ['item_type' => trans('general.asset'), 'error' => $asset->getErrors()->first()]));
         }
 
         return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist'));

app/Http/Controllers/ManufacturersController.php

@@ -2,8 +2,12 @@
 
 namespace App\Http\Controllers;
 
+use App\Helpers\Helper;
 use App\Http\Requests\ImageUploadRequest;
+use App\Models\Actionlog;
+use App\Models\Asset;
 use App\Models\Manufacturer;
+use App\Models\User;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Storage;
@@ -218,22 +222,37 @@ class ManufacturersController extends Controller
      * @return Redirect
      * @throws \Illuminate\Auth\Access\AuthorizationException
      */
-    public function restore($manufacturers_id)
+    public function restore($id)
     {
-        $this->authorize('create', Manufacturer::class);
+        $this->authorize('delete', Manufacturer::class);
-        $manufacturer = Manufacturer::onlyTrashed()->where('id', $manufacturers_id)->first();
 
-        if ($manufacturer) {
+        if ($manufacturer = Manufacturer::withTrashed()->find($id)) {
+
+            if ($manufacturer->deleted_at == '') {
+                return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')]));
+            }
 
-            // Not sure why this is necessary - it shouldn't fail validation here, but it fails without this, so....
-            $manufacturer->setValidating(false);
             if ($manufacturer->restore()) {
+                $logaction = new Actionlog();
+                $logaction->item_type = Manufacturer::class;
+                $logaction->item_id = $manufacturer->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+                // Redirect them to the deleted page if there are more, otherwise the section index
+                $deleted_manufacturers = Manufacturer::onlyTrashed()->count();
+                if ($deleted_manufacturers > 0) {
+                    return redirect()->back()->with('success', trans('admin/manufacturers/message.success.restored'));
+                }
                 return redirect()->route('manufacturers.index')->with('success', trans('admin/manufacturers/message.restore.success'));
             }
 
-            return redirect()->back()->with('error', 'Could not restore.');
+            // Check validation to make sure we're not restoring an asset with the same asset tag (or unique attribute) as an existing asset
+            return redirect()->back()->with('error', trans('general.could_not_restore', ['item_type' => trans('general.manufacturer'), 'error' => $manufacturer->getErrors()->first()]));
         }
 
-        return redirect()->back()->with('error', trans('admin/manufacturers/message.does_not_exist'));
+        return redirect()->route('manufacturers.index')->with('error', trans('admin/manufacturers/message.does_not_exist'));
+
     }
 }

app/Http/Controllers/Users/UsersController.php

@@ -7,10 +7,10 @@ use App\Http\Controllers\Controller;
 use App\Http\Controllers\UserNotFoundException;
 use App\Http\Requests\ImageUploadRequest;
 use App\Http\Requests\SaveUserRequest;
+use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
 use App\Models\Group;
-use App\Models\Ldap;
 use App\Models\Setting;
 use App\Models\User;
 use App\Notifications\WelcomeNotification;
@@ -385,18 +385,35 @@ class UsersController extends Controller
      */
     public function getRestore($id = null)
     {
-        $this->authorize('update', User::class);
+        if ($user = User::withTrashed()->find($id)) {
-        // Get user information
+            $this->authorize('delete', $user);
-        if (! User::onlyTrashed()->find($id)) {
+
-            return redirect()->route('users.index')->with('error', trans('admin/users/messages.user_not_found'));
+            if ($user->deleted_at == '') {
+                return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.user')]));
+            }
+
+            if ($user->restore()) {
+                $logaction = new Actionlog();
+                $logaction->item_type = User::class;
+                $logaction->item_id = $user->id;
+                $logaction->created_at = date('Y-m-d H:i:s');
+                $logaction->user_id = Auth::user()->id;
+                $logaction->logaction('restored');
+
+                // Redirect them to the deleted page if there are more, otherwise the section index
+                $deleted_users = User::onlyTrashed()->count();
+                if ($deleted_users > 0) {
+                    return redirect()->back()->with('success', trans('admin/users/message.success.restored'));
+                }
+                return redirect()->route('users.index')->with('success', trans('admin/users/message.success.restored'));
+
+            }
+
+            // Check validation to make sure we're not restoring a user with the same username as an existing user
+            return redirect()->back()->with('error', trans('general.could_not_restore', ['item_type' => trans('general.user'), 'error' => $user->getErrors()->first()]));
         }
 
-        // Restore the user
+        return redirect()->route('users.index')->with('error', trans('admin/users/message.does_not_exist'));
-        if (User::withTrashed()->where('id', $id)->restore()) {
-            return redirect()->route('users.index')->with('success', trans('admin/users/message.success.restored'));
-        }
-
-        return redirect()->route('users.index')->with('error', 'User could not be restored.');
     }
 
     /**

app/Http/Transformers/AssetModelsTransformer.php

@@ -73,7 +73,7 @@ class AssetModelsTransformer
 
         $permissions_array['available_actions'] = [
             'update' => (Gate::allows('update', AssetModel::class) && ($assetmodel->deleted_at == '')),
-            'delete' => (Gate::allows('delete', AssetModel::class) && ($assetmodel->assets_count == 0)),
+            'delete' => $assetmodel->isDeletable(),
             'clone' => (Gate::allows('create', AssetModel::class) && ($assetmodel->deleted_at == '')),
             'restore' => (Gate::allows('create', AssetModel::class) && ($assetmodel->deleted_at != '')),
         ];

app/Http/Transformers/AssetsTransformer.php

@@ -147,7 +147,7 @@ class AssetsTransformer
             'clone'         => Gate::allows('create', Asset::class) ? true : false,
             'restore'       => ($asset->deleted_at!='' && Gate::allows('create', Asset::class)) ? true : false,
             'update'        => ($asset->deleted_at=='' && Gate::allows('update', Asset::class)) ? true : false,
-            'delete'        => ($asset->deleted_at=='' && $asset->assigned_to =='' && Gate::allows('delete', Asset::class)) ? true : false,
+            'delete'        => ($asset->deleted_at=='' && $asset->assigned_to =='' && Gate::allows('delete', Asset::class) && ($asset->deleted_at == '')) ? true : false,
         ];      
 
 

app/Http/Transformers/UsersTransformer.php

@@ -79,7 +79,7 @@ class UsersTransformer
 
         $permissions_array['available_actions'] = [
             'update' => (Gate::allows('update', User::class) && ($user->deleted_at == '')),
-            'delete' => (Gate::allows('delete', User::class) && ($user->assets_count == 0) && ($user->licenses_count == 0) && ($user->accessories_count == 0)),
+            'delete' => $user->isDeletable(),
             'clone' => (Gate::allows('create', User::class) && ($user->deleted_at == '')),
             'restore' => (Gate::allows('create', User::class) && ($user->deleted_at != '')),
         ];

app/Models/AssetModel.php

@@ -6,6 +6,7 @@ use App\Models\Traits\Searchable;
 use App\Presenters\Presentable;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\SoftDeletes;
+use Illuminate\Support\Facades\Gate;
 use Illuminate\Support\Facades\Storage;
 use Watson\Validating\ValidatingTrait;
 
@@ -188,6 +189,21 @@ class AssetModel extends SnipeModel
         return false;
     }
 
+
+    /**
+     * Checks if the model is deletable
+     *
+     * @author A. Gianotto <snipe@snipe.net>
+     * @since [v6.3.4]
+     * @return bool
+     */
+    public function isDeletable()
+    {
+        return Gate::allows('delete', $this)
+            && ($this->assets_count == 0)
+            && ($this->deleted_at == '');
+    }
+
     /**
      * Get uploads for this model
      *

app/Models/Category.php

@@ -100,7 +100,8 @@ class Category extends SnipeModel
     {
 
         return Gate::allows('delete', $this)
-                && ($this->itemCount() == 0);
+                && ($this->itemCount() == 0)
+                && ($this->deleted_at == '');
     }
 
     /**

app/Models/Manufacturer.php

@@ -77,7 +77,8 @@ class Manufacturer extends SnipeModel
             && ($this->assets()->count() === 0)
             && ($this->licenses()->count() === 0)
             && ($this->consumables()->count() === 0)
-            && ($this->accessories()->count() === 0);
+            && ($this->accessories()->count() === 0)
+            && ($this->deleted_at == '');
     }
 
     public function assets()

app/Models/User.php

@@ -17,6 +17,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Foundation\Auth\Access\Authorizable;
 use Illuminate\Notifications\Notifiable;
+use Illuminate\Support\Facades\Gate;
 use Laravel\Passport\HasApiTokens;
 use Watson\Validating\ValidatingTrait;
 
@@ -201,6 +202,23 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         return $this->checkPermissionSection('superuser');
     }
 
+    /**
+     * Checks if the user is deletable
+     *
+     * @author A. Gianotto <snipe@snipe.net>
+     * @since [v6.3.4]
+     * @return bool
+     */
+    public function isDeletable()
+    {
+        return Gate::allows('delete', $this)
+            && ($this->assets()->count() === 0)
+            && ($this->licenses()->count() === 0)
+            && ($this->consumables()->count() === 0)
+            && ($this->accessories()->count() === 0)
+            && ($this->deleted_at == '');
+    }
+
 
     /**
      * Establishes the user -> company relationship

resources/lang/en/general.php

@@ -72,6 +72,8 @@ return [
     'consumable'			=> 'Consumable',
     'consumables'			=> 'Consumables',
     'country'  				=> 'Country',
+    'could_not_restore'     => 'Error restoring :item_type: :error',
+    'not_deleted'           => 'The :item_type is not deleted so it cannot be restored',
     'create'				=> 'Create New',
     'created'               => 'Item Created',
     'created_asset'			=> 'created asset',

resources/views/partials/bootstrap-table.blade.php

@@ -279,7 +279,11 @@
                     + ' data-title="{{  trans('general.delete') }}" onClick="return false;">'
                     + '<i class="fas fa-trash" aria-hidden="true"></i><span class="sr-only">{{ trans('general.delete') }}</span></a>&nbsp;';
             } else {
-                actions += '<span data-tooltip="true" title="{{ trans('general.cannot_be_deleted') }}"><a class="btn btn-danger btn-sm delete-asset disabled" onClick="return false;"><i class="fas fa-trash"></i></a></span>&nbsp;';
+                // Do not show the delete button on things that are already deleted
+                if ((row.available_actions) && (row.available_actions.restore != true)) {
+                    actions += '<span data-tooltip="true" title="{{ trans('general.cannot_be_deleted') }}"><a class="btn btn-danger btn-sm delete-asset disabled" onClick="return false;"><i class="fas fa-trash"></i></a></span>&nbsp;';
+                }
+
             }
 
 

routes/api.php

@@ -706,6 +706,13 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi
                 ]
             )->name('api.manufacturers.selectlist');
 
+            Route::post('{id}/restore',
+                [
+                    Api\ManufacturersController::class,
+                    'restore'
+                ]
+            )->name('api.manufacturers.restore');
+
         }); 
     
         Route::resource('manufacturers', 
@@ -742,6 +749,13 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi
                 ]
             )->name('api.models.assets');
 
+            Route::post('{id}/restore',
+                [
+                    Api\AssetModelsController::class,
+                    'restore'
+                ]
+            )->name('api.models.restore');
+
         }); 
     
         Route::resource('models',