Refactored existing user API work

- Updated routes to use new format.
- Changed how hidden fields are exposed to be more flexible to different
  use-cases.
- Updated properties available on read/list results.
- Started adding testing coverage.
- Removed old unused UserRepo 'getAllUsers' function.

Related to #2701, Progression of #2734

app/Api/ListingResponseBuilder.php

@@ -2,8 +2,10 @@
 
 namespace BookStack\Api;
 
+use BookStack\Model;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 
 class ListingResponseBuilder
@@ -11,7 +13,11 @@ class ListingResponseBuilder
     protected $query;
     protected $request;
     protected $fields;
-    protected $hiddenFields;
+
+    /**
+     * @var array<callable>
+     */
+    protected $resultModifiers = [];
 
     protected $filterOperators = [
         'eq'   => '=',
@@ -25,25 +31,28 @@ class ListingResponseBuilder
 
     /**
      * ListingResponseBuilder constructor.
+     * The given fields will be forced visible within the model results.
      */
-    public function __construct(Builder $query, Request $request, array $fields, array $hiddenFields )
+    public function __construct(Builder $query, Request $request, array $fields)
     {
         $this->query = $query;
         $this->request = $request;
         $this->fields = $fields;
-        $this->hiddenFields = $hiddenFields;
     }
 
     /**
      * Get the response from this builder.
      */
-    public function toResponse()
+    public function toResponse(): JsonResponse
     {
         $filteredQuery = $this->filterQuery($this->query);
 
         $total = $filteredQuery->count();
-        $data = $this->fetchData($filteredQuery);
+        $data = $this->fetchData($filteredQuery)->each(function($model) {
-        $data = $data->makeVisible($this->hiddenFields);
+            foreach ($this->resultModifiers as $modifier) {
+                $modifier($model);
+            }
+        });
 
         return response()->json([
             'data'  => $data,
@@ -52,7 +61,16 @@ class ListingResponseBuilder
     }
 
     /**
-     * Fetch the data to return in the response.
+     * Add a callback to modify each element of the results
+     * @param (callable(Model)) $modifier
+     */
+    public function modifyResults($modifier): void
+    {
+        $this->resultModifiers[] = $modifier;
+    }
+
+    /**
+     * Fetch the data to return within the response.
      */
     protected function fetchData(Builder $query): Collection
     {

app/Auth/Role.php

@@ -28,6 +28,8 @@ class Role extends Model implements Loggable
 
     protected $fillable = ['display_name', 'description', 'external_auth_id'];
 
+    protected $hidden = ['pivot'];
+
     /**
      * The roles that belong to the role.
      */

app/Auth/User.php

@@ -72,7 +72,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      */
     protected $hidden = [
         'password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email',
-        'created_at', 'updated_at', 'image_id',
+        'created_at', 'updated_at', 'image_id', 'roles', 'avatar',
     ];
 
     /**

app/Auth/UserRepo.php

@@ -52,23 +52,14 @@ class UserRepo
         return User::query()->where('slug', '=', $slug)->firstOrFail();
     }
 
-    /**
-     * Get all the users with their permissions.
-     */
-    public function getAllUsers(): Collection
-    {
-        return User::query()->with('roles', 'avatar')->orderBy('name', 'asc')->get();
-    }
-
     /**
      * Get all users as Builder for API
      */
-    public function getUsersBuilder(int $id = null ) : Builder
+    public function getApiUsersBuilder() : Builder
     {
-        $query = User::query()->select(['*'])
+        return User::query()->select(['*'])
-            ->withLastActivityAt()
+            ->scopes('withLastActivityAt')
-            ->with(['roles', 'avatar']);
+            ->with(['avatar']);
-        return $query;
     }
     /**
      * Get all the users with their permissions in a paginated format.

app/Http/Controllers/Api/ApiController.php

@@ -10,15 +10,19 @@ use Illuminate\Http\JsonResponse;
 abstract class ApiController extends Controller
 {
     protected $rules = [];
-    protected $printHidden = [];
+    protected $fieldsToExpose = [];
 
     /**
      * Provide a paginated listing JSON response in a standard format
      * taking into account any pagination parameters passed by the user.
      */
-    protected function apiListingResponse(Builder $query, array $fields, array $protectedFieldsToPrint = []): JsonResponse
+    protected function apiListingResponse(Builder $query, array $fields, array $modifiers = []): JsonResponse
     {
-        $listing = new ListingResponseBuilder($query, request(), $fields, $protectedFieldsToPrint);
+        $listing = new ListingResponseBuilder($query, request(), $fields);
+
+        foreach ($modifiers as $modifier) {
+            $listing->modifyResults($modifier);
+        }
 
         return $listing->toResponse();
     }

app/Http/Controllers/Api/UserApiController.php

@@ -2,60 +2,78 @@
 
 namespace BookStack\Http\Controllers\Api;
 
-use BookStack\Exceptions\PermissionsException;
 use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
-use Exception;
+use Closure;
-use Illuminate\Http\Request;
 
 class UserApiController extends ApiController
 {
-    protected $user;
     protected $userRepo;
 
-    protected $printHidden = [
+    protected $fieldsToExpose = [
-        'email', 'created_at', 'updated_at', 'last_activity_at'
+        'email', 'created_at', 'updated_at', 'last_activity_at', 'external_auth_id'
     ];
 
-# TBD: Endpoints to create / update users
+    protected $rules = [
-#     protected $rules = [
+        'create' => [
-#         'create' => [
+        ],
-#         ],
+        'update' => [
-#         'update' => [
+        ],
-#         ],
+    ];
-#     ];
 
-    public function __construct(User $user, UserRepo $userRepo)
+    public function __construct(UserRepo $userRepo)
     {
-        $this->user = $user;
         $this->userRepo = $userRepo;
     }
 
     /**
-     * Get a listing of users
+     * Get a listing of users in the system.
+     * Requires permission to manage users.
      */
     public function list()
     {
         $this->checkPermission('users-manage');
 
-        $users = $this->userRepo->getUsersBuilder();
+        $users = $this->userRepo->getApiUsersBuilder();
 
         return $this->apiListingResponse($users, [
-            'id', 'name', 'slug', 'email',
+            'id', 'name', 'slug', 'email', 'external_auth_id',
             'created_at', 'updated_at', 'last_activity_at',
-        ], $this->printHidden);
+        ], [Closure::fromCallable([$this, 'listFormatter'])]);
     }
 
     /**
-     * View the details of a single user
+     * View the details of a single user.
+     * Requires permission to manage users.
      */
     public function read(string $id)
     {
         $this->checkPermission('users-manage');
 
         $singleUser = $this->userRepo->getById($id);
-        $singleUser = $singleUser->makeVisible($this->printHidden);
+        $this->singleFormatter($singleUser);
 
         return response()->json($singleUser);
     }
+
+    /**
+     * Format the given user model for single-result display.
+     */
+    protected function singleFormatter(User $user)
+    {
+        $this->listFormatter($user);
+        $user->load('roles:id,display_name');
+        $user->makeVisible(['roles']);
+    }
+
+    /**
+     * Format the given user model for a listing multi-result display.
+     */
+    protected function listFormatter(User $user)
+    {
+        $user->makeVisible($this->fieldsToExpose);
+        $user->setAttribute('profile_url', $user->getProfileUrl());
+        $user->setAttribute('edit_url', $user->getEditUrl());
+        $user->setAttribute('avatar_url', $user->getAvatar());
+    }
 }

routes/api.php

@@ -10,6 +10,7 @@ use BookStack\Http\Controllers\Api\ChapterExportApiController;
 use BookStack\Http\Controllers\Api\PageApiController;
 use BookStack\Http\Controllers\Api\PageExportApiController;
 use BookStack\Http\Controllers\Api\SearchApiController;
+use BookStack\Http\Controllers\Api\UserApiController;
 use Illuminate\Support\Facades\Route;
 
 /**
@@ -66,5 +67,5 @@ Route::get('shelves/{id}', [BookshelfApiController::class, 'read']);
 Route::put('shelves/{id}', [BookshelfApiController::class, 'update']);
 Route::delete('shelves/{id}', [BookshelfApiController::class, 'delete']);
 
-Route::get('users', 'UserApiController@list');
+Route::get('users', [UserApiController::class, 'list']);
-Route::get('users/{id}', 'UserApiController@read');
+Route::get('users/{id}', [UserApiController::class, 'read']);

tests/Api/UsersApiTest.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace Tests\Api;
+
+use BookStack\Auth\Role;
+use BookStack\Auth\User;
+use Tests\TestCase;
+
+class UsersApiTest extends TestCase
+{
+    use TestsApi;
+
+    protected $baseEndpoint = '/api/users';
+
+    public function test_users_manage_permission_needed_for_all_endpoints()
+    {
+        // TODO
+    }
+
+    public function test_index_endpoint_returns_expected_shelf()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $firstUser */
+        $firstUser = User::query()->orderBy('id', 'asc')->first();
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJson(['data' => [
+            [
+                'id'   => $firstUser->id,
+                'name' => $firstUser->name,
+                'slug' => $firstUser->slug,
+                'email' => $firstUser->email,
+                'profile_url' => $firstUser->getProfileUrl(),
+                'edit_url' => $firstUser->getEditUrl(),
+                'avatar_url' => $firstUser->getAvatar(),
+            ],
+        ]]);
+    }
+
+    public function test_read_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var User $user */
+        $user = User::query()->first();
+        /** @var Role $userRole */
+        $userRole = $user->roles()->first();
+
+        $resp = $this->getJson($this->baseEndpoint . "/{$user->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id'         => $user->id,
+            'slug'       => $user->slug,
+            'email'      => $user->email,
+            'external_auth_id' => $user->external_auth_id,
+            'roles' => [
+                [
+                    'id' => $userRole->id,
+                    'display_name' => $userRole->display_name,
+                ]
+            ],
+        ]);
+    }
+}