OpusProjects · blaipr · Sep 2, 2026 · Sep 2, 2026
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -553,6 +553,15 @@ mistaken for a missing row. Without that attribute this check would turn every u
 "not found", which is worth measuring through the application's own PDO options rather than the
 `mariadb` client, since the client does not set it and answers 0 for the same statement.
 
+The same count, asked as the wrong question. `deleteByIdBatch()` has the identical shape and a
+second way to get it wrong: `AuthToken`, `Client`, `Category` and `CustomFieldDefinition` refused
+only when the delete affected **zero** rows, where the other nine compare it against
+`count($ids)` — so a selection of five of which one was already gone came back as five removed.
+Three of their unit tests stubbed "1 affected" for five ids and passed, and an integration test was
+named `deleteMultiplePartialMatchIsNotDetected`, pinning the gap as known rather than fixing it.
+**A test that records a weaker behaviour with a name saying so is a defect somebody chose to
+describe** — read it as a lead, not as a decision, and check what its siblings do.
+
 **A static factory a subclass cannot use.** `SPException` offers `error()`, `info()`, `critical()`,
 `warning()` and `from()`, each doing `new static($message, …)`. Four subclasses fix their own message
 and take `int $type` first instead — `AccountPermissionException`, `UnauthorizedActionException`,

diff --git a/src/Application/Auth/Services/AuthToken.php b/src/Application/Auth/Services/AuthToken.php
@@ -155,7 +155,12 @@ public function delete(int $id): void
      */
     public function deleteByIdBatch(array $ids): void
     {
-        if ($this->authTokenRepository->deleteByIdBatch($ids)->getAffectedNumRows() === 0) {
+        // A batch that removed fewer rows than it was given ids has not done what it was asked,
+        // and answering the caller with success reports the removal of something another session
+        // had already deleted — or of an id that never existed — as done. `=== 0` only refuses
+        // when *nothing* matched, so five ids of which one was stale came back as five removed.
+        // This is the comparison the other nine services make.
+        if ($this->authTokenRepository->deleteByIdBatch($ids)->getAffectedNumRows() !== count($ids)) {
             throw new ServiceException(__u('Error while removing the tokens'), SPException::WARNING);
         }
     }

diff --git a/src/Application/Category/Services/Category.php b/src/Application/Category/Services/Category.php
@@ -132,7 +132,12 @@ public function delete(int $id): CategoryService
      */
     public function deleteByIdBatch(array $ids): void
     {
-        if ($this->categoryRepository->deleteByIdBatch($ids)->getAffectedNumRows() === 0) {
+        // A batch that removed fewer rows than it was given ids has not done what it was asked,
+        // and answering the caller with success reports the removal of something another session
+        // had already deleted — or of an id that never existed — as done. `=== 0` only refuses
+        // when *nothing* matched, so five ids of which one was stale came back as five removed.
+        // This is the comparison the other nine services make.
+        if ($this->categoryRepository->deleteByIdBatch($ids)->getAffectedNumRows() !== count($ids)) {
             throw new ServiceException(__u('Error while deleting categories'), SPException::WARNING);
         }
     }

diff --git a/src/Application/Client/Services/Client.php b/src/Application/Client/Services/Client.php
@@ -136,7 +136,12 @@ public function delete(int $id): ClientService
      */
     public function deleteByIdBatch(array $ids): void
     {
-        if ($this->clientRepository->deleteByIdBatch($ids)->getAffectedNumRows() === 0) {
+        // A batch that removed fewer rows than it was given ids has not done what it was asked,
+        // and answering the caller with success reports the removal of something another session
+        // had already deleted — or of an id that never existed — as done. `=== 0` only refuses
+        // when *nothing* matched, so five ids of which one was stale came back as five removed.
+        // This is the comparison the other nine services make.
+        if ($this->clientRepository->deleteByIdBatch($ids)->getAffectedNumRows() !== count($ids)) {
             throw new ServiceException(
                 __u('Error while deleting the clients'),
                 SPException::WARNING

diff --git a/src/Application/CustomField/Services/CustomFieldDefinition.php b/src/Application/CustomField/Services/CustomFieldDefinition.php
@@ -80,7 +80,10 @@ public function deleteByIdBatch(array $ids): void
             function () use ($ids) {
                 $affectedNumRows = $this->customFieldDefinitionRepository->deleteByIdBatch($ids)->getAffectedNumRows();
 
-                if ($affectedNumRows === 0) {
+                // Fewer rows than ids means at least one was already gone, and reporting that as
+                // a full removal is the defect the other nine services refuse. Inside the
+                // transaction, so the refusal takes the partial delete with it.
+                if ($affectedNumRows !== count($ids)) {
                     throw ServiceException::warning(__u('Error while deleting the fields'));
                 }
             },

diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/AuthToken/AuthTokenTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/AuthToken/AuthTokenTest.php
@@ -75,6 +75,10 @@ public function create()
     #[Test]
     public function deleteMultiple()
     {
+        // The batch delete asserts that as many rows were affected as ids were sent.
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            return new QueryResult([], 3, 0);
+        };
         $container = $this->buildContainer(
             IntegrationTestCase::buildRequest(
                 'get',
@@ -88,6 +92,33 @@ public function deleteMultiple()
         $this->expectOutputString('{"status":"OK","description":"Authorizations deleted","data":null}');
     }
 
+    /**
+     * If the DELETE affects fewer rows than ids were sent — one of them no longer existed, or
+     * another session removed it first — the batch is reported as a failure rather than as a
+     * partial success. This service used to refuse only when *nothing* matched, so a selection of
+     * which one item was already gone came back as the whole selection removed.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    #[Test]
+    public function deleteMultiplePartialFailure()
+    {
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            // Only 1 of the 2 requested ids was actually removed.
+            return new QueryResult([], 1, 0);
+        };
+
+        $container = $this->buildContainer(
+            IntegrationTestCase::buildRequest('get', 'index.php', ['r' => 'authToken/delete', 'items' => [100, 200]])
+        );
+
+        IntegrationTestCase::runApp($container);
+
+        $this->expectOutputString('{"status":"ERROR","description":"Error while removing the tokens","data":null}');
+    }
+
     /**
      * @throws ContainerExceptionInterface
      * @throws Exception

diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/Category/CategoryTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/Category/CategoryTest.php
@@ -33,6 +33,7 @@
 use Psr\Container\NotFoundExceptionInterface;
 use SP\Domain\Category\Models\Category;
 use SP\Domain\Common\Dtos\QueryResult;
+use SP\Infrastructure\Database\QueryData;
 use SP\Tests\Support\BodyChecker;
 use SP\Tests\Support\Generators\CategoryGenerator;
 use SP\Tests\Support\IntegrationTestCase;
@@ -68,6 +69,10 @@ public function create()
     #[Test]
     public function deleteMultiple()
     {
+        // The batch delete asserts that as many rows were affected as ids were sent.
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            return new QueryResult([], 3, 0);
+        };
         $container = $this->buildContainer(
             IntegrationTestCase::buildRequest('get', 'index.php', ['r' => 'category/delete', 'items' => [100, 200, 300]]
             )
@@ -78,6 +83,33 @@ public function deleteMultiple()
         $this->expectOutputString('{"status":"OK","description":"Categories deleted","data":null}');
     }
 
+    /**
+     * If the DELETE affects fewer rows than ids were sent — one of them no longer existed, or
+     * another session removed it first — the batch is reported as a failure rather than as a
+     * partial success. This service used to refuse only when *nothing* matched, so a selection of
+     * which one item was already gone came back as the whole selection removed.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    #[Test]
+    public function deleteMultiplePartialFailure()
+    {
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            // Only 1 of the 2 requested ids was actually removed.
+            return new QueryResult([], 1, 0);
+        };
+
+        $container = $this->buildContainer(
+            IntegrationTestCase::buildRequest('get', 'index.php', ['r' => 'category/delete', 'items' => [100, 200]])
+        );
+
+        IntegrationTestCase::runApp($container);
+
+        $this->expectOutputString('{"status":"ERROR","description":"Error while deleting categories","data":null}');
+    }
+
     /**
      * @throws ContainerExceptionInterface
      * @throws Exception

diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/Client/ClientTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/Client/ClientTest.php
@@ -33,6 +33,7 @@
 use Psr\Container\NotFoundExceptionInterface;
 use SP\Domain\Client\Models\Client;
 use SP\Domain\Common\Dtos\QueryResult;
+use SP\Infrastructure\Database\QueryData;
 use SP\Tests\Support\BodyChecker;
 use SP\Tests\Support\Generators\ClientGenerator;
 use SP\Tests\Support\IntegrationTestCase;
@@ -68,6 +69,10 @@ public function create()
     #[Test]
     public function deleteMultiple()
     {
+        // The batch delete asserts that as many rows were affected as ids were sent.
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            return new QueryResult([], 3, 0);
+        };
         $container = $this->buildContainer(
             IntegrationTestCase::buildRequest('get', 'index.php', ['r' => 'client/delete', 'items' => [100, 200, 300]])
         );
@@ -77,6 +82,33 @@ public function deleteMultiple()
         $this->expectOutputString('{"status":"OK","description":"Clients deleted","data":null}');
     }
 
+    /**
+     * If the DELETE affects fewer rows than ids were sent — one of them no longer existed, or
+     * another session removed it first — the batch is reported as a failure rather than as a
+     * partial success. This service used to refuse only when *nothing* matched, so a selection of
+     * which one item was already gone came back as the whole selection removed.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    #[Test]
+    public function deleteMultiplePartialFailure()
+    {
+        $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
+            // Only 1 of the 2 requested ids was actually removed.
+            return new QueryResult([], 1, 0);
+        };
+
+        $container = $this->buildContainer(
+            IntegrationTestCase::buildRequest('get', 'index.php', ['r' => 'client/delete', 'items' => [100, 200]])
+        );
+
+        IntegrationTestCase::runApp($container);
+
+        $this->expectOutputString('{"status":"ERROR","description":"Error while deleting the clients","data":null}');
+    }
+
     /**
      * @throws ContainerExceptionInterface
      * @throws Exception

diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/CustomField/CustomFieldTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/CustomField/CustomFieldTest.php
@@ -242,11 +242,8 @@ public function deleteMultiple()
     }
 
     /**
-     * Unlike Tag's batch delete (which requires every requested id to be affected),
-     * CustomFieldDefinitionService::deleteByIdBatch() only checks for `$affectedNumRows === 0`
-     * — see CustomFieldDefinition.php lines 77-89 in src/Application/CustomField/Services/.
-     * So when none of the requested ids matched a row, the transactionAware() closure throws,
-     * the transaction rolls back, and the batch is reported as a failure.
+     * When none of the requested ids matched a row, the transactionAware() closure throws, the
+     * transaction rolls back, and the batch is reported as a failure.
      *
      * @throws ContainerExceptionInterface
      * @throws Exception
@@ -270,17 +267,19 @@ public function deleteMultipleNoneFound()
     }
 
     /**
-     * ...but a PARTIAL match (some requested ids existed, some didn't) is NOT caught by that
-     * `=== 0` check, so it is reported as a full "Fields deleted" success even though some of
-     * the requested rows were never removed. This pins the current (weaker-than-Tag) behaviour
-     * in place — see the note above for the exact guard that lets it through.
+     * ...and so is a PARTIAL match, where some of the requested ids existed and some did not.
+     *
+     * It used not to be. The `=== 0` guard refused only when nothing at all matched, so a
+     * selection of which one item had already been deleted came back as "Fields deleted" while
+     * some of the requested rows were never removed. This test recorded that as the current,
+     * weaker-than-Tag behaviour; it now asserts the refusal, which is what Tag has always done.
      *
      * @throws ContainerExceptionInterface
      * @throws Exception
      * @throws NotFoundExceptionInterface
      */
     #[Test]
-    public function deleteMultiplePartialMatchIsNotDetected()
+    public function deleteMultiplePartialMatchIsRefused()
     {
         $this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
             // Only 1 of the 2 requested ids actually matched a row.
@@ -293,7 +292,7 @@ public function deleteMultiplePartialMatchIsNotDetected()
 
         IntegrationTestCase::runApp($container);
 
-        $this->expectOutputString('{"status":"OK","description":"Fields deleted","data":null}');
+        $this->expectOutputString('{"status":"ERROR","description":"Error while deleting the fields","data":null}');
     }
 
     /**

diff --git a/tests/Unit/Application/Auth/Services/AuthTokenTest.php b/tests/Unit/Application/Auth/Services/AuthTokenTest.php
@@ -90,7 +90,33 @@ public function testDeleteByIdBatch()
             ->expects(self::once())
             ->method('deleteByIdBatch')
             ->with($ids)
-            ->willReturn(new QueryResult(null, 1));
+            ->willReturn(new QueryResult(null, count($ids)));
+
+        $this->authToken->deleteByIdBatch($ids);
+    }
+
+    /**
+     * A batch of five ids of which one was already gone is not a removal of five.
+     *
+     * This service refused only when *nothing* matched, so an administrator selecting several
+     * tokens while a colleague deleted one of them was told the whole selection had been
+     * removed. Nine of the thirteen services already compare the count against the ids they were
+     * given; these four did not.
+     *
+     * @throws ServiceException
+     */
+    public function testDeleteByIdBatchWithOneAlreadyGone()
+    {
+        $ids = array_map(fn() => self::$faker->numberBetween(1, 1000), range(0, 4));
+
+        $this->authTokenRepository
+            ->expects(self::once())
+            ->method('deleteByIdBatch')
+            ->with($ids)
+            ->willReturn(new QueryResult(null, count($ids) - 1));
+
+        $this->expectException(ServiceException::class);
+        $this->expectExceptionMessage('Error while removing the tokens');
 
         $this->authToken->deleteByIdBatch($ids);
     }

diff --git a/tests/Unit/Application/Category/Services/CategoryTest.php b/tests/Unit/Application/Category/Services/CategoryTest.php
@@ -249,7 +249,33 @@ public function testDeleteByIdBatch()
             ->expects(self::once())
             ->method('deleteByIdBatch')
             ->with($ids)
-            ->willReturn(new QueryResult(null, 1));
+            ->willReturn(new QueryResult(null, count($ids)));
+
+        $this->category->deleteByIdBatch($ids);
+    }
+
+    /**
+     * A batch of five ids of which one was already gone is not a removal of five.
+     *
+     * This service refused only when *nothing* matched, so an administrator selecting several
+     * categories while a colleague deleted one of them was told the whole selection had been
+     * removed. Nine of the thirteen services already compare the count against the ids they were
+     * given; these four did not.
+     *
+     * @throws ServiceException
+     */
+    public function testDeleteByIdBatchWithOneAlreadyGone()
+    {
+        $ids = array_map(fn() => self::$faker->numberBetween(1, 1000), range(0, 4));
+
+        $this->categoryRepository
+            ->expects(self::once())
+            ->method('deleteByIdBatch')
+            ->with($ids)
+            ->willReturn(new QueryResult(null, count($ids) - 1));
+
+        $this->expectException(ServiceException::class);
+        $this->expectExceptionMessage('Error while deleting categories');
 
         $this->category->deleteByIdBatch($ids);
     }

diff --git a/tests/Unit/Application/Client/Services/ClientTest.php b/tests/Unit/Application/Client/Services/ClientTest.php
@@ -251,7 +251,33 @@ public function testDeleteByIdBatch()
             ->expects(self::once())
             ->method('deleteByIdBatch')
             ->with($ids)
-            ->willReturn(new QueryResult(null, 1));
+            ->willReturn(new QueryResult(null, count($ids)));
+
+        $this->client->deleteByIdBatch($ids);
+    }
+
+    /**
+     * A batch of five ids of which one was already gone is not a removal of five.
+     *
+     * This service refused only when *nothing* matched, so an administrator selecting several
+     * clients while a colleague deleted one of them was told the whole selection had been
+     * removed. Nine of the thirteen services already compare the count against the ids they were
+     * given; these four did not.
+     *
+     * @throws ServiceException
+     */
+    public function testDeleteByIdBatchWithOneAlreadyGone()
+    {
+        $ids = array_map(fn() => self::$faker->numberBetween(1, 1000), range(0, 4));
+
+        $this->clientRepository
+            ->expects(self::once())
+            ->method('deleteByIdBatch')
+            ->with($ids)
+            ->willReturn(new QueryResult(null, count($ids) - 1));
+
+        $this->expectException(ServiceException::class);
+        $this->expectExceptionMessage('Error while deleting the clients');
 
         $this->client->deleteByIdBatch($ids);
     }