From ff145abbe720f36604585005b26cf62873169f56 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 12 Aug 2024 16:13:03 -0500 Subject: [PATCH 01/12] use array for eager loading, makes ide prettier --- app/Http/Controllers/Users/UsersController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 1e203e71d..bad16458d 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -232,7 +232,7 @@ class UsersController extends Controller $permissions = $request->input('permissions', []); app('request')->request->set('permissions', $permissions); - $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id); + $user = User::with(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed()->find($id); // User is valid - continue... if ($user) { @@ -263,7 +263,7 @@ class UsersController extends Controller $user->activated = $request->input('activated', 0); $user->jobtitle = $request->input('jobtitle', null); $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id', null); + $user->location_id = $request->input('location_id', null); //here $user->company_id = Company::getIdForUser($request->input('company_id', null)); $user->manager_id = $request->input('manager_id', null); $user->notes = $request->input('notes'); From cc3b8e0681f74276958c988d986de07cff3e798d Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 12 Aug 2024 16:58:21 -0500 Subject: [PATCH 02/12] this should more or less work, but i need to determine if this is the best way --- app/Http/Controllers/Users/UsersController.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index bad16458d..96fbae49a 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -263,7 +263,16 @@ class UsersController extends Controller $user->activated = $request->input('activated', 0); $user->jobtitle = $request->input('jobtitle', null); $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id', null); //here + $user->location_id = $request->input('location_id', null); + if ($request->has('company_id')) { + if ($user->assets->count() > 0) { + if ($user->assets()->pluck('company_id') != $user->getRawOriginal('company_id')) { + { + return back()->with('error', 'this user has assets, check them in first'); + } + } + } + } $user->company_id = Company::getIdForUser($request->input('company_id', null)); $user->manager_id = $request->input('manager_id', null); $user->notes = $request->input('notes'); From ec863df0074ce785aa3ac80f4871accb8eb2145b Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 12 Aug 2024 16:58:53 -0500 Subject: [PATCH 03/12] rm conditional that might be unnecessary --- app/Http/Controllers/Users/UsersController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 96fbae49a..adf2d5f3d 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -266,11 +266,11 @@ class UsersController extends Controller $user->location_id = $request->input('location_id', null); if ($request->has('company_id')) { if ($user->assets->count() > 0) { - if ($user->assets()->pluck('company_id') != $user->getRawOriginal('company_id')) { + //if ($user->assets()->pluck('company_id') != $user->getRawOriginal('company_id')) { { return back()->with('error', 'this user has assets, check them in first'); } - } + //} } } $user->company_id = Company::getIdForUser($request->input('company_id', null)); From 09f2739298933ea8150a6dfda4aa19b3c5baa94f Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 13 Aug 2024 13:45:41 -0500 Subject: [PATCH 04/12] works, un-reassignable licenses are an issue --- app/Http/Controllers/Users/UsersController.php | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index adf2d5f3d..9ff015248 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -186,7 +186,7 @@ class UsersController extends Controller { $this->authorize('update', User::class); - $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id); + $user = User::with(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed()->find($id); if ($user) { @@ -235,9 +235,15 @@ class UsersController extends Controller $user = User::with(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed()->find($id); // User is valid - continue... + + if ($user) { $this->authorize('update', $user); + if ($request->has('company_id') && $user->allAssignedCount() > 0 && Setting::getSettings()->full_multiple_companies_support) { + return back()->with('error', 'this user has assets, check them in first'); + } + // Figure out of this user was an admin before this edit $orig_permissions_array = $user->decodePermissions(); $orig_superuser = '0'; @@ -264,15 +270,7 @@ class UsersController extends Controller $user->jobtitle = $request->input('jobtitle', null); $user->phone = $request->input('phone'); $user->location_id = $request->input('location_id', null); - if ($request->has('company_id')) { - if ($user->assets->count() > 0) { - //if ($user->assets()->pluck('company_id') != $user->getRawOriginal('company_id')) { - { - return back()->with('error', 'this user has assets, check them in first'); - } - //} - } - } + $user->company_id = Company::getIdForUser($request->input('company_id', null)); $user->manager_id = $request->input('manager_id', null); $user->notes = $request->input('notes'); From 120cfd13c544fb79e69f5f59aa44f908b4a39947 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 13 Aug 2024 14:07:40 -0500 Subject: [PATCH 05/12] translation --- app/Http/Controllers/Users/UsersController.php | 2 +- resources/lang/en-US/admin/users/message.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 9ff015248..631ab3bc3 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -241,7 +241,7 @@ class UsersController extends Controller $this->authorize('update', $user); if ($request->has('company_id') && $user->allAssignedCount() > 0 && Setting::getSettings()->full_multiple_companies_support) { - return back()->with('error', 'this user has assets, check them in first'); + return back()->with('error', trans('admin/users/message.multi_company_items_assigned')); } // Figure out of this user was an admin before this edit diff --git a/resources/lang/en-US/admin/users/message.php b/resources/lang/en-US/admin/users/message.php index 4d014775b..0f41d463e 100644 --- a/resources/lang/en-US/admin/users/message.php +++ b/resources/lang/en-US/admin/users/message.php @@ -53,6 +53,7 @@ return array( 'ldap_could_not_search' => 'Could not search the LDAP server. Please check your LDAP server configuration in the LDAP config file.
Error from LDAP Server:', 'ldap_could_not_get_entries' => 'Could not get entries from the LDAP server. Please check your LDAP server configuration in the LDAP config file.
Error from LDAP Server:', 'password_ldap' => 'The password for this account is managed by LDAP/Active Directory. Please contact your IT department to change your password. ', + 'multi_company_items_assigned' => 'This user has items assigned, please check them in before moving companies.' ), 'deletefile' => array( From 20ec420ba3a51638f7934e5a3820907760fc544c Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 14 Aug 2024 13:53:29 -0500 Subject: [PATCH 06/12] not quite done, api side needs some work --- app/Http/Controllers/Api/UsersController.php | 2 +- .../Controllers/Users/UsersController.php | 122 ++++++++---------- app/Http/Requests/SaveUserRequest.php | 10 ++ routes/web/users.php | 5 +- 4 files changed, 70 insertions(+), 69 deletions(-) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 9200f80b1..ae6968a47 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -427,7 +427,7 @@ class UsersController extends Controller * @param \Illuminate\Http\Request $request * @param int $id */ - public function update(SaveUserRequest $request, $id) : JsonResponse + public function update(SaveUserRequest $request, User $user): JsonResponse { $this->authorize('update', User::class); diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 631ab3bc3..e609727c0 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -214,90 +214,84 @@ class UsersController extends Controller * @return \Illuminate\Http\RedirectResponse * @throws \Illuminate\Auth\Access\AuthorizationException */ - public function update(SaveUserRequest $request, $id = null) + public function update(SaveUserRequest $request, User $user) { $this->authorize('update', User::class); // This is a janky hack to prevent people from changing admin demo user data on the public demo. // The $ids 1 and 2 are special since they are seeded as superadmins in the demo seeder. // Thanks, jerks. You are why we can't have nice things. - snipe - - if ((($id == 1) || ($id == 2)) && (config('app.lock_passwords'))) { + if ((($user->id == 1) || ($user->id == 2)) && (config('app.lock_passwords'))) { return redirect()->route('users.index')->with('error', trans('general.permission_denied_superuser_demo')); } - // We need to reverse the UI specific logic for our // permissions here before we update the user. $permissions = $request->input('permissions', []); app('request')->request->set('permissions', $permissions); - $user = User::with(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed()->find($id); + $user->load(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed(); - // User is valid - continue... + $this->authorize('update', $user); - - if ($user) { - $this->authorize('update', $user); - - if ($request->has('company_id') && $user->allAssignedCount() > 0 && Setting::getSettings()->full_multiple_companies_support) { - return back()->with('error', trans('admin/users/message.multi_company_items_assigned')); - } + //see if i can get this working at request level + //if ($request->has('company_id') && $user->allAssignedCount() > 0 && Setting::getSettings()->full_multiple_companies_support) { + // return back()->with('error', trans('admin/users/message.multi_company_items_assigned')); + //} // Figure out of this user was an admin before this edit - $orig_permissions_array = $user->decodePermissions(); - $orig_superuser = '0'; - if (is_array($orig_permissions_array)) { - if (array_key_exists('superuser', $orig_permissions_array)) { - $orig_superuser = $orig_permissions_array['superuser']; - } + $orig_permissions_array = $user->decodePermissions(); + $orig_superuser = '0'; + if (is_array($orig_permissions_array)) { + if (array_key_exists('superuser', $orig_permissions_array)) { + $orig_superuser = $orig_permissions_array['superuser']; } + } - // Only save groups if the user is a superuser - if (auth()->user()->isSuperUser()) { - $user->groups()->sync($request->input('groups')); - } + // Only save groups if the user is a superuser + if (auth()->user()->isSuperUser()) { + $user->groups()->sync($request->input('groups')); + } - // Update the user fields - $user->username = trim($request->input('username')); - $user->email = trim($request->input('email')); - $user->first_name = $request->input('first_name'); - $user->last_name = $request->input('last_name'); - $user->two_factor_optin = $request->input('two_factor_optin') ?: 0; - $user->locale = $request->input('locale'); - $user->employee_num = $request->input('employee_num'); - $user->activated = $request->input('activated', 0); - $user->jobtitle = $request->input('jobtitle', null); - $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id', null); + // Update the user fields + $user->username = trim($request->input('username')); + $user->email = trim($request->input('email')); + $user->first_name = $request->input('first_name'); + $user->last_name = $request->input('last_name'); + $user->two_factor_optin = $request->input('two_factor_optin') ?: 0; + $user->locale = $request->input('locale'); + $user->employee_num = $request->input('employee_num'); + $user->activated = $request->input('activated', 0); + $user->jobtitle = $request->input('jobtitle', null); + $user->phone = $request->input('phone'); + $user->location_id = $request->input('location_id', null); + $user->company_id = Company::getIdForUser($request->input('company_id', null)); + $user->manager_id = $request->input('manager_id', null); + $user->notes = $request->input('notes'); + $user->department_id = $request->input('department_id', null); + $user->address = $request->input('address', null); + $user->city = $request->input('city', null); + $user->state = $request->input('state', null); + $user->country = $request->input('country', null); + // if a user is editing themselves we should always keep activated true + $user->activated = $request->input('activated', $request->user()->is($user) ? 1 : 0); + $user->zip = $request->input('zip', null); + $user->remote = $request->input('remote', 0); + $user->vip = $request->input('vip', 0); + $user->website = $request->input('website', null); + $user->start_date = $request->input('start_date', null); + $user->end_date = $request->input('end_date', null); + $user->autoassign_licenses = $request->input('autoassign_licenses', 0); - $user->company_id = Company::getIdForUser($request->input('company_id', null)); - $user->manager_id = $request->input('manager_id', null); - $user->notes = $request->input('notes'); - $user->department_id = $request->input('department_id', null); - $user->address = $request->input('address', null); - $user->city = $request->input('city', null); - $user->state = $request->input('state', null); - $user->country = $request->input('country', null); - // if a user is editing themselves we should always keep activated true - $user->activated = $request->input('activated', $request->user()->is($user) ? 1 : 0); - $user->zip = $request->input('zip', null); - $user->remote = $request->input('remote', 0); - $user->vip = $request->input('vip', 0); - $user->website = $request->input('website', null); - $user->start_date = $request->input('start_date', null); - $user->end_date = $request->input('end_date', null); - $user->autoassign_licenses = $request->input('autoassign_licenses', 0); + // Update the location of any assets checked out to this user + Asset::where('assigned_type', User::class) + ->where('assigned_to', $user->id) + ->update(['location_id' => $request->input('location_id', null)]); - // Update the location of any assets checked out to this user - Asset::where('assigned_type', User::class) - ->where('assigned_to', $user->id) - ->update(['location_id' => $request->input('location_id', null)]); - - // Do we want to update the user password? - if ($request->filled('password')) { - $user->password = bcrypt($request->input('password')); - } + // Do we want to update the user password? + if ($request->filled('password')) { + $user->password = bcrypt($request->input('password')); + } // Update the location of any assets checked out to this user @@ -325,13 +319,7 @@ class UsersController extends Controller return redirect()->to(Helper::getRedirectOption($request, $user->id, 'Users')) ->with('success', trans('admin/users/message.success.update')); } - return redirect()->back()->withInput()->withErrors($user->getErrors()); - - - } - - return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', compact('id'))); } /** diff --git a/app/Http/Requests/SaveUserRequest.php b/app/Http/Requests/SaveUserRequest.php index b38193c15..f77defb0b 100644 --- a/app/Http/Requests/SaveUserRequest.php +++ b/app/Http/Requests/SaveUserRequest.php @@ -31,9 +31,19 @@ class SaveUserRequest extends FormRequest */ public function rules() { + //dd($this->user); $rules = [ 'department_id' => 'nullable|exists:departments,id', 'manager_id' => 'nullable|exists:users,id', + 'company_id' => [ + // determines if the user is being moved between companies and checks to see if they have any items assigned + function ($attribute, $value, $fail) { + dd($this->user); + if (($this->has('company_id')) && ($this->user->allAssignedCount() > 0) && (Setting::getSettings()->full_multiple_companies_support)) { + $fail(trans('admin/users/message.error.multi_company_items_assigned')); + } + } + ] ]; switch ($this->method()) { diff --git a/routes/web/users.php b/routes/web/users.php index 95de20063..3bf1555ca 100644 --- a/routes/web/users.php +++ b/routes/web/users.php @@ -145,10 +145,13 @@ 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::patch('/{user}', [Users\UsersController::class, 'update'])->name('users.update'); + Route::put('/{user}', [Users\UsersController::class, 'update'])->name('users.put-update'); }); Route::resource('users', Users\UsersController::class, [ 'middleware' => ['auth'], 'parameters' => ['user' => 'user_id'], + 'except' => ['update'] ]); \ No newline at end of file From f031309f8ffce5747c29e24d467b93cf61ace657 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 14 Aug 2024 16:09:15 -0500 Subject: [PATCH 07/12] set up api controller for route/model binding --- app/Http/Controllers/Api/UsersController.php | 18 +----------------- app/Http/Requests/SaveUserRequest.php | 1 - 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index ae6968a47..856b3b6a6 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -431,9 +431,6 @@ class UsersController extends Controller { $this->authorize('update', User::class); - if ($user = User::find($id)) { - - $this->authorize('update', $user); /** @@ -443,12 +440,10 @@ class UsersController extends Controller * */ - - if ((($id == 1) || ($id == 2)) && (config('app.lock_passwords'))) { + if ((($user->id == 1) || ($user->id == 2)) && (config('app.lock_passwords'))) { return response()->json(Helper::formatStandardApiResponse('error', null, 'Permission denied. You cannot update user information via API on the demo.')); } - $user->fill($request->all()); if ($user->id == $request->input('manager_id')) { @@ -473,16 +468,13 @@ class UsersController extends Controller $user->permissions = $permissions_array; } - // Update the location of any assets checked out to this user Asset::where('assigned_type', User::class) ->where('assigned_to', $user->id)->update(['location_id' => $request->input('location_id', null)]); - app('App\Http\Requests\ImageUploadRequest')->handleImages($user, 600, 'image', 'avatars', 'avatar'); if ($user->save()) { - // Check if the request has groups passed and has a value, AND that the user us a superuser if (($request->has('groups')) && (auth()->user()->isSuperUser())) { @@ -496,18 +488,10 @@ class UsersController extends Controller // Sync the groups since the user is a superuser and the groups pass validation $user->groups()->sync($request->input('groups')); - - } - return response()->json(Helper::formatStandardApiResponse('success', (new UsersTransformer)->transformUser($user), trans('admin/users/message.success.update'))); } - return response()->json(Helper::formatStandardApiResponse('error', null, $user->getErrors())); - } - - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id')))); - } /** diff --git a/app/Http/Requests/SaveUserRequest.php b/app/Http/Requests/SaveUserRequest.php index f77defb0b..7598e7433 100644 --- a/app/Http/Requests/SaveUserRequest.php +++ b/app/Http/Requests/SaveUserRequest.php @@ -38,7 +38,6 @@ class SaveUserRequest extends FormRequest 'company_id' => [ // determines if the user is being moved between companies and checks to see if they have any items assigned function ($attribute, $value, $fail) { - dd($this->user); if (($this->has('company_id')) && ($this->user->allAssignedCount() > 0) && (Setting::getSettings()->full_multiple_companies_support)) { $fail(trans('admin/users/message.error.multi_company_items_assigned')); } From afaf53cdfc37d49ba1afbbe178cd974fbb83920f Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 14 Aug 2024 18:14:21 -0500 Subject: [PATCH 08/12] failing ui test --- resources/views/users/edit.blade.php | 4 ++- routes/web/users.php | 3 +- tests/Feature/Users/Api/UpdateUserTest.php | 30 ++++++++++++++++++++ tests/Feature/Users/Ui/UpdateUserTest.php | 32 ++++++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index e3f691f15..986c18ab6 100755 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -66,7 +66,9 @@
-
+ {{csrf_field()}} @if($user->id) diff --git a/routes/web/users.php b/routes/web/users.php index 3bf1555ca..e55541a93 100644 --- a/routes/web/users.php +++ b/routes/web/users.php @@ -146,8 +146,7 @@ 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::patch('/{user}', [Users\UsersController::class, 'update'])->name('users.update'); - Route::put('/{user}', [Users\UsersController::class, 'update'])->name('users.put-update'); + Route::match(['put', 'patch'], '/{user}', [Users\UsersController::class, 'update'])->name('users.update'); }); Route::resource('users', Users\UsersController::class, [ diff --git a/tests/Feature/Users/Api/UpdateUserTest.php b/tests/Feature/Users/Api/UpdateUserTest.php index 1c66bbdda..2901beea9 100644 --- a/tests/Feature/Users/Api/UpdateUserTest.php +++ b/tests/Feature/Users/Api/UpdateUserTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Users\Api; +use App\Models\Asset; use App\Models\Company; use App\Models\Department; use App\Models\Group; @@ -344,4 +345,33 @@ class UpdateUserTest extends TestCase $this->assertTrue($user->refresh()->groups->contains($groupB)); } + public function testMultiCompanyUserCannotBeMovedIfHasAsset() + { + $this->settings->enableMultipleFullCompanySupport(); + + $companyA = Company::factory()->create(); + $companyB = Company::factory()->create(); + + $user = User::factory()->create([ + 'company_id' => $companyA->id, + ]); + $superUser = User::factory()->superuser()->create(); + + $asset = Asset::factory()->create(); + + // no assets assigned, therefore success + $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ + 'username' => 'test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('success'); + + $asset->checkOut($user, $superUser); + + // asset assigned, therefore error + $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ + 'username' => 'test', + 'company_id' => $companyB->id, + ])->assertMessagesAre('error'); + } + } diff --git a/tests/Feature/Users/Ui/UpdateUserTest.php b/tests/Feature/Users/Ui/UpdateUserTest.php index bef1d59a0..31530af7c 100644 --- a/tests/Feature/Users/Ui/UpdateUserTest.php +++ b/tests/Feature/Users/Ui/UpdateUserTest.php @@ -2,6 +2,8 @@ namespace Tests\Feature\Users\Ui; +use App\Models\Asset; +use App\Models\Company; use App\Models\User; use Tests\TestCase; @@ -79,4 +81,34 @@ class UpdateUserTest extends TestCase $this->assertEquals(1, $admin->refresh()->activated); } + + public function testMultiCompanyUserCannotBeMovedIfHasAsset() + { + $this->settings->enableMultipleFullCompanySupport(); + + $companyA = Company::factory()->create(); + $companyB = Company::factory()->create(); + + $user = User::factory()->create([ + 'company_id' => $companyA->id, + ]); + $superUser = User::factory()->superuser()->create(); + + $asset = Asset::factory()->create(); + + // no assets assigned, therefore success + $this->actingAs($superUser)->put(route('users.update', $user), [ + 'first_name' => 'test', + 'username' => 'test', + 'company_id' => $companyB->id, + ])->assertRedirect(route('users.index')); + + //$asset->checkOut($user, $superUser); + + // asset assigned, therefore error + //$this->actingAs($superUser)->patchJson(route('users.update', $user), [ + // 'username' => 'test', + // 'company_id' => $companyB->id, + //])->assertMessagesAre('error'); + } } From 9622e05cf5b545bac1f506af085e6c7e27259848 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 14 Aug 2024 18:41:06 -0500 Subject: [PATCH 09/12] correct api test --- tests/Feature/Users/Api/UpdateUserTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/Users/Api/UpdateUserTest.php b/tests/Feature/Users/Api/UpdateUserTest.php index 2901beea9..e7314756d 100644 --- a/tests/Feature/Users/Api/UpdateUserTest.php +++ b/tests/Feature/Users/Api/UpdateUserTest.php @@ -371,7 +371,7 @@ class UpdateUserTest extends TestCase $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ 'username' => 'test', 'company_id' => $companyB->id, - ])->assertMessagesAre('error'); + ])->assertStatusMessageIs('error'); } } From dec4691c73b28bab35ed8fdb1ff83578ad517bb0 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Fri, 16 Aug 2024 12:50:09 -0500 Subject: [PATCH 10/12] should be good to go now --- routes/api.php | 8 +++++--- tests/Feature/Users/Api/UpdateUserTest.php | 14 ++++++++++++++ tests/Feature/Users/Ui/UpdateUserTest.php | 21 +++++++++++++-------- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/routes/api.php b/routes/api.php index 0eb0d834c..c59654d65 100644 --- a/routes/api.php +++ b/routes/api.php @@ -1070,18 +1070,20 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi ] )->name('api.users.restore'); - }); + }); + + Route::match(['put', 'patch'], '{user}/update', [Api\UsersController::class, 'update']) + ->name('api.users.update'); Route::resource('users', Api\UsersController::class, ['names' => [ 'index' => 'api.users.index', 'show' => 'api.users.show', - 'update' => 'api.users.update', 'store' => 'api.users.store', 'destroy' => 'api.users.destroy', ], - 'except' => ['create', 'edit'], + 'except' => ['create', 'edit', 'update'], 'parameters' => ['user' => 'user_id'], ] ); // end users API routes diff --git a/tests/Feature/Users/Api/UpdateUserTest.php b/tests/Feature/Users/Api/UpdateUserTest.php index e7314756d..4632a80c3 100644 --- a/tests/Feature/Users/Api/UpdateUserTest.php +++ b/tests/Feature/Users/Api/UpdateUserTest.php @@ -365,6 +365,13 @@ class UpdateUserTest extends TestCase 'company_id' => $companyB->id, ])->assertStatusMessageIs('success'); + // same test but PUT + $this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [ + 'username' => 'test', + 'first_name' => 'Test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('success'); + $asset->checkOut($user, $superUser); // asset assigned, therefore error @@ -372,6 +379,13 @@ class UpdateUserTest extends TestCase 'username' => 'test', 'company_id' => $companyB->id, ])->assertStatusMessageIs('error'); + + // same test but PUT + $this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [ + 'username' => 'test', + 'first_name' => 'Test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('error'); } } diff --git a/tests/Feature/Users/Ui/UpdateUserTest.php b/tests/Feature/Users/Ui/UpdateUserTest.php index 31530af7c..3e6867021 100644 --- a/tests/Feature/Users/Ui/UpdateUserTest.php +++ b/tests/Feature/Users/Ui/UpdateUserTest.php @@ -98,17 +98,22 @@ class UpdateUserTest extends TestCase // no assets assigned, therefore success $this->actingAs($superUser)->put(route('users.update', $user), [ - 'first_name' => 'test', - 'username' => 'test', - 'company_id' => $companyB->id, + 'first_name' => 'test', + 'username' => 'test', + 'company_id' => $companyB->id, + 'redirect_option' => 'index' ])->assertRedirect(route('users.index')); - //$asset->checkOut($user, $superUser); + $asset->checkOut($user, $superUser); // asset assigned, therefore error - //$this->actingAs($superUser)->patchJson(route('users.update', $user), [ - // 'username' => 'test', - // 'company_id' => $companyB->id, - //])->assertMessagesAre('error'); + $response = $this->actingAs($superUser)->patchJson(route('users.update', $user), [ + 'first_name' => 'test', + 'username' => 'test', + 'company_id' => $companyB->id, + 'redirect_option' => 'index' + ]); + + $this->followRedirects($response)->assertSee('error'); } } From 70e5e0f9df91c10d94568c98168be1f5fdc5686a Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Fri, 16 Aug 2024 12:52:06 -0500 Subject: [PATCH 11/12] get rid of dd --- app/Http/Requests/SaveUserRequest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Requests/SaveUserRequest.php b/app/Http/Requests/SaveUserRequest.php index 7598e7433..e16826d16 100644 --- a/app/Http/Requests/SaveUserRequest.php +++ b/app/Http/Requests/SaveUserRequest.php @@ -31,7 +31,6 @@ class SaveUserRequest extends FormRequest */ public function rules() { - //dd($this->user); $rules = [ 'department_id' => 'nullable|exists:departments,id', 'manager_id' => 'nullable|exists:users,id', From a8cd1027f3cc1716b76461f9db7dbc17b8a130b4 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 20 Aug 2024 11:40:15 -0500 Subject: [PATCH 12/12] rm commented code --- app/Http/Controllers/Users/UsersController.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index e609727c0..3da0c5a3a 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -234,12 +234,7 @@ class UsersController extends Controller $this->authorize('update', $user); - //see if i can get this working at request level - //if ($request->has('company_id') && $user->allAssignedCount() > 0 && Setting::getSettings()->full_multiple_companies_support) { - // return back()->with('error', trans('admin/users/message.multi_company_items_assigned')); - //} - - // Figure out of this user was an admin before this edit + // Figure out of this user was an admin before this edit $orig_permissions_array = $user->decodePermissions(); $orig_superuser = '0'; if (is_array($orig_permissions_array)) {