diff --git a/app/Actions/CreateBucketAction.php b/app/Actions/CreateBucketAction.php index 0397bb7..e19771f 100644 --- a/app/Actions/CreateBucketAction.php +++ b/app/Actions/CreateBucketAction.php @@ -77,12 +77,10 @@ public function execute( */ private function validateTypeConstraints(BucketTypeEnum $type, BucketAllocationTypeEnum $allocationType): void { - if ($type === BucketTypeEnum::OVERFLOW && $allocationType !== BucketAllocationTypeEnum::UNLIMITED) { - throw new InvalidArgumentException('Overflow buckets must use unlimited allocation type'); - } - - if ($type !== BucketTypeEnum::OVERFLOW && $allocationType === BucketAllocationTypeEnum::UNLIMITED) { - throw new InvalidArgumentException('Only overflow buckets can use unlimited allocation type'); + $allowedTypes = $type->getAllowedAllocationTypes(); + if (! in_array($allocationType, $allowedTypes, true)) { + $allowed = implode(', ', array_map(fn ($t) => $t->value, $allowedTypes)); + throw new InvalidArgumentException("Invalid allocation type for {$type->value} bucket. Allowed: {$allowed}"); } } diff --git a/app/Enums/BucketTypeEnum.php b/app/Enums/BucketTypeEnum.php index d7bcc84..4c1e891 100644 --- a/app/Enums/BucketTypeEnum.php +++ b/app/Enums/BucketTypeEnum.php @@ -21,4 +21,17 @@ public static function values(): array { return array_column(self::cases(), 'value'); } + + /** + * Get valid allocation types for this bucket type. + * + * @return BucketAllocationTypeEnum[] + */ + public function getAllowedAllocationTypes(): array + { + return match ($this) { + self::NEED, self::WANT => [BucketAllocationTypeEnum::FIXED_LIMIT, BucketAllocationTypeEnum::PERCENTAGE], + self::OVERFLOW => [BucketAllocationTypeEnum::UNLIMITED], + }; + } } diff --git a/app/Http/Controllers/BucketController.php b/app/Http/Controllers/BucketController.php index 19c2b45..e5f9997 100644 --- a/app/Http/Controllers/BucketController.php +++ b/app/Http/Controllers/BucketController.php @@ -178,17 +178,13 @@ private function validateBucketTypeConstraints( Scenario $scenario, ?Bucket $excludeBucket = null ): ?JsonResponse { - if ($type === BucketTypeEnum::OVERFLOW && $allocationType !== BucketAllocationTypeEnum::UNLIMITED) { - return response()->json([ - 'message' => 'Validation failed.', - 'errors' => ['allocation_type' => ['Overflow buckets must use unlimited allocation type.']], - ], 422); - } + $allowedTypes = $type->getAllowedAllocationTypes(); + if (! in_array($allocationType, $allowedTypes, true)) { + $allowed = implode(', ', array_map(fn ($t) => $t->getLabel(), $allowedTypes)); - if ($type !== BucketTypeEnum::OVERFLOW && $allocationType === BucketAllocationTypeEnum::UNLIMITED) { return response()->json([ 'message' => 'Validation failed.', - 'errors' => ['allocation_type' => ['Only overflow buckets can use unlimited allocation type.']], + 'errors' => ['allocation_type' => ["{$type->getLabel()} buckets only support: {$allowed}."]], ], 422); } diff --git a/app/Models/Bucket.php b/app/Models/Bucket.php index fadb215..216c9c2 100644 --- a/app/Models/Bucket.php +++ b/app/Models/Bucket.php @@ -139,25 +139,6 @@ public function getFormattedAllocationValue(): string return $this->allocation_type->formatValue($this->allocation_value); } - /** - * Validation rules for bucket creation/update. - */ - public static function validationRules($scenarioId = null): array - { - $rules = [ - 'name' => 'required|string|max:255', - 'allocation_type' => 'required|in:'.implode(',', BucketAllocationTypeEnum::values()), - 'priority' => 'required|integer|min:1', - ]; - - // Add scenario-specific priority uniqueness if scenario ID provided - if ($scenarioId) { - $rules['priority'] .= '|unique:buckets,priority,NULL,id,scenario_id,'.$scenarioId; - } - - return $rules; - } - /** * Get allocation value validation rules based on type. */ diff --git a/tests/Unit/Actions/CreateBucketActionTest.php b/tests/Unit/Actions/CreateBucketActionTest.php index 2c86fa5..aac2d0c 100644 --- a/tests/Unit/Actions/CreateBucketActionTest.php +++ b/tests/Unit/Actions/CreateBucketActionTest.php @@ -127,7 +127,7 @@ public function test_default_type_is_need(): void public function test_overflow_bucket_must_use_unlimited_allocation(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Overflow buckets must use unlimited allocation type'); + $this->expectExceptionMessage('Invalid allocation type for overflow bucket. Allowed: unlimited'); $this->action->execute( $this->scenario, @@ -141,7 +141,7 @@ public function test_overflow_bucket_must_use_unlimited_allocation(): void public function test_non_overflow_bucket_cannot_use_unlimited_allocation(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Only overflow buckets can use unlimited allocation type'); + $this->expectExceptionMessage('Invalid allocation type for need bucket. Allowed: fixed_limit, percentage'); $this->action->execute( $this->scenario, @@ -174,7 +174,7 @@ public function test_cannot_create_second_overflow_bucket(): void public function test_want_bucket_cannot_use_unlimited_allocation(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Only overflow buckets can use unlimited allocation type'); + $this->expectExceptionMessage('Invalid allocation type for want bucket. Allowed: fixed_limit, percentage'); $this->action->execute( $this->scenario, diff --git a/tests/Unit/Enums/BucketTypeEnumTest.php b/tests/Unit/Enums/BucketTypeEnumTest.php index a777808..142ad18 100644 --- a/tests/Unit/Enums/BucketTypeEnumTest.php +++ b/tests/Unit/Enums/BucketTypeEnumTest.php @@ -2,6 +2,7 @@ namespace Tests\Unit\Enums; +use App\Enums\BucketAllocationTypeEnum; use App\Enums\BucketTypeEnum; use PHPUnit\Framework\TestCase; @@ -30,4 +31,30 @@ public function test_values_returns_all_string_values(): void $this->assertContains('want', $values); $this->assertContains('overflow', $values); } + + public function test_need_allows_fixed_limit_and_percentage(): void + { + $allowed = BucketTypeEnum::NEED->getAllowedAllocationTypes(); + + $this->assertCount(2, $allowed); + $this->assertContains(BucketAllocationTypeEnum::FIXED_LIMIT, $allowed); + $this->assertContains(BucketAllocationTypeEnum::PERCENTAGE, $allowed); + } + + public function test_want_allows_fixed_limit_and_percentage(): void + { + $allowed = BucketTypeEnum::WANT->getAllowedAllocationTypes(); + + $this->assertCount(2, $allowed); + $this->assertContains(BucketAllocationTypeEnum::FIXED_LIMIT, $allowed); + $this->assertContains(BucketAllocationTypeEnum::PERCENTAGE, $allowed); + } + + public function test_overflow_only_allows_unlimited(): void + { + $allowed = BucketTypeEnum::OVERFLOW->getAllowedAllocationTypes(); + + $this->assertCount(1, $allowed); + $this->assertContains(BucketAllocationTypeEnum::UNLIMITED, $allowed); + } }