Better validation for relations on delete

[snipe]

Still working through some failing tests on this, but wanted to put it up to see if we even like this approach.

 • Tests\Feature\Users\Ui\DeleteUserTest > disallow user deletion if still managing people
  Failed asserting that two strings are equal.

  The following errors occurred during the last request:

  This user still manages 3 users. Please select another manager for them first.

  at tests/Feature/Users/Ui/DeleteUserTest.php:23
     19▕
     20▕         $response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
     21▕             ->delete(route('users.destroy', $manager->id))
     22▕             ->assertStatus(302)
  ➜  23▕             ->assertRedirect(route('users.index'));
     24▕
     25▕         $this->followRedirects($response)->assertSee('Error');
     26▕     }
     27▕
  --- Expected
  +++ Actual
  @@ @@
  -'https://snipe-it.local:8890/users'
  +'https://snipe-it.local:8890'
  • Tests\Feature\Users\Ui\DeleteUserTest > disallow user deletion if still managing locations
  Failed asserting that two strings are equal.

  The following errors occurred during the last request:

  This user still manages 3 locations. Please select another manager first.

  at tests/Feature/Users/Ui/DeleteUserTest.php:38
     34▕
     35▕         $response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
     36▕             ->delete(route('users.destroy', $manager->id))
     37▕             ->assertStatus(302)
  ➜  38▕             ->assertRedirect(route('users.index'));
     39▕
     40▕            $this->followRedirects($response)->assertSee('Error');
     41▕     }
     42▕
  --- Expected
  +++ Actual
  @@ @@
  -'https://snipe-it.local:8890/users'
  +'https://snipe-it.local:8890'

  Tests:  2 failed, 3 incomplete, 314 passed
  Time:   136.72s

The failure seems to be on the GUI redirect, where it seems to be redirecting back to the dashboard for some reason, even though I don't see where we'd be doing that.

On the API side, the response now looks like this:

{
    "status": "error",
    "messages": {
        "user": [
            "We would feel really bad if you deleted yourself, please reconsider."
        ],
        "managed_users": [
            "This user still manages 3 users. Please select another manager for them first."
        ],
        "assigned_assets": [
            "This user still has 5 assets assigned. Please check their assets in first."
        ],
        "assigned_licenses": [
            "This user still has a license seats assigned. Please check it in first."
        ],
        "assigned_accessories": [
            "This user still has 2 accessories assigned. Please check their assets in first."
        ]
    },
    "payload": null
}

[what-the-diff]

PR Summary

• Addition and Update in API Users Controller
  This modification refers to the incorporation of the DeleteUserRequest in the Api/UsersController.php file, and a subsequent update to the destroy method in the same file to include the newly added DeleteUserRequest $request parameter. This will enhance the function's consistency and clarity, leading to a more efficient user deletion process.

• Streamlining API Users Controller
  A simplification of the codebase was achieved by removing unwarranted conditions in the destroy method in the Api/UsersController.php file. This should result in a clearer understanding of the code and potentially improved performance.

• Addition and Update in Users Controller
  In the Users/UsersController.php file, we've similarly introduced the DeleteUserRequest. An update to the destroy method in the same file was made to include DeleteUserRequest $request parameter. This mirrors the updates made in the API Users Controller, ensuring that whether the API or website interface is used, deletion functionality remains consistent.

• New File Creation and Content Addition
  A new file called DeleteUserRequest.php was created in the app/Http/Requests directory. This file comes with a specific set of rules for request authorization and defines validation rules and messages, improving error messaging and reducing the potential for unintended deletion outcomes.

[uberbrady]

This looks great, and I love the DeleteUserRequest FormRequest thingee - it starts to shrink our controllers down to something more manageable, and I also love being able to re-use the same request between the API and GUI controllers. Very slick work! I did mention that, if you wanted, you could consider moving the ->authorize() calls up into the FormRequest if you wanted to - but that's not a dealbreaker for me at all. Thanks for doing this!

[uberbrady]

If you wanted to, you could move this logic to the DeleteUserRequest.

[snipe]

I did mention that, if you wanted, you could consider moving the ->authorize() calls up into the FormRequest if you wanted to - but that's not a dealbreaker for me at all. Thanks for doing this!

We don't do that anywhere else, and it seemed too big a leap for this PR. We'd need to extensively test anything that touches permissions.

[snipe]

I'm a little puzzled why the tests are failing here. When I test this directly in the browser, it behaves as expected, redirecting me back to the users page. I'm sure it's something dumb, but...