diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php index 76157c9a5..671923ab1 100644 --- a/app/Api/ApiDocsGenerator.php +++ b/app/Api/ApiDocsGenerator.php @@ -102,8 +102,8 @@ class ApiDocsGenerator $this->controllerClasses[$className] = $class; } - $rules = collect($class->getValidationRules()[$methodName] ?? [])->map(function($validations) { - return array_map(function($validation) { + $rules = collect($class->getValidationRules()[$methodName] ?? [])->map(function ($validations) { + return array_map(function ($validation) { return $this->getValidationAsString($validation); }, $validations); })->toArray(); @@ -129,6 +129,7 @@ class ApiDocsGenerator } $class = get_class($validation); + throw new Exception("Cannot provide string representation of rule for class: {$class}"); } diff --git a/app/Api/ListingResponseBuilder.php b/app/Api/ListingResponseBuilder.php index 6da92040b..7de5ddf07 100644 --- a/app/Api/ListingResponseBuilder.php +++ b/app/Api/ListingResponseBuilder.php @@ -48,7 +48,7 @@ class ListingResponseBuilder $filteredQuery = $this->filterQuery($this->query); $total = $filteredQuery->count(); - $data = $this->fetchData($filteredQuery)->each(function($model) { + $data = $this->fetchData($filteredQuery)->each(function ($model) { foreach ($this->resultModifiers as $modifier) { $modifier($model); } @@ -61,7 +61,8 @@ class ListingResponseBuilder } /** - * Add a callback to modify each element of the results + * Add a callback to modify each element of the results. + * * @param (callable(Model)) $modifier */ public function modifyResults($modifier): void diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index f9cfc078e..606fd5d65 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -60,7 +60,7 @@ class UserRepo } /** - * Get all users as Builder for API + * Get all users as Builder for API. */ public function getApiUsersBuilder(): Builder { @@ -159,6 +159,7 @@ class UserRepo /** * Create a new basic instance of user with the given pre-validated data. + * * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data */ public function createWithoutActivity(array $data, bool $emailConfirmed = false): User @@ -188,6 +189,7 @@ class UserRepo /** * As per "createWithoutActivity" but records a "create" activity. + * * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data */ public function create(array $data, bool $sendInvite = false): User @@ -199,12 +201,15 @@ class UserRepo } Activity::add(ActivityType::USER_CREATE, $user); + return $user; } /** * Update the given user with the given data. + * * @param array{name: ?string, email: ?string, external_auth_id: ?string, password: ?string, roles: ?array, language: ?string} $data + * * @throws UserUpdateException */ public function update(User $user, array $data, bool $manageUsersAllowed): User @@ -307,10 +312,10 @@ class UserRepo }; return [ - 'pages' => $query(Page::visible()->where('draft', '=', false)), + 'pages' => $query(Page::visible()->where('draft', '=', false)), 'chapters' => $query(Chapter::visible()), - 'books' => $query(Book::visible()), - 'shelves' => $query(Bookshelf::visible()), + 'books' => $query(Book::visible()), + 'shelves' => $query(Bookshelf::visible()), ]; } @@ -322,10 +327,10 @@ class UserRepo $createdBy = ['created_by' => $user->id]; return [ - 'pages' => Page::visible()->where($createdBy)->count(), + 'pages' => Page::visible()->where($createdBy)->count(), 'chapters' => Chapter::visible()->where($createdBy)->count(), - 'books' => Book::visible()->where($createdBy)->count(), - 'shelves' => Bookshelf::visible()->where($createdBy)->count(), + 'books' => Book::visible()->where($createdBy)->count(), + 'shelves' => Bookshelf::visible()->where($createdBy)->count(), ]; } diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index aa2a2481c..d58904938 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -16,7 +16,7 @@ class UserApiController extends ApiController protected $userRepo; protected $fieldsToExpose = [ - 'email', 'created_at', 'updated_at', 'last_activity_at', 'external_auth_id' + 'email', 'created_at', 'updated_at', 'last_activity_at', 'external_auth_id', ]; public function __construct(UserRepo $userRepo) @@ -27,6 +27,7 @@ class UserApiController extends ApiController $this->middleware(function ($request, $next) { $this->checkPermission('users-manage'); $this->preventAccessInDemoMode(); + return $next($request); }); } @@ -35,29 +36,29 @@ class UserApiController extends ApiController { return [ 'create' => [ - 'name' => ['required', 'min:2'], + 'name' => ['required', 'min:2'], 'email' => [ - 'required', 'min:2', 'email', new Unique('users', 'email') + 'required', 'min:2', 'email', new Unique('users', 'email'), ], 'external_auth_id' => ['string'], - 'language' => ['string'], - 'password' => [Password::default()], - 'roles' => ['array'], - 'roles.*' => ['integer'], - 'send_invite' => ['boolean'], + 'language' => ['string'], + 'password' => [Password::default()], + 'roles' => ['array'], + 'roles.*' => ['integer'], + 'send_invite' => ['boolean'], ], 'update' => [ - 'name' => ['min:2'], + 'name' => ['min:2'], 'email' => [ 'min:2', 'email', - (new Unique('users', 'email'))->ignore($userId ?? null) + (new Unique('users', 'email'))->ignore($userId ?? null), ], 'external_auth_id' => ['string'], - 'language' => ['string'], - 'password' => [Password::default()], - 'roles' => ['array'], - 'roles.*' => ['integer'], + 'language' => ['string'], + 'password' => [Password::default()], + 'roles' => ['array'], + 'roles.*' => ['integer'], ], 'delete' => [ 'migrate_ownership_id' => ['integer', 'exists:users,id'], @@ -113,6 +114,7 @@ class UserApiController extends ApiController /** * Update an existing user in the system. * Requires permission to manage users. + * * @throws UserUpdateException */ public function update(Request $request, string $id) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 13a86f6f7..d616974c6 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -54,6 +54,7 @@ abstract class Controller extends BaseController protected function showPermissionError() { $message = request()->wantsJson() ? trans('errors.permissionJson') : trans('errors.permission'); + throw new NotifyException($message, '/', 403); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 3b443aa81..a635bbaa6 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -75,12 +75,12 @@ class UserController extends Controller $passwordRequired = ($authMethod === 'standard' && !$sendInvite); $validationRules = [ - 'name' => ['required'], - 'email' => ['required', 'email', 'unique:users,email'], - 'language' => ['string'], + 'name' => ['required'], + 'email' => ['required', 'email', 'unique:users,email'], + 'language' => ['string'], 'roles' => ['array'], 'roles.*' => ['integer'], - 'password' => $passwordRequired ? ['required', Password::default()] : null, + 'password' => $passwordRequired ? ['required', Password::default()] : null, 'password-confirm' => $passwordRequired ? ['required', 'same:password'] : null, 'external_auth_id' => $externalAuth ? ['required'] : null, ]; diff --git a/routes/api.php b/routes/api.php index c7b8887b6..a87169ee5 100644 --- a/routes/api.php +++ b/routes/api.php @@ -71,4 +71,4 @@ Route::get('users', [UserApiController::class, 'list']); Route::post('users', [UserApiController::class, 'create']); Route::get('users/{id}', [UserApiController::class, 'read']); Route::put('users/{id}', [UserApiController::class, 'update']); -Route::delete('users/{id}', [UserApiController::class, 'delete']); \ No newline at end of file +Route::delete('users/{id}', [UserApiController::class, 'delete']); diff --git a/tests/Api/UsersApiTest.php b/tests/Api/UsersApiTest.php index e1bcb02d5..ddbdac0f8 100644 --- a/tests/Api/UsersApiTest.php +++ b/tests/Api/UsersApiTest.php @@ -56,13 +56,13 @@ class UsersApiTest extends TestCase $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id'); $resp->assertJson(['data' => [ [ - 'id' => $firstUser->id, - 'name' => $firstUser->name, - 'slug' => $firstUser->slug, - 'email' => $firstUser->email, + 'id' => $firstUser->id, + 'name' => $firstUser->name, + 'slug' => $firstUser->slug, + 'email' => $firstUser->email, 'profile_url' => $firstUser->getProfileUrl(), - 'edit_url' => $firstUser->getEditUrl(), - 'avatar_url' => $firstUser->getAvatar(), + 'edit_url' => $firstUser->getEditUrl(), + 'avatar_url' => $firstUser->getAvatar(), ], ]]); } @@ -74,24 +74,24 @@ class UsersApiTest extends TestCase $role = Role::query()->first(); $resp = $this->postJson($this->baseEndpoint, [ - 'name' => 'Benny Boris', - 'email' => 'bboris@example.com', - 'password' => 'mysuperpass', - 'language' => 'it', - 'roles' => [$role->id], + 'name' => 'Benny Boris', + 'email' => 'bboris@example.com', + 'password' => 'mysuperpass', + 'language' => 'it', + 'roles' => [$role->id], 'send_invite' => false, ]); $resp->assertStatus(200); $resp->assertJson([ - 'name' => 'Benny Boris', - 'email' => 'bboris@example.com', + 'name' => 'Benny Boris', + 'email' => 'bboris@example.com', 'external_auth_id' => '', - 'roles' => [ + 'roles' => [ [ - 'id' => $role->id, + 'id' => $role->id, 'display_name' => $role->display_name, - ] + ], ], ]); $this->assertDatabaseHas('users', ['email' => 'bboris@example.com']); @@ -109,8 +109,8 @@ class UsersApiTest extends TestCase Notification::fake(); $resp = $this->postJson($this->baseEndpoint, [ - 'name' => 'Benny Boris', - 'email' => 'bboris@example.com', + 'name' => 'Benny Boris', + 'email' => 'bboris@example.com', 'send_invite' => true, ]); @@ -140,7 +140,7 @@ class UsersApiTest extends TestCase $resp = $this->postJson($this->baseEndpoint, [ 'email' => $existingUser->email, - 'name' => 'Benny Boris', + 'name' => 'Benny Boris', ]); $resp->assertStatus(422); $resp->assertJson($this->validationResponse(['email' => ['The email has already been taken.']])); @@ -158,15 +158,15 @@ class UsersApiTest extends TestCase $resp->assertStatus(200); $resp->assertJson([ - 'id' => $user->id, - 'slug' => $user->slug, - 'email' => $user->email, + 'id' => $user->id, + 'slug' => $user->slug, + 'email' => $user->email, 'external_auth_id' => $user->external_auth_id, - 'roles' => [ + 'roles' => [ [ - 'id' => $userRole->id, + 'id' => $userRole->id, 'display_name' => $userRole->display_name, - ] + ], ], ]); } @@ -178,19 +178,19 @@ class UsersApiTest extends TestCase $user = $this->getAdmin(); $roles = Role::query()->pluck('id'); $resp = $this->putJson($this->baseEndpoint . "/{$user->id}", [ - 'name' => 'My updated user', - 'email' => 'barrytest@example.com', - 'roles' => $roles, + 'name' => 'My updated user', + 'email' => 'barrytest@example.com', + 'roles' => $roles, 'external_auth_id' => 'btest', - 'password' => 'barrytester', - 'language' => 'fr', + 'password' => 'barrytester', + 'language' => 'fr', ]); $resp->assertStatus(200); $resp->assertJson([ - 'id' => $user->id, - 'name' => 'My updated user', - 'email' => 'barrytest@example.com', + 'id' => $user->id, + 'name' => 'My updated user', + 'email' => 'barrytest@example.com', 'external_auth_id' => 'btest', ]); $user->refresh(); @@ -210,9 +210,9 @@ class UsersApiTest extends TestCase $resp->assertStatus(200); $this->assertDatabaseHas('users', [ - 'id' => $user->id, - 'name' => $user->name, - 'email' => $user->email, + 'id' => $user->id, + 'name' => $user->name, + 'email' => $user->email, 'password' => $user->password, ]); $this->assertEquals($roleCount, $user->roles()->count()); diff --git a/tests/HelpTest.php b/tests/HelpTest.php index 2e08abfbc..bf883bd39 100644 --- a/tests/HelpTest.php +++ b/tests/HelpTest.php @@ -4,7 +4,6 @@ namespace Tests; class HelpTest extends TestCase { - public function test_wysiwyg_help_shows_tiny_and_tiny_license_link() { $resp = $this->get('/help/wysiwyg'); @@ -21,5 +20,4 @@ class HelpTest extends TestCase $contents = file_get_contents($expectedPath); $this->assertStringContainsString('GNU LESSER GENERAL PUBLIC LICENSE', $contents); } - }