[Multi-Company] Fixes Users Being Moved With Items Still Assigned #15284
Conversation
PR Summary
|
| @@ -427,13 +427,10 @@ public function show($id) : JsonResponse | array | |||
| * @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); | |||
There was a problem hiding this comment.
I'm assuming this can go away now, and was there to first check that a user could update users in general before doing the check to find the user? but wanted to double check.
|
Hm....tests failed on MySQL and PHP 8.3... Another PR failed on the same or similar error recently. Kinda odd. I just re-triggered it and it went green. |
|
Yeah, that seems to happen sometimes. 🤷♀️ |
Thanks to the article you sent! |
|
After merging this, I'm getting a gateway timeout when trying to create a new user. We're looking into it. |
| }); | ||
| }); | ||
|
|
||
| Route::match(['put', 'patch'], '{user}/update', [Api\UsersController::class, 'update']) |
There was a problem hiding this comment.
This is causing a method not found error. #15435
Description
Adds a check to determine whether a user has any items assigned before being moved between companies. The logic happens in SaveUserRequest - so the logic works in both the UI and API. Route model binding is also now present for these routes - added to get the handy
$this->userin the request, as well as remove some lines in the controllers.One shortcoming - if the user has un-reassignable licenses, then it gets difficult to move the user, because you'd need to make the license reassignable before checking it in. We can talk about that next time we chat.
Fixes #15265
Type of change
How Has This Been Tested?
Tests written.
Test Configuration: