Merge pull request #16312 from snipe/small_cleanups

Small cleanups before release
This commit is contained in:
snipe 2025-02-24 13:53:28 +00:00 committed by GitHub
commit 0bea07e2f9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 179 additions and 174 deletions

View file

@ -128,11 +128,13 @@ class Handler extends ExceptionHandler
$model_name = last(explode('\\', $e->getModel()));
$route = str_plural(strtolower(last(explode('\\', $e->getModel())))).'.index';
// Sigh. Fucking laravel.
// Sigh.
if ($route == 'assets.index') {
$route = 'hardware.index';
} elseif ($route == 'reporttemplates.index') {
$route = 'reports/custom';
} elseif ($route == 'assetmodels.index') {
$route = 'models.index';
} elseif ($route == 'predefinedkits.index') {
$route = 'kits.index';
}

View file

@ -178,28 +178,16 @@ class AssetModelsController extends Controller
* @since [v1.0]
* @param int $modelId
*/
public function destroy($modelId) : RedirectResponse
public function destroy(AssetModel $model) : RedirectResponse
{
$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.does_not_exist'));
}
if ($model->assets()->count() > 0) {
// Throw an error that this model is associated with assets
return redirect()->route('models.index')->with('error', trans('admin/models/message.assoc_users'));
}
if ($model->image) {
try {
Storage::disk('public')->delete('models/'.$model->image);
$model->update(['image' => null]);
} catch (\Exception $e) {
Log::info($e);
}
}
// Delete the model
$model->delete();
@ -270,23 +258,20 @@ class AssetModelsController extends Controller
* @since [v1.0]
* @param int $modelId
*/
public function getClone($modelId = null) : View | RedirectResponse
public function getClone(AssetModel $model) : View | RedirectResponse
{
$this->authorize('create', AssetModel::class);
// Check if the model exists
if (is_null($model_to_clone = AssetModel::find($modelId))) {
return redirect()->route('models.index')->with('error', trans('admin/models/message.does_not_exist'));
}
$model = clone $model_to_clone;
$cloned_model = clone $model;
$model->id = null;
$model->deleted_at = null;
// Show the page
return view('models/edit')
->with('depreciation_list', Helper::depreciationList())
->with('item', $model)
->with('model_id', $model_to_clone->id)
->with('clone_model', $model_to_clone);
->with('model_id', $model->id)
->with('clone_model', $cloned_model);
}
@ -305,7 +290,7 @@ class AssetModelsController extends Controller
/**
* Returns a view that allows the user to bulk edit model attrbutes
* Returns a view that allows the user to bulk edit model attributes
*
* @author [A. Gianotto] [<snipe@snipe.net>]
* @since [v1.7]

View file

@ -58,11 +58,9 @@ class AssetModelsFilesController extends Controller
* @param int $fileId
* @since [v1.0]
*/
public function show($modelId = null, $fileId = null) : StreamedResponse | Response | RedirectResponse | BinaryFileResponse
public function show(AssetModel $model, $fileId = null) : StreamedResponse | Response | RedirectResponse | BinaryFileResponse
{
$model = AssetModel::find($modelId);
// the asset is valid
if (isset($model->id)) {
$this->authorize('view', $model);
if (! $log = Actionlog::find($fileId)) {
@ -87,12 +85,6 @@ class AssetModelsFilesController extends Controller
}
return StorageHelper::downloader($file);
}
// Prepare the error message
$error = trans('admin/hardware/message.does_not_exist', ['id' => $fileId]);
// Redirect to the hardware management page
return redirect()->route('hardware.index')->with('error', $error);
}
/**
@ -103,29 +95,21 @@ class AssetModelsFilesController extends Controller
* @param int $fileId
* @since [v1.0]
*/
public function destroy($modelId = null, $fileId = null) : RedirectResponse
public function destroy(AssetModel $model, $fileId = null) : RedirectResponse
{
$model = AssetModel::find($modelId);
$this->authorize('update', $model);
$rel_path = 'private_uploads/assetmodels';
// the asset is valid
if (isset($model->id)) {
$this->authorize('update', $model);
$log = Actionlog::find($fileId);
if ($log) {
if (Storage::exists($rel_path.'/'.$log->filename)) {
Storage::delete($rel_path.'/'.$log->filename);
}
$log->delete();
return redirect()->back()->withFragment('files')->with('success', trans('admin/hardware/message.deletefile.success'));
$this->authorize('update', $model);
$log = Actionlog::find($fileId);
if ($log) {
if (Storage::exists($rel_path.'/'.$log->filename)) {
Storage::delete($rel_path.'/'.$log->filename);
}
$log->delete();
return redirect()->back()->withFragment('files')->with('success', trans('admin/hardware/message.deletefile.success'));
}
// Redirect to the hardware management page
return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist'));
return redirect()->back()->withFragment('files')->with('success', trans('admin/hardware/message.deletefile.success'));
}
}

View file

@ -22,43 +22,34 @@ class UserFilesController extends Controller
*@author [A. Gianotto] [<snipe@snipe.net>]
* @since [v1.6]
*/
public function store(UploadFileRequest $request, $userId = null)
public function store(UploadFileRequest $request, User $user)
{
$user = User::find($userId);
$destinationPath = config('app.private_uploads').'/users';
$this->authorize('update', $user);
$files = $request->file('file');
if (isset($user->id)) {
$this->authorize('update', $user);
$logActions = [];
$files = $request->file('file');
if (is_null($files)) {
return redirect()->back()->with('error', trans('admin/users/message.upload.nofiles'));
}
foreach ($files as $file) {
$file_name = $request->handleFile('private_uploads/users/', 'user-'.$user->id, $file);
//Log the uploaded file to the log
$logAction = new Actionlog();
$logAction->item_id = $user->id;
$logAction->item_type = User::class;
$logAction->created_by = auth()->id();
$logAction->note = $request->input('notes');
$logAction->target_id = null;
$logAction->created_at = date("Y-m-d H:i:s");
$logAction->filename = $file_name;
$logAction->action_type = 'uploaded';
if (! $logAction->save()) {
return JsonResponse::create(['error' => 'Failed validation: '.print_r($logAction->getErrors(), true)], 500);
}
$logActions[] = $logAction;
}
// dd($logActions);
return redirect()->back()->withFragment('files')->with('success', trans('admin/users/message.upload.success'));
if (is_null($files)) {
return redirect()->back()->with('error', trans('admin/users/message.upload.nofiles'));
}
foreach ($files as $file) {
$file_name = $request->handleFile('private_uploads/users/', 'user-'.$user->id, $file);
//Log the uploaded file to the log
$logAction = new Actionlog();
$logAction->item_id = $user->id;
$logAction->item_type = User::class;
$logAction->created_by = auth()->id();
$logAction->note = $request->input('notes');
$logAction->target_id = null;
$logAction->created_at = date("Y-m-d H:i:s");
$logAction->filename = $file_name;
$logAction->action_type = 'uploaded';
if (! $logAction->save()) {
return JsonResponse::create(['error' => 'Failed validation: '.print_r($logAction->getErrors(), true)], 500);
}
return redirect()->back()->withFragment('files')->with('success', trans('admin/users/message.upload.success'));
}
return redirect()->back()->with('error', trans('admin/users/message.upload.nofiles'));
}
@ -110,7 +101,7 @@ class UserFilesController extends Controller
* @return mixed
* @throws \Illuminate\Auth\Access\AuthorizationException
*/
public function show($userId = null, $fileId = null)
public function show(User $user, $fileId = null)
{
@ -118,29 +109,21 @@ class UserFilesController extends Controller
return redirect()->route('users.show')->with('error', 'Invalid file request');
}
if ($user = User::find($userId)) {
$this->authorize('view', $user);
if ($log = Actionlog::whereNotNull('filename')->where('item_id', $user->id)->find($fileId)) {
$file = 'private_uploads/users/'.$log->filename;
if ($log = Actionlog::whereNotNull('filename')->where('item_id', $user->id)->find($fileId)) {
$file = 'private_uploads/users/'.$log->filename;
try {
return StorageHelper::showOrDownloadFile($file, $log->filename);
} catch (\Exception $e) {
return redirect()->route('users.show', ['user' => $user])->with('error', trans('general.file_not_found'));
}
try {
return StorageHelper::showOrDownloadFile($file, $log->filename);
} catch (\Exception $e) {
return redirect()->route('users.show', ['user' => $user])->with('error', trans('general.file_not_found'));
}
// The log record doesn't exist somehow
return redirect()->route('users.show', ['user' => $user])->with('error', trans('general.log_record_not_found'));
return redirect()->back()->with('error', trans('general.file_not_found'));
}
// Redirect to the user management page if the user doesn't exist
return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', ['id' => $userId]));
// The log record doesn't exist somehow
return redirect()->route('users.show', ['user' => $user])->with('error', trans('general.log_record_not_found'));
}
}

View file

@ -146,7 +146,7 @@ class ActionlogsTransformer
} elseif ($actionlog->itemType() == 'license') {
$file_url = route('show.licensefile', ['licenseId' => $actionlog->item->id, 'fileId' => $actionlog->id]);
} elseif ($actionlog->itemType() == 'user') {
$file_url = route('show/userfile', ['userId' => $actionlog->item->id, 'fileId' => $actionlog->id]);
$file_url = route('show/userfile', ['user' => $actionlog->item->id, 'fileId' => $actionlog->id]);
}
}
}

View file

@ -49,7 +49,7 @@ class BreadcrumbsServiceProvider extends ServiceProvider
Breadcrumbs::for('hardware.index', fn (Trail $trail) =>
$trail->parent('home', route('home'))
->push(trans('general.assets'), route('hardware.index'))
->push(request()->status.' Assets', route('hardware.index', ['status' => request()->status]))
->push(request()->status.' '.trans('general.assets'), route('hardware.index', ['status' => request()->status]))
);
} else {
@ -81,10 +81,20 @@ class BreadcrumbsServiceProvider extends ServiceProvider
/**
* Asset Model Breadcrumbs
*/
Breadcrumbs::for('models.index', fn (Trail $trail) =>
$trail->parent('hardware.index', route('hardware.index'))
->push(trans('general.asset_models'), route('models.index'))
);
if ((request()->is('models*')) && (request()->status=='deleted')) {
Breadcrumbs::for('models.index', fn (Trail $trail) =>
$trail->parent('hardware.index', route('hardware.index'))
->push(trans('general.asset_models'), route('models.index'))
->push(trans('general.deleted_models'), route('models.index', ['status' => request()->status]))
);
} else {
Breadcrumbs::for('models.index', fn (Trail $trail) =>
$trail->parent('hardware.index', route('hardware.index'))
->push(trans('general.asset_models'), route('models.index'))
);
}
Breadcrumbs::for('models.create', fn (Trail $trail) =>
$trail->parent('models.index', route('models.index'))
@ -511,10 +521,16 @@ class BreadcrumbsServiceProvider extends ServiceProvider
/**
* Users Breadcrumbs
*/
Breadcrumbs::for('users.index', fn (Trail $trail) =>
$trail->parent('home', route('home'))
->push(trans('general.users'), route('users.index'))
);
if ((request()->is('users*')) && (request()->status=='deleted')) {
Breadcrumbs::for('users.index', fn(Trail $trail) => $trail->parent('home', route('home'))
->push(trans('general.users'), route('users.index'))
->push(trans('general.deleted_users'), route('users.index'))
);
} else {
Breadcrumbs::for('users.index', fn(Trail $trail) => $trail->parent('home', route('home'))
->push(trans('general.users'), route('users.index'))
);
}
Breadcrumbs::for('users.create', fn (Trail $trail) =>
$trail->parent('users.index', route('users.index'))

View file

@ -3,7 +3,7 @@
return array(
'about_models_title' => 'About Asset Models',
'about_models_text' => 'Asset Models are a way to group identical assets. "MBP 2013", "IPhone 6s", etc.',
'deleted' => 'This model has been deleted.',
'deleted' => 'This model has been deleted.',
'bulk_delete' => 'Bulk Delete Asset Models',
'bulk_delete_help' => 'Use the checkboxes below to confirm the deletion of the selected asset models. Asset models that have assets associated with them cannot be deleted until the assets are associated with a different model.',
'bulk_delete_warn' => 'You are about to delete one asset model.|You are about to delete :model_count asset models.',

View file

@ -597,5 +597,7 @@ return [
'select_all_none' => 'Select/Unselect All',
'generic_model_not_found' => 'That :model was not found or you do not have permission to access it',
'deleted_models' => 'Deleted Asset Models',
'deleted_users' => 'Deleted Users',
];

View file

@ -214,7 +214,7 @@ dir="{{ Helper::determineLanguageDirection() }}">
</a>
<ul class="dropdown-menu">
@can('create', \App\Models\Asset::class)
<li{!! (Request::is('hardware/create') ? ' class="active">' : '') !!}>
<li{!! (Request::is('hardware/create') ? ' class="active"' : '') !!}>
<a href="{{ route('hardware.create') }}" tabindex="-1">
<x-icon type="assets" />
{{ trans('general.asset') }}

View file

@ -31,7 +31,18 @@
<div class="row">
@if ($model->deleted_at!='')
<div class="col-md-12">
<div class="callout callout-warning">
<x-icon type="warning" />
{{ trans('admin/models/general.deleted') }}
</div>
</div>
@endif
<div class="col-md-9">
<div class="nav-tabs-custom">
<ul class="nav nav-tabs">
@ -153,6 +164,24 @@
</li>
@endif
@if ($model->created_by)
<li>{{ trans('general.created_by') }}:
{{ $model->adminuser->present()->name() }}
</li>
@endif
@if ($model->deleted_at)
<li>
<strong>
<span class="text-danger">
{{ trans('general.deleted') }}:
{{ Helper::getFormattedDateObject($model->deleted_at, 'datetime', false) }}
</span>
</strong>
</li>
@endif
@if ($model->min_amt)
<li>{{ trans('general.min_amt') }}:
{{$model->min_amt }}
@ -243,7 +272,7 @@
</div>
@can('update', \App\Models\AssetModel::class)
<div class="col-md-12" style="padding-bottom: 5px;">
<a href="{{ route('models.edit', $model->id) }}" style="width: 100%;" class="btn btn-sm btn-warning btn-social hidden-print">
<a href="{{ ($model->deleted_at=='') ? route('models.edit', $model->id) : '#' }}" style="width: 100%;" class="btn btn-sm btn-warning btn-social hidden-print{{ ($model->deleted_at!='') ? ' disabled' : '' }}">
<x-icon type="edit" />
{{ trans('admin/models/table.edit') }}
</a>

View file

@ -12,7 +12,21 @@
<div class="row">
@if ($user->deleted_at!='')
<div class="col-md-12">
<div class="callout callout-warning">
<x-icon type="warning" />
{{ trans('admin/users/message.user_deleted_warning') }}
</div>
</div>
@endif
<div class="col-md-12">
<div class="nav-tabs-custom">
<ul class="nav nav-tabs hidden-print">
@ -115,27 +129,6 @@
@can('update', $user)
<li class="dropdown pull-right">
<a class="dropdown-toggle" data-toggle="dropdown" href="#">
<span class="hidden-xs"><x-icon type="cog" /></span>
<span class="hidden-lg hidden-md hidden-xl"><x-icon type="cog" class="fa-2x" /></span>
<span class="hidden-xs hidden-sm">
{{ trans('button.actions') }}
</span>
<span class="caret"></span>
</a>
<ul class="dropdown-menu">
<li><a href="{{ route('users.edit', $user->id) }}">{{ trans('admin/users/general.edit') }}</a></li>
<li><a href="{{ route('users.clone.show', $user->id) }}">{{ trans('admin/users/general.clone') }}</a></li>
@if ((Auth::user()->id !== $user->id) && (!config('app.lock_passwords')) && ($user->deleted_at=='') && ($user->isDeletable()))
<li><a href="{{ route('users.destroy', $user->id) }}">{{ trans('button.delete') }}</a></li>
@endif
</ul>
</li>
@endcan
@can('update', \App\Models\User::class)
<li class="pull-right">
<a href="#" data-toggle="modal" data-target="#uploadFileModal">
<span class="hidden-xs"><x-icon type="paperclip" /></span>
@ -150,15 +143,6 @@
<div class="tab-pane active" id="details">
<div class="row">
@if ($user->deleted_at!='')
<div class="col-md-12">
<div class="callout callout-warning">
<x-icon type="warning" />
{{ trans('admin/users/message.user_deleted_warning') }}
</div>
</div>
@endif
<div class="info-stack-container">
<!-- Start button column -->
<div class="col-md-3 col-xs-12 col-sm-push-9 info-stack">
@ -180,7 +164,7 @@
@can('update', $user)
<div class="col-md-12">
<a href="{{ route('users.edit', $user->id) }}" style="width: 100%;" class="btn btn-sm btn-warning btn-social hidden-print">
<a href="{{ ($user->deleted_at=='') ? route('users.edit', $user->id) : '#' }}" style="width: 100%;" class="btn btn-sm btn-warning btn-social hidden-print{{ ($user->deleted_at!='') ? ' disabled' : '' }}">
<x-icon type="edit" />
{{ trans('admin/users/general.edit') }}
</a>
@ -1044,7 +1028,6 @@
data-bulk-button-id="#bulkLocationsEditButton"
data-bulk-form-id="#locationsBulkForm"
data-search="true"
data-show-footer="true"
data-side-pagination="server"
data-show-columns="true"
data-show-fullscreen="true"
@ -1077,7 +1060,6 @@
data-bulk-button-id="#bulkUserEditButton"
data-bulk-form-id="#usersBulkForm"
data-search="true"
data-show-footer="true"
data-side-pagination="server"
data-show-columns="true"
data-show-fullscreen="true"

View file

@ -10,33 +10,33 @@ use Illuminate\Support\Facades\Route;
Route::group(['prefix' => 'models', 'middleware' => ['auth']], function () {
Route::post('{modelID}/upload',
Route::post('{model}/upload',
[AssetModelsFilesController::class, 'store']
)->name('upload/models');
)->name('upload/models')->withTrashed();
Route::get('{modelID}/showfile/{fileId}/{download?}',
Route::get('{model}/showfile/{fileId}/{download?}',
[AssetModelsFilesController::class, 'show']
)->name('show/modelfile');
)->name('show/modelfile')->withTrashed();
Route::delete('{modelID}/showfile/{fileId}/delete',
Route::delete('{model}/showfile/{fileId}/delete',
[AssetModelsFilesController::class, 'destroy']
)->name('delete/modelfile');
)->name('delete/modelfile')->withTrashed();
Route::get(
'{modelId}/clone',
'{model}/clone',
[
AssetModelsController::class,
'getClone'
]
)->name('models.clone.create');
)->name('models.clone.create')->withTrashed();
Route::post(
'{modelId}/clone',
'{model}/clone',
[
AssetModelsController::class,
'postCreate'
]
)->name('models.clone.store');
)->name('models.clone.store')->withTrashed();
Route::get(
'{modelId}/view',
@ -92,5 +92,4 @@ Route::group(['prefix' => 'models', 'middleware' => ['auth']], function () {
Route::resource('models', AssetModelsController::class, [
'middleware' => ['auth'],
'parameters' => ['model' => 'model_id'],
]);
])->withTrashed();

View file

@ -33,20 +33,20 @@ Route::group(['prefix' => 'users', 'middleware' => ['auth']], function () {
)->name('users.export');
Route::get(
'{userId}/clone',
'{user}/clone',
[
Users\UsersController::class,
'getClone'
]
)->name('users.clone.show');
)->name('users.clone.show')->withTrashed();
Route::post(
'{userId}/clone',
'{user}/clone',
[
Users\UsersController::class,
'postCreate'
]
)->name('users.clone.store');
)->name('users.clone.store')->withTrashed();
Route::post(
'{userId}/restore',
@ -65,12 +65,12 @@ Route::group(['prefix' => 'users', 'middleware' => ['auth']], function () {
)->name('unsuspend/user');
Route::post(
'{userId}/upload',
'{user}/upload',
[
Users\UserFilesController::class,
'store'
]
)->name('upload/user');
)->name('upload/user')->withTrashed();
Route::delete(
'{userId}/deletefile/{fileId}',
@ -81,12 +81,12 @@ Route::group(['prefix' => 'users', 'middleware' => ['auth']], function () {
)->name('userfile.destroy');
Route::get(
'{userId}/showfile/{fileId}',
'{user}/showfile/{fileId}',
[
Users\UserFilesController::class,
'show'
]
)->name('show/userfile');
)->name('show/userfile')->withTrashed();
Route::post(
'{userId}/password',
@ -145,11 +145,8 @@ Route::group(['prefix' => 'users', 'middleware' => ['auth']], function () {
]
)->name('users/bulkeditsave');
// pulling this out of the resource because I need route model binding in the request
Route::match(['put', 'patch'], '/{user}', [Users\UsersController::class, 'update'])->name('users.update');
});
Route::resource('users', Users\UsersController::class, [
'middleware' => ['auth'],
'except' => ['update']
]);
'middleware' => ['auth']
])->withTrashed();

View file

@ -0,0 +1,18 @@
<?php
namespace Tests\Feature\Users\Ui;
use App\Models\User;
use Tests\TestCase;
class CloneUserTest extends TestCase
{
public function testPageRenders()
{
$this->actingAs(User::factory()->superuser()->create())
->get(route('users.clone.show', User::factory()->create()))
->assertOk();
}
}

View file

@ -48,6 +48,14 @@ class DeleteUserTest extends TestCase
$this->followRedirects($response)->assertSee(trans('general.error'));
}
public function testCanViewSoftDeletedUser()
{
$user = User::factory()->deletedUser()->viewUsers()->create();
$response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
->get(route('users.show', $user->id))
->assertStatus(200);
}
public function testFmcsPermissionsToDeleteUser()
{