Skip to content

Commit d12e8ec

Browse files
committed
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
1 parent 89f84c9 commit d12e8ec

File tree

6 files changed

+57
-8
lines changed

6 files changed

+57
-8
lines changed

app/Access/UserInviteException.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
namespace BookStack\Access;
4+
5+
use Exception;
6+
7+
class UserInviteException extends Exception
8+
{
9+
//
10+
}

app/Access/UserInviteService.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@ class UserInviteService extends UserTokenService
1313
/**
1414
* Send an invitation to a user to sign into BookStack
1515
* Removes existing invitation tokens.
16+
* @throws UserInviteException
1617
*/
1718
public function sendInvitation(User $user)
1819
{
1920
$this->deleteByUser($user);
2021
$token = $this->createTokenForUser($user);
21-
$user->notify(new UserInviteNotification($token));
22+
23+
try {
24+
$user->notify(new UserInviteNotification($token));
25+
} catch (\Exception $exception) {
26+
throw new UserInviteException($exception->getMessage(), $exception->getCode(), $exception);
27+
}
2228
}
2329
}

app/Users/Controllers/UserController.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace BookStack\Users\Controllers;
44

55
use BookStack\Access\SocialDriverManager;
6+
use BookStack\Access\UserInviteException;
67
use BookStack\Exceptions\ImageUploadException;
78
use BookStack\Exceptions\UserUpdateException;
89
use BookStack\Http\Controller;
@@ -14,6 +15,7 @@
1415
use Exception;
1516
use Illuminate\Http\Request;
1617
use Illuminate\Support\Facades\DB;
18+
use Illuminate\Support\Facades\Log;
1719
use Illuminate\Validation\Rules\Password;
1820
use Illuminate\Validation\ValidationException;
1921

@@ -91,9 +93,16 @@ public function store(Request $request)
9193

9294
$validated = $this->validate($request, array_filter($validationRules));
9395

94-
DB::transaction(function () use ($validated, $sendInvite) {
95-
$this->userRepo->create($validated, $sendInvite);
96-
});
96+
try {
97+
DB::transaction(function () use ($validated, $sendInvite) {
98+
$this->userRepo->create($validated, $sendInvite);
99+
dd('post-create');
100+
});
101+
} catch (UserInviteException $e) {
102+
Log::error("Failed to send user invite with error: {$e->getMessage()}");
103+
$this->showErrorNotification(trans('errors.users_could_not_send_invite'));
104+
return redirect('/settings/users/create')->withInput();
105+
}
97106

98107
return redirect('/settings/users');
99108
}

app/Users/UserRepo.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace BookStack\Users;
44

5+
use BookStack\Access\UserInviteException;
56
use BookStack\Access\UserInviteService;
67
use BookStack\Activity\ActivityType;
78
use BookStack\Entities\EntityProvider;
@@ -83,6 +84,7 @@ public function createWithoutActivity(array $data, bool $emailConfirmed = false)
8384
* As per "createWithoutActivity" but records a "create" activity.
8485
*
8586
* @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
87+
* @throws UserInviteException
8688
*/
8789
public function create(array $data, bool $sendInvite = false): User
8890
{

lang/en/errors.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
// Users
7979
'users_cannot_delete_only_admin' => 'You cannot delete the only admin',
8080
'users_cannot_delete_guest' => 'You cannot delete the guest user',
81+
'users_could_not_send_invite' => 'Could not create user since invite email failed to send',
8182

8283
// Roles
8384
'role_cannot_be_edited' => 'This role cannot be edited',

tests/User/UserManagementTest.php

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Tests\User;
44

5+
use BookStack\Access\UserInviteException;
56
use BookStack\Access\UserInviteService;
67
use BookStack\Activity\ActivityType;
78
use BookStack\Uploads\Image;
@@ -229,7 +230,7 @@ public function test_user_creation_is_not_performed_if_the_invitation_sending_fa
229230

230231
// Simulate an invitation sending failure
231232
$this->mock(UserInviteService::class, function (MockInterface $mock) {
232-
$mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class);
233+
$mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
233234
});
234235

235236
$this->asAdmin()->post('/settings/users/create', [
@@ -247,22 +248,42 @@ public function test_user_create_activity_is_not_persisted_if_the_invitation_sen
247248
{
248249
/** @var User $user */
249250
$user = User::factory()->make();
250-
$adminRole = Role::getRole('admin');
251251

252252
$this->mock(UserInviteService::class, function (MockInterface $mock) {
253-
$mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class);
253+
$mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
254254
});
255255

256256
$this->asAdmin()->post('/settings/users/create', [
257257
'name' => $user->name,
258258
'email' => $user->email,
259259
'send_invite' => 'true',
260-
'roles[' . $adminRole->id . ']' => 'true',
261260
]);
262261

263262
$this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']);
264263
}
265264

265+
public function test_return_to_form_with_warning_if_the_invitation_sending_fails()
266+
{
267+
$logger = $this->withTestLogger();
268+
/** @var User $user */
269+
$user = User::factory()->make();
270+
271+
$this->mock(UserInviteService::class, function (MockInterface $mock) {
272+
$mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
273+
});
274+
275+
$resp = $this->asAdmin()->post('/settings/users/create', [
276+
'name' => $user->name,
277+
'email' => $user->email,
278+
'send_invite' => 'true',
279+
]);
280+
281+
$resp->assertRedirect('/settings/users/create');
282+
$this->assertSessionError('Could not create user since invite email failed to send');
283+
$this->assertEquals($user->email, session()->getOldInput('email'));
284+
$this->assertTrue($logger->hasErrorThatContains('Failed to send user invite with error:'));
285+
}
286+
266287
public function test_user_create_update_fails_if_locale_is_invalid()
267288
{
268289
$user = $this->users->editor();

0 commit comments

Comments
 (0)