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

diff --git a/app/Access/UserInviteException.php b/app/Access/UserInviteException.php
new file mode 100644 (file)
index 0000000..70e7a78
--- /dev/null
+++ b/app/Access/UserInviteException.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace BookStack\Access;
+
+use Exception;
+
+class UserInviteException extends Exception
+{
+    //
+}

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

diff --git a/app/Users/Controllers/UserController.php b/app/Users/Controllers/UserController.php
index 185d6101c61413213e757057d086da16e89b36db..00bbe61186e5fab4fe6b4ac99a0547ec5b92c180 100644 (file)
--- a/app/Users/Controllers/UserController.php
+++ b/app/Users/Controllers/UserController.php
@@ -3,6 +3,7 @@
 namespace BookStack\Users\Controllers;
 
 use BookStack\Access\SocialDriverManager;
+use BookStack\Access\UserInviteException;
 use BookStack\Exceptions\ImageUploadException;
 use BookStack\Exceptions\UserUpdateException;
 use BookStack\Http\Controller;
@@ -14,6 +15,7 @@ use BookStack\Util\SimpleListOptions;
 use Exception;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\ValidationException;
 
@@ -91,9 +93,16 @@ class UserController extends Controller
 
         $validated = $this->validate($request, array_filter($validationRules));
 
-        DB::transaction(function () use ($validated, $sendInvite) {
-            $this->userRepo->create($validated, $sendInvite);
-        });
+        try {
+            DB::transaction(function () use ($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');
     }

diff --git a/app/Users/UserRepo.php b/app/Users/UserRepo.php
index 32e23ecdec132aced9684ab4413d86b2d50641a3..5c8ace8facade25f25412b7b63800ca560ada235 100644 (file)
--- a/app/Users/UserRepo.php
+++ b/app/Users/UserRepo.php
@@ -2,6 +2,7 @@
 
 namespace BookStack\Users;
 
+use BookStack\Access\UserInviteException;
 use BookStack\Access\UserInviteService;
 use BookStack\Activity\ActivityType;
 use BookStack\Entities\EntityProvider;
@@ -83,6 +84,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
+     * @throws UserInviteException
      */
     public function create(array $data, bool $sendInvite = false): User
     {

diff --git a/lang/en/errors.php b/lang/en/errors.php
index 752eb5672f1687616f1ca012d4a1b3b665c492b5..9c40aa9ed339578a1585ddefda68760cd2563c8e 100644 (file)
--- a/lang/en/errors.php
+++ b/lang/en/errors.php
@@ -78,6 +78,7 @@ return [
     // Users
     'users_cannot_delete_only_admin' => 'You cannot delete the only admin',
     '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
     'role_cannot_be_edited' => 'This role cannot be edited',

diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php
index 93d35f5d0472ee5be987e1a9c1adb6e66909b60f..8fe855afa771a9e94f4728a45ec015fe645f7f88 100644 (file)
--- a/tests/User/UserManagementTest.php
+++ b/tests/User/UserManagementTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\User;
 
+use BookStack\Access\UserInviteException;
 use BookStack\Access\UserInviteService;
 use BookStack\Activity\ActivityType;
 use BookStack\Uploads\Image;
@@ -229,7 +230,7 @@ class UserManagementTest extends TestCase
 
         // Simulate an invitation sending failure
         $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', [
@@ -247,22 +248,42 @@ class UserManagementTest extends TestCase
     {
         /** @var User $user */
         $user = User::factory()->make();
-        $adminRole = Role::getRole('admin');
 
         $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', [
             'name'                          => $user->name,
             'email'                         => $user->email,
             'send_invite'                   => 'true',
-            'roles[' . $adminRole->id . ']' => 'true',
         ]);
 
         $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()
     {
         $user = $this->users->editor();