fix: a preset naming a deleted user does not break account saving

src/Application/Account/Services/AccountPreset.php

@@ -36,6 +36,12 @@
 use SP\Domain\Core\Exceptions\ConstraintException;
 use SP\Domain\Core\Exceptions\QueryException;
 use SP\Domain\Core\Exceptions\SPException;
+use SP\Domain\User\Ports\UserRepository;
+use SP\Domain\User\Models\UserGroup as UserGroupModel;
+use SP\Domain\User\Models\User as UserModel;
+use SP\Domain\User\Ports\UserGroupRepository;
+use SP\Domain\Common\Dtos\QueryResult;
+use SP\Domain\Common\Models\Simple;
 use SP\Domain\ItemPreset\Models\AccountPermission;
 use SP\Domain\ItemPreset\Models\ItemPreset as ItemPresetModel;
 use SP\Domain\ItemPreset\Models\Password;
@@ -50,14 +56,18 @@ final class AccountPreset extends Service implements AccountPresetService
 {
     /**
      * @param ItemPresetService<ItemPresetModel> $itemPresetService
+     * @param UserRepository<UserModel> $userRepository
+     * @param UserGroupRepository<UserGroupModel> $userGroupRepository
      */
     public function __construct(
         Application                                   $application,
         private readonly ItemPresetService $itemPresetService,
         private readonly AccountToUserGroupRepository $accountToUserGroupRepository,
         private readonly AccountToUserRepository      $accountToUserRepository,
         private readonly ConfigDataInterface          $configData,
-        private readonly PasswordValidator $passwordValidator
+        private readonly PasswordValidator $passwordValidator,
+        private readonly UserRepository               $userRepository,
+        private readonly UserGroupRepository          $userGroupRepository
     ) {
         parent::__construct($application);
     }
@@ -165,10 +175,21 @@ public function addPresetPermissions(int $accountId): void
             if ($accountPermission !== null) {
                 $userData = $this->context->getUserData();
 
-                $usersView = array_diff($accountPermission->getUsersView(), [$userData->id]);
-                $usersEdit = array_diff($accountPermission->getUsersEdit(), [$userData->id]);
-                $userGroupsView = array_diff($accountPermission->getUserGroupsView(), [$userData->userGroupId]);
-                $userGroupsEdit = array_diff($accountPermission->getUserGroupsEdit(), [$userData->userGroupId]);
+                // Only ids that still exist. The preset carries them inside a serialized blob, and
+                // no foreign key reaches in there — the one on ItemPreset covers the preset's own
+                // scope columns, not its contents. So a user or group named in a fixed preset can
+                // be deleted with nothing to stop it, and the next account anybody in that
+                // preset's scope saved failed on the foreign key these ids do have, inside the
+                // transaction, rolling the whole save back. Every account create and edit for
+                // those people, until an administrator worked out which preset to edit.
+                $usersView = $this->existingUsers(array_diff($accountPermission->getUsersView(), [$userData->id]));
+                $usersEdit = $this->existingUsers(array_diff($accountPermission->getUsersEdit(), [$userData->id]));
+                $userGroupsView = $this->existingUserGroups(
+                    array_diff($accountPermission->getUserGroupsView(), [$userData->userGroupId])
+                );
+                $userGroupsEdit = $this->existingUserGroups(
+                    array_diff($accountPermission->getUserGroupsEdit(), [$userData->userGroupId])
+                );
 
                 if (!empty($usersView)) {
                     $this->accountToUserRepository->addByType($accountId, $usersView);
@@ -188,4 +209,36 @@ public function addPresetPermissions(int $accountId): void
             }
         }
     }
+
+    /**
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    private function existingUsers(array $ids): array
+    {
+        if (empty($ids)) {
+            return [];
+        }
+
+        return $this->userRepository->getExistingIds($ids);
+    }
+
+    /**
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    private function existingUserGroups(array $ids): array
+    {
+        if (empty($ids)) {
+            return [];
+        }
+
+        return $this->userGroupRepository->getExistingIds($ids);
+    }
 }

src/Domain/User/Ports/UserGroupRepository.php

@@ -88,6 +88,17 @@ public function getById(int $id): QueryResult;
      */
     public function getAll(): QueryResult;
 
+    /**
+     * Which of the given ids still exist
+     *
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    public function getExistingIds(array $ids): array;
+
     /**
      * Deletes all the items for given ids
      *

src/Domain/User/Ports/UserRepository.php

@@ -168,6 +168,17 @@ public function getUserEmail(): QueryResult;
      */
     public function getUserEmailById(array $ids): QueryResult;
 
+    /**
+     * Which of the given ids still exist
+     *
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    public function getExistingIds(array $ids): array;
+
     /**
      * Returns the usage of the given user's id
      *

src/Infrastructure/Adapter/Out/User/Repositories/User.php

@@ -566,6 +566,34 @@ public function getUserEmail(): QueryResult
         return $this->db->runQuery(QueryData::build($query)->setMapClassName(UserModel::class));
     }
 
+    /**
+     * Which of the given ids still exist
+     *
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    public function getExistingIds(array $ids): array
+    {
+        if (empty($ids)) {
+            return [];
+        }
+
+        $query = $this->queryFactory
+            ->newSelect()
+            ->cols(['id'])
+            ->from(UserModel::TABLE)
+            ->where('id IN (:ids)', ['ids' => $ids]);
+
+        $result = $this->db->runQuery(QueryData::build($query)->setMapClassName(Simple::class));
+
+        // Array access rather than ->id: Simple declares no properties, every read goes through
+        // the model's outer-property bag, and static analysis cannot see through that.
+        return array_map(static fn(Simple $row): int => (int)$row['id'], $result->getDataAsArray());
+    }
+
     /**
      * Return the email of the given user's id
      *

src/Infrastructure/Adapter/Out/User/Repositories/UserGroup.php

@@ -195,6 +195,34 @@ public function getByName(string $name): QueryResult
         return $this->db->runQuery(QueryData::buildWithMapper($query, UserGroupModel::class));
     }
 
+    /**
+     * Which of the given ids still exist
+     *
+     * @param int[] $ids
+     *
+     * @return int[]
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    public function getExistingIds(array $ids): array
+    {
+        if (empty($ids)) {
+            return [];
+        }
+
+        $query = $this->queryFactory
+            ->newSelect()
+            ->cols(['id'])
+            ->from(UserGroupModel::TABLE)
+            ->where('id IN (:ids)', ['ids' => $ids]);
+
+        $result = $this->db->runQuery(QueryData::build($query)->setMapClassName(Simple::class));
+
+        // Array access rather than ->id: Simple declares no properties, every read goes through
+        // the model's outer-property bag, and static analysis cannot see through that.
+        return array_map(static fn(Simple $row): int => (int)$row['id'], $result->getDataAsArray());
+    }
+
     /**
      * Returns all the items
      *

tests/Unit/Application/Account/Services/AccountPresetTest.php

@@ -46,6 +46,11 @@
 use SP\Domain\Common\Validators\ValidatorInterface;
 use SP\Tests\Support\Generators\AccountDataGenerator;
 use SP\Tests\Support\Generators\ItemPresetDataGenerator;
+use SP\Domain\User\Ports\UserRepository;
+use SP\Domain\User\Ports\UserGroupRepository;
+use SP\Domain\Common\Models\Simple;
+use SP\Domain\Common\Dtos\QueryResult;
+use SP\Domain\ItemPreset\Models\AccountPermission;
 use SP\Tests\Support\UnitaryTestCase;
 
 /**
@@ -62,6 +67,10 @@ class AccountPresetTest extends UnitaryTestCase
     private ValidatorInterface|MockObject           $passwordValidator;
     private MockObject|AccountToUserGroupRepository $accountToUserGroupRepository;
     private AccountToUserRepository|MockObject      $accountToUserRepository;
+    private UserRepository|MockObject               $userRepository;
+    private UserGroupRepository|MockObject          $userGroupRepository;
+    /** @var int[] Ids a test has decided are gone from the database. */
+    private array                                   $deletedIds = [];
 
     /**
      * @throws QueryException
@@ -549,6 +558,53 @@ private function buildPasswordPresetWithExpireDays(int $expireDays): Password
         );
     }
 
+    /**
+     * A preset naming a user who has since been deleted still saves the account.
+     *
+     * The permission preset keeps its user and group ids inside a serialized blob, and no foreign
+     * key reaches in there — the one on ItemPreset covers the preset's own scope columns, not its
+     * contents. So deleting a user named in a fixed preset is allowed, and the ids AccountToUser
+     * *does* have a foreign key on then fail on the next insert: error 1452, raised inside
+     * Account::create()'s transaction, rolling the whole save back. Every account create and edit
+     * by anybody in that preset's scope, until an administrator worked out which preset to edit.
+     *
+     * The ones that still exist are applied; the stale one is dropped.
+     *
+     * @throws ConstraintException
+     * @throws QueryException
+     * @throws SPException
+     */
+    #[Test]
+    public function testAddPresetPermissionsSkipsAUserThatNoLongerExists()
+    {
+        $accountPermission = new AccountPermission(
+            usersView: [11, 12],
+            usersEdit: [],
+            userGroupsView: [],
+            userGroupsEdit: []
+        );
+
+        $this->itemPresetService
+            ->expects(self::once())
+            ->method('getForCurrentUser')
+            ->with('account.permission')
+            ->willReturn(
+                ItemPresetDataGenerator::factory()
+                                       ->buildItemPresetData($accountPermission)
+                                       ->mutate(['fixed' => 1])
+            );
+
+        // 12 has been deleted since the preset named it.
+        $this->deletedIds = [12];
+
+        $this->accountToUserRepository
+            ->expects(self::once())
+            ->method('addByType')
+            ->with(100, [11], false);
+
+        $this->accountPreset->addPresetPermissions(100);
+    }
+
     protected function setUp(): void
     {
         parent::setUp();
@@ -560,6 +616,16 @@ protected function setUp(): void
         $this->passwordValidator = $this->createMock(PasswordValidator::class);
         $this->accountToUserGroupRepository = $this->createMock(AccountToUserGroupRepository::class);
         $this->accountToUserRepository = $this->createMock(AccountToUserRepository::class);
+        $this->userRepository = $this->createMock(UserRepository::class);
+        $this->userGroupRepository = $this->createMock(UserGroupRepository::class);
+
+        // Every id the preset names still exists unless a test puts one in $deletedIds, which is
+        // the ordinary case — the filter is there for the one where it does not. One stub rather
+        // than a per-test override, because the first stub registered is the one that answers.
+        $echo = fn(array $ids): array => array_values(array_diff($ids, $this->deletedIds));
+
+        $this->userRepository->method('getExistingIds')->willReturnCallback($echo);
+        $this->userGroupRepository->method('getExistingIds')->willReturnCallback($echo);
 
         $this->accountPreset =
             new AccountPreset(
@@ -568,7 +634,10 @@ protected function setUp(): void
                 $this->accountToUserGroupRepository,
                 $this->accountToUserRepository,
                 $configData,
-                $this->passwordValidator
+                $this->passwordValidator,
+                $this->userRepository,
+                $this->userGroupRepository
             );
     }
+
 }