Users: Improved user response for failed invite sending

Added specific handling to show relevant error message when user
creation fails due to invite sending errors, while also returning user
to the form with previous input.
Includes test to cover.

For #5195
This commit is contained in:
Dan Brown 2024-09-29 16:41:18 +01:00
parent 89f84c9a95
commit d12e8ec923
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
6 changed files with 57 additions and 8 deletions

View File

@ -0,0 +1,10 @@
<?php
namespace BookStack\Access;
use Exception;
class UserInviteException extends Exception
{
//
}

View File

@ -13,11 +13,17 @@ class UserInviteService extends UserTokenService
/** /**
* Send an invitation to a user to sign into BookStack * Send an invitation to a user to sign into BookStack
* Removes existing invitation tokens. * Removes existing invitation tokens.
* @throws UserInviteException
*/ */
public function sendInvitation(User $user) public function sendInvitation(User $user)
{ {
$this->deleteByUser($user); $this->deleteByUser($user);
$token = $this->createTokenForUser($user); $token = $this->createTokenForUser($user);
try {
$user->notify(new UserInviteNotification($token)); $user->notify(new UserInviteNotification($token));
} catch (\Exception $exception) {
throw new UserInviteException($exception->getMessage(), $exception->getCode(), $exception);
}
} }
} }

View File

@ -3,6 +3,7 @@
namespace BookStack\Users\Controllers; namespace BookStack\Users\Controllers;
use BookStack\Access\SocialDriverManager; use BookStack\Access\SocialDriverManager;
use BookStack\Access\UserInviteException;
use BookStack\Exceptions\ImageUploadException; use BookStack\Exceptions\ImageUploadException;
use BookStack\Exceptions\UserUpdateException; use BookStack\Exceptions\UserUpdateException;
use BookStack\Http\Controller; use BookStack\Http\Controller;
@ -14,6 +15,7 @@ use BookStack\Util\SimpleListOptions;
use Exception; use Exception;
use Illuminate\Http\Request; use Illuminate\Http\Request;
use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
use Illuminate\Validation\Rules\Password; use Illuminate\Validation\Rules\Password;
use Illuminate\Validation\ValidationException; use Illuminate\Validation\ValidationException;
@ -91,9 +93,16 @@ class UserController extends Controller
$validated = $this->validate($request, array_filter($validationRules)); $validated = $this->validate($request, array_filter($validationRules));
try {
DB::transaction(function () use ($validated, $sendInvite) { DB::transaction(function () use ($validated, $sendInvite) {
$this->userRepo->create($validated, $sendInvite); $this->userRepo->create($validated, $sendInvite);
dd('post-create');
}); });
} catch (UserInviteException $e) {
Log::error("Failed to send user invite with error: {$e->getMessage()}");
$this->showErrorNotification(trans('errors.users_could_not_send_invite'));
return redirect('/settings/users/create')->withInput();
}
return redirect('/settings/users'); return redirect('/settings/users');
} }

View File

@ -2,6 +2,7 @@
namespace BookStack\Users; namespace BookStack\Users;
use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService; use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType; use BookStack\Activity\ActivityType;
use BookStack\Entities\EntityProvider; use BookStack\Entities\EntityProvider;
@ -83,6 +84,7 @@ class UserRepo
* As per "createWithoutActivity" but records a "create" activity. * 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 * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
* @throws UserInviteException
*/ */
public function create(array $data, bool $sendInvite = false): User public function create(array $data, bool $sendInvite = false): User
{ {

View File

@ -78,6 +78,7 @@ return [
// Users // Users
'users_cannot_delete_only_admin' => 'You cannot delete the only admin', 'users_cannot_delete_only_admin' => 'You cannot delete the only admin',
'users_cannot_delete_guest' => 'You cannot delete the guest user', 'users_cannot_delete_guest' => 'You cannot delete the guest user',
'users_could_not_send_invite' => 'Could not create user since invite email failed to send',
// Roles // Roles
'role_cannot_be_edited' => 'This role cannot be edited', 'role_cannot_be_edited' => 'This role cannot be edited',

View File

@ -2,6 +2,7 @@
namespace Tests\User; namespace Tests\User;
use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService; use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType; use BookStack\Activity\ActivityType;
use BookStack\Uploads\Image; use BookStack\Uploads\Image;
@ -229,7 +230,7 @@ class UserManagementTest extends TestCase
// Simulate an invitation sending failure // Simulate an invitation sending failure
$this->mock(UserInviteService::class, function (MockInterface $mock) { $this->mock(UserInviteService::class, function (MockInterface $mock) {
$mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class); $mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
}); });
$this->asAdmin()->post('/settings/users/create', [ $this->asAdmin()->post('/settings/users/create', [
@ -247,22 +248,42 @@ class UserManagementTest extends TestCase
{ {
/** @var User $user */ /** @var User $user */
$user = User::factory()->make(); $user = User::factory()->make();
$adminRole = Role::getRole('admin');
$this->mock(UserInviteService::class, function (MockInterface $mock) { $this->mock(UserInviteService::class, function (MockInterface $mock) {
$mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class); $mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
}); });
$this->asAdmin()->post('/settings/users/create', [ $this->asAdmin()->post('/settings/users/create', [
'name' => $user->name, 'name' => $user->name,
'email' => $user->email, 'email' => $user->email,
'send_invite' => 'true', 'send_invite' => 'true',
'roles[' . $adminRole->id . ']' => 'true',
]); ]);
$this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']); $this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']);
} }
public function test_return_to_form_with_warning_if_the_invitation_sending_fails()
{
$logger = $this->withTestLogger();
/** @var User $user */
$user = User::factory()->make();
$this->mock(UserInviteService::class, function (MockInterface $mock) {
$mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
});
$resp = $this->asAdmin()->post('/settings/users/create', [
'name' => $user->name,
'email' => $user->email,
'send_invite' => 'true',
]);
$resp->assertRedirect('/settings/users/create');
$this->assertSessionError('Could not create user since invite email failed to send');
$this->assertEquals($user->email, session()->getOldInput('email'));
$this->assertTrue($logger->hasErrorThatContains('Failed to send user invite with error:'));
}
public function test_user_create_update_fails_if_locale_is_invalid() public function test_user_create_update_fails_if_locale_is_invalid()
{ {
$user = $this->users->editor(); $user = $this->users->editor();