From cb4af693b1c4101f426cfb8504546bef6e6cf5fd Mon Sep 17 00:00:00 2001 From: myrmidex Date: Sat, 27 Jun 2026 20:03:54 +0200 Subject: [PATCH] 57 - Add DELETE to the Bucket API --- src/Entity/Bucket.php | 3 +- tests/Functional/BucketApiTest.php | 126 ++++++++++++++++++++++++ tests/Functional/BucketOwnerApiTest.php | 33 +++++++ 3 files changed, 161 insertions(+), 1 deletion(-) diff --git a/src/Entity/Bucket.php b/src/Entity/Bucket.php index 641b1b5..ce6c171 100644 --- a/src/Entity/Bucket.php +++ b/src/Entity/Bucket.php @@ -3,6 +3,7 @@ namespace App\Entity; use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Delete; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\GetCollection; use ApiPlatform\Metadata\Patch; @@ -26,7 +27,7 @@ use Symfony\Component\Validator\Constraints as Assert; #[UniquePriorityPerScenario] #[BufferOnlyForFixedLimit] #[ApiResource( - operations: [new GetCollection(), new Get(), new Post(), new Patch()], + operations: [new GetCollection(), new Get(), new Post(), new Patch(), new Delete()], security: "is_granted('ROLE_USER')", )] class Bucket diff --git a/tests/Functional/BucketApiTest.php b/tests/Functional/BucketApiTest.php index d1266be..6c00cba 100644 --- a/tests/Functional/BucketApiTest.php +++ b/tests/Functional/BucketApiTest.php @@ -4,9 +4,12 @@ namespace App\Tests\Functional; use App\Entity\Bucket; use App\Entity\Scenario; +use App\Entity\Stream; use App\Entity\User; use App\Enum\BucketAllocationType; use App\Enum\BucketType; +use App\Enum\StreamFrequency; +use App\Enum\StreamType; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; @@ -89,6 +92,23 @@ final class BucketApiTest extends WebTestCase return '/api/scenarios/'.$scenario->getId(); } + private function persistStream(Scenario $scenario, Bucket $bucket, string $name): Stream + { + $stream = new Stream(); + $stream->setScenario($scenario); + $stream->setBucket($bucket); + $stream->setName($name); + $stream->setAmount(1000); + $stream->setType(StreamType::INCOME); + $stream->setFrequency(StreamFrequency::MONTHLY); + $stream->setStartDate(new \DateTimeImmutable('2026-01-01')); + + $this->em->persist($stream); + $this->em->flush(); + + return $stream; + } + /** * Asserts that the JSON-LD 422 response body contains a Hydra * ConstraintViolationList violation at the given propertyPath, pinning @@ -688,6 +708,112 @@ final class BucketApiTest extends WebTestCase ); } + public function testDeleteRemovesAnOwnedBucket(): void + { + $scenario = $this->persistScenario('Household Budget'); + $bucket = $this->persistBucket($scenario, 'Rent', 1); + + $this->login(); + + $this->client->request( + 'DELETE', + '/api/buckets/'.$bucket->getId(), + server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ], + ); + + self::assertSame( + 204, + $this->client->getResponse()->getStatusCode(), + 'DELETE /api/buckets/{id} for an owned bucket must return 204 No Content.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Bucket::class)->find($bucket->getId()); + + self::assertNull( + $persisted, + 'A successful DELETE /api/buckets/{id} must remove the Bucket from the database.', + ); + } + + /** + * CHARACTERIZATION TEST — pins existing DB-level behaviour, not new logic. + * + * `stream.bucket_id` has an `ON DELETE SET NULL` foreign key (migration + * Version20260619175824, predating #57). This test confirms the + * API-level/ORM consequence: deleting a Bucket that a Stream points at + * does NOT cascade-delete the Stream — the Stream survives with its + * `bucket` relation nulled out. The cascade already works; this test + * was born green. + */ + public function testDeletingABucketReferencedByAStreamNullsTheStreamsBucket(): void + { + $scenario = $this->persistScenario('Household Budget'); + $bucket = $this->persistBucket($scenario, 'Rent', 1); + $stream = $this->persistStream($scenario, $bucket, 'Paycheck'); + + $this->login(); + + $this->client->request( + 'DELETE', + '/api/buckets/'.$bucket->getId(), + server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ], + ); + + self::assertSame( + 204, + $this->client->getResponse()->getStatusCode(), + 'DELETE /api/buckets/{id} for an owned bucket referenced by a stream must still return 204 No Content.', + ); + + $this->em->clear(); + + $persistedBucket = $this->em->find(Bucket::class, $bucket->getId()); + + self::assertNull( + $persistedBucket, + 'The deleted Bucket must no longer exist.', + ); + + $persistedStream = $this->em->find(Stream::class, $stream->getId()); + + self::assertNotNull( + $persistedStream, + 'The Stream that referenced the deleted Bucket must survive the delete — only its bucket relation ' + .'is nulled out, the stream itself is not cascade-deleted.', + ); + self::assertNull( + $persistedStream->getBucket(), + 'After deleting the referenced Bucket, the Stream\'s bucket relation must be SET NULL by the ' + .'database foreign key (ON DELETE SET NULL on stream.bucket_id).', + ); + } + + public function testDeleteIsRejectedWhenUnauthenticated(): void + { + $scenario = $this->persistScenario('Household Budget'); + $bucket = $this->persistBucket($scenario, 'Rent', 1); + + $this->client->request( + 'DELETE', + '/api/buckets/'.$bucket->getId(), + server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ], + ); + + self::assertSame( + 401, + $this->client->getResponse()->getStatusCode(), + 'DELETE /api/buckets/{id} must require authentication.', + ); + } + public function testPostWithNonJsonContentTypeIsRejected(): void { $this->login(); diff --git a/tests/Functional/BucketOwnerApiTest.php b/tests/Functional/BucketOwnerApiTest.php index 7e8c1ba..ea7d276 100644 --- a/tests/Functional/BucketOwnerApiTest.php +++ b/tests/Functional/BucketOwnerApiTest.php @@ -317,6 +317,39 @@ final class BucketOwnerApiTest extends WebTestCase ); } + public function testDeleteOnABucketOwnedBySomeoneElseReturnsNotFound(): void + { + $othersScenario = $this->persistScenarioFor($this->otherUser, 'Someone Else\'s Fund'); + $othersBucket = $this->persistBucket($othersScenario, 'Their Rent', 1); + + $this->login(); + + $this->client->request( + 'DELETE', + '/api/buckets/'.$othersBucket->getId(), + server: [ + 'HTTP_ACCEPT' => 'application/ld+json', + ], + ); + + self::assertSame( + 404, + $this->client->getResponse()->getStatusCode(), + 'DELETE /api/buckets/{id} for a bucket whose scenario is owned by another user must return 404, not ' + .'403 — no existence oracle for buckets you do not own, scoped transitively via scenario.owner.', + ); + + $this->em->clear(); + + $persisted = $this->em->getRepository(Bucket::class)->find($othersBucket->getId()); + + self::assertNotNull( + $persisted, + 'A rejected DELETE against a non-owned bucket must never delete it — no cross-tenant deletion must ' + .'leak through the 404 item-resolution failure.', + ); + } + /** * Mirrors the class-level docblock on {@see testPostUnderAForeignScenarioCurrentlyReturnsBadRequestNotFound()}: * this pins OBSERVED behaviour for an attempted cross-tenant re-parent via PATCH, not an assumed contract.