-
Notifications
You must be signed in to change notification settings - Fork 0
adds enums #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds enums #1
Conversation
📝 WalkthroughWalkthroughIntroduces enum validation support for DTO properties. The Factory now detects enum definitions in property configurations and calls Property's new setEnum method, which manages enum validators using an IsInSet validator with configurable strict comparison mode. Changes
Sequence DiagramsequenceDiagram
participant Factory as Factory
participant Property as Property
participant IsInSet as IsInSet Validator
participant Value as Data Source
Factory->>Factory: Read property definition<br/>(includes enum array)
Factory->>Property: setEnum(allowedValues, strict)
activate Property
Property->>Property: Remove existing<br/>enum validator
Property->>IsInSet: Create new IsInSet validator<br/>(allowedValues, strict)
Property-->>Factory: Return self (fluent)
deactivate Property
Note over Value,IsInSet: During validation
Value->>IsInSet: Check if value in enum
alt Valid
IsInSet-->>Value: ✓ Pass
else Invalid
IsInSet-->>Value: ✗ Fail (throws)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Dto/EnumValidationTest.php (1)
198-203: Optional: Consider usingtearDown()for YAML cleanup.The temporary file cleanup is handled in a
finallyblock, which works correctly. For consistency with PHPUnit conventions, you could alternatively use atearDown()method or@afterannotation to manage temporary file cleanup across tests.Alternative approach using tearDown
private $tempFiles = []; protected function tearDown(): void { parent::tearDown(); foreach ($this->tempFiles as $file) { if (file_exists($file)) { unlink($file); } } $this->tempFiles = []; } public function testEnumLoadedFromYamlFile() { // ... existing code ... $tempFile = sys_get_temp_dir() . '/test_enum_' . uniqid() . '.yaml'; file_put_contents($tempFile, $yamlContent); $this->tempFiles[] = $tempFile; // ... rest of test without finally block ... }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Dto/Factory.phpsrc/Dto/Property.phptests/Dto/EnumValidationTest.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Dto/Property.php (1)
src/Dto/Validation.php (1)
Validation(11-29)
src/Dto/Factory.php (1)
src/Dto/Property.php (1)
setEnum(200-207)
tests/Dto/EnumValidationTest.php (4)
src/Dto/Dto.php (1)
Dto(15-261)src/Dto/Factory.php (2)
Factory(17-318)create(44-71)src/Dto/Property.php (1)
validate(234-256)src/Dto/Validation.php (1)
Validation(11-29)
🪛 GitHub Actions: CI
src/Dto/Property.php
[error] 204-204: Class "Neuron\Validation\IsInSet" not found during Enum validation.
🔇 Additional comments (2)
src/Dto/Factory.php (1)
188-192: LGTM - Enum handling logic is correct.The implementation follows the same pattern as other validators (length, range, pattern) and correctly defaults
stricttotruefor type-safe comparisons.tests/Dto/EnumValidationTest.php (1)
8-311: Comprehensive test coverage.The test suite thoroughly exercises enum validation across multiple scenarios including valid/invalid values, case sensitivity, integer enums, strict vs loose comparison, YAML definitions, optional properties, nested objects, and multiple enums. The test structure follows PHPUnit best practices.
However, all tests will fail until the missing
Validation\IsInSetclass is implemented (see review comment on Property.php).
| public function setEnum( array $allowedValues, bool $strict = true ): Property | ||
| { | ||
| $this->validators->remove( 'enum' ); | ||
|
|
||
| $this->validators->add( 'enum', new Validation\IsInSet( $allowedValues, $strict ) ); | ||
|
|
||
| return $this; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for IsInSet class definition
echo "=== Searching for IsInSet class definition ==="
rg -n --type=php "class IsInSet" || echo "No IsInSet class found"
echo ""
echo "=== Searching for any references to IsInSet ==="
rg -n --type=php "IsInSet" || echo "No IsInSet references found"
echo ""
echo "=== Listing all files in Validation namespace ==="
fd -e php . -x rg -l "namespace.*Validation" {} \; | sort
echo ""
echo "=== Checking existing validators ==="
fd -e php . -path "*/Validation/*" -type fRepository: Neuron-PHP/dto
Length of output: 563
🏁 Script executed:
#!/bin/bash
# Find the Validation namespace location
echo "=== Searching for Validation namespace declarations ==="
rg -n "namespace.*Validation" --type php
echo ""
echo "=== Finding all PHP files in src directory ==="
fd -e php . src/
echo ""
echo "=== Checking structure of src/Validation or similar ==="
find src -type d -name "*[Vv]alidat*" 2>/dev/null
echo ""
echo "=== Searching for other validators pattern (e.g., class Is*) ==="
rg -n "class Is[A-Z]" --type phpRepository: Neuron-PHP/dto
Length of output: 579
🏁 Script executed:
#!/bin/bash
# Read the Validation.php file to understand the structure
echo "=== Content of src/Dto/Validation.php ==="
cat -n src/Dto/Validation.php
echo ""
echo "=== Check how IsInSet is used in Property.php around line 204 ==="
sed -n '190,210p' src/Dto/Property.php | cat -nRepository: Neuron-PHP/dto
Length of output: 1363
🏁 Script executed:
#!/bin/bash
# Search for other validator usage patterns in Property.php
echo "=== Searching for validator usage patterns ==="
rg -n "validators->add" --type php -B 2 -A 2
echo ""
echo "=== Searching for Validation namespace usage ==="
rg -n "Validation\\\\" --type php
echo ""
echo "=== Check what other setters look like ==="
sed -n '150,250p' src/Dto/Property.php | grep -A 10 "public function set"Repository: Neuron-PHP/dto
Length of output: 4574
🏁 Script executed:
#!/bin/bash
# Search for where the validators are defined
echo "=== Searching for IsStringLength definition ==="
rg -n "class IsStringLength" --type php
echo ""
echo "=== Searching for IsRegExPattern definition ==="
rg -n "class IsRegExPattern" --type php
echo ""
echo "=== Searching for imports or use statements in Property.php ==="
head -20 src/Dto/Property.php
echo ""
echo "=== Check what Validation\Collection is ==="
rg -n "class Collection" --type php -A 5Repository: Neuron-PHP/dto
Length of output: 976
🏁 Script executed:
#!/bin/bash
# The validators are from Neuron\Validation namespace, let's check for external dependencies
echo "=== Checking for composer.json ==="
cat composer.json 2>/dev/null || echo "No composer.json found"
echo ""
echo "=== Checking for any package/vendor references ==="
find . -name "composer.lock" -o -name "vendor" -type d 2>/dev/null
echo ""
echo "=== Searching for all Validation\Is* usage in Property.php ==="
grep -n "Validation\\\\Is" src/Dto/Property.phpRepository: Neuron-PHP/dto
Length of output: 2250
Missing IsInSet validator in external dependency.
Line 204 uses Validation\IsInSet, which should come from the external neuron-php/validation package (currently required as ^0.7.0). The class does not exist in this repository because validators are provided by that dependency, not implemented locally. Verify that the installed version of neuron-php/validation includes the IsInSet class, or update the package version if this validator was added in a later release.
🧰 Tools
🪛 GitHub Actions: CI
[error] 204-204: Class "Neuron\Validation\IsInSet" not found during Enum validation.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.