Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DoctrineBridge] Fix required guess of boolean fields#16302
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
enumag commentedOct 20, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
enumag commentedOct 20, 2015
Currently boolean fields of Doctrine entities are correctly guessed as checkboxes but incorrectly guessed as required. |
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.
please use'boolean' !== $classMetadata->getTypeOfField($property)
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.
actually also use the constant please:https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/Type.php#L40
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.
Right... Forgot about the coding style.
Tobion commentedOct 22, 2015
Please also refactor guessType to use DBAL constants. |
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.
Should I add SIMPLE_ARRAY and JSON_ARRAY?
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.
Not sure if they work as collection form type as well. So lets do that in another PR if necessary.
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.
Not sure why this is here. There doesn't seem to be a corresponding type in Doctrine.
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.
Tobion commentedOct 22, 2015
👍 Should be merged in 2.3 by merger! Status: Reviewed |
enumag commentedOct 22, 2015
@Tobion Thanks! I refactored guessPattern and guessMaxLength to use the constants as well. |
fabpot commentedOct 23, 2015
Thank you@enumag. |
…mag)This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes#16302).Discussion----------[DoctrineBridge] Fix required guess of boolean fields| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------b21d498 [DoctrineBridge] Fix required guess of boolean fields