-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[3.0] [Form] Setting "empty_data" to "" still results in NULL #5906
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
Comments
Bummer. This issue is more or less the same as #4715. The problem, like there, is that "empty_data" is responsible for creating the empty view data so that it can be mapped with the child data. Even if we assign Decoupling the data mapping from the form like proposed in #5480 might be a solution to this. Then the reverse transformation would take place before creating a new object to map into (if at all). "empty_data" could then refer to the model data that is used if the form is empty (would solve #4715) while the current "empty_data" would need a different name, e.g. "data_constructor". |
Hi, Any plan for this issue ? I also noticed that if we add a data transformer (something like NullToBlankTransformer) to a text field, the value is this time always at '', never null ... It looks like it come from : https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L1042 Actually with 2.3, I did the trick with calling submit() with 2nd param to false, and then add a view transformer to the field, which does nothing but bypass the null set statement in the viewToNorm method. (if it can help someone before the real fix). |
Fixed in the #10126 . It was also added the new test. But merged into master-dev with is not passing the tests. |
Is this really fixed? In what version? When I try to use |
@chadwickmeyer #10126 hasn't been merged so this is not fixed yet (therefore 8000 the current issue is still open). |
@jakzal Is there a plan to fix this? #10126 was rejected, because it says the fix needs to come from this bug report. So my question is, how is this not a major issue for other people? Is everyone just forced to make every field that is not required, into nullable=true? Otherwise they would get database errors on every persist... That seems like bad database architecture. There are a lot of reasons why you wouldn't want your field to be null. |
Am I misreading the documentation or does it say that empty_data should be able to be reset to '' or 0 or whatever you want instead of null? This documentation is really confusing if this attribute doesn't actually do that: http://symfony.com/doc/current/reference/forms/types/text.html#empty-data |
This is still an issue. |
Will this bug ever be fixed? |
I've run into this issue, a little surprised more haven't. |
anyone have a workaround for this? |
I have a Stackoverflow question on it, with one suggested hack work around, for whatever it's worth: http://stackoverflow.com/questions/24792285/symfony-form-field-attribute-empty-data-ignored |
I've also ran into this issue, what is the status of this? |
I've been running into this issue for quite a while, I just now found this bug ticket. Is it really difficult implement a fix? It's seems like an important issue to have open for over two years. |
May be U have a code in entity like this?
|
+1 |
2 similar comments
👍 |
+1 |
I would also like to see a solution to this. 👍 |
#14955 cover this case without BC, but when '' returned from empty_data that is Closure, still will be null returned. Case with closure can be covered by adding new param to options like 'empty_data_is_norm', I think it is not so often used case and can be documented at http://symfony.com/doc/current/cookbook/form/use_empty_data.html |
I don't think this can be solved reasonably in the current version of the Form component. Tagged for a future version. |
After 3 hours debugging here is the issue: https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Form/Form.php#L1187 private function viewToNorm($value)
} The "text" and "textarea" form types don't have transformers, therefore when the $value is empty string the viewToNorm return null instead of ''. I'm not sure what is the best way to fix it, but default transformer for text fields should help. |
Guys, any comments about this? Why?
|
@webmozart May be a BC break for any one extending We should probably do it in an extension until 3.4 though :/ Or maybe we could create a We could then trigger a deprecation in What do you think ? |
you already provide implementations for the interface, so extenders don't need to do it themselves. the only BC breaks would be accidental overrides, if someone is using the same method names as DataTransformerInterface, but for another purpose. |
@backbone87 of courses :) I believe I've spent to much time reading on this repository and now I've developed a BC break paranoia 😨 so I forget about the fundamentals... thanks for taking the time to remind me it! |
closes symfony#5906. The submitted data should always be transformed back to the model as a string as NULL in this case could stand for "unset this value" whereas a string property of a class could rely on the string type. Furthermore, this prevents potential issues with PHP 7 which allows type hinting of strings in functions.
closes symfony#5906. The submitted data should always be transformed back to the model as a string as NULL in this case could stand for "unset this value" whereas a string property of a class could rely on the string type. Furthermore, this prevents potential issues with PHP 7 which allows type hinting of strings in functions.
See #18357. |
closes symfony#5906. The submitted data should always be transformed back to the model as a string as NULL in this case could stand for "unset this value" whereas a string property of a class could rely on the string type. Furthermore, this prevents potential issues with PHP 7 which allows type hinting of strings in functions.
closes s A3E2 ymfony#5906. The submitted data should always be transformed back to the model as a string as NULL in this case could stand for "unset this value" whereas a string property of a class could rely on the string type. Furthermore, this prevents potential issues with PHP 7 which allows type hinting of strings in functions.
closes symfony#5906. The submitted data should always be transformed back to the model as a string as NULL in this case could stand for "unset this value" whereas a string property of a class could rely on the string type. Furthermore, this prevents potential issues with PHP 7 which allows type hinting of strings in functions.
…ace` (HeahDude) This PR was merged into the 3.1-dev branch. Discussion ---------- [Form] Let `TextType` implement `DataTransformerInterface` | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #5906 | License | MIT | Doc PR | ~ Commits ------- 7e97fbb [Form] let `TextType` implements `DataTransformerInterface`
For people still on 2.8... It seems that The simplest solution would be to write an |
@seggen-ibuildings you're right (ref #18357 (comment)). But this looks more like an |
@HeahDude I have nothing personal against Transformers, so I am not Anti :P The way I see it, |
Absolutely! We're saying the same thing differently, but you're right this method defines the default transformer and uses it only when none is provided. |
what is the purpose to transform an empty string to null? for my PoV is better delete that transformation with ternary operator that convert blank string to null So even for the types that needs null instead of blank set empty_data to null. This one have to be fixed, possibly in an elegant way, well anyway, At least should be pointed out that it will be converted to null when the form mapping is triggered. thank you anyway |
…verride them from inside a template. But, could we also control these from inside of our form class? Earlier, I mentioned that the options for a field are totally different than the variables for a field. Occasionally, a field has an option - like placeholder - and a variable with the same name, but that's not always true. But clearly, there must be some connection between options and variables. So what is it?! Form Type Classes & Options First, behind every field type is a class. Obviously, for the subFamily field, the class behind this is EntityType: 50 lines src/AppBundle/Form/GenusFormType.php ... lines 1 - 6 use Symfony\Bridge\Doctrine\Form\Type\EntityType; ... lines 8 - 13 class GenusFormType extends AbstractType { public function buildForm(FormBuilderInterface , array ) { {dollar_sign}builder ->add('name') ->add('subFamily', EntityType::class, [ ... lines 21 - 25 ]) ... lines 27 - 39 ; } ... lines 42 - 48 } name is a text type, so the class behind it, is, well, TextType. I'll use the Shift+Shift shortcut in my editor to open the TextType file, from the Symfony Form component: 69 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/TextType.php ... lines 1 - 11 namespace Symfony\Component\Form\Extension\Core\Type; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\DataTransformerInterface; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; class TextType extends AbstractType implements DataTransformerInterface { public function buildForm(FormBuilderInterface , array ) { // When empty_data is explicitly set to an empty string, // a string should always be returned when NULL is submitted // This gives more control and thus helps preventing some issues // with PHP 7 which allows type hinting strings in functions // See symfony/symfony#5906 (comment) if ('' === {dollar_sign}options['empty_data']) { {dollar_sign}builder->addViewTransformer({dollar_sign}this); } } ... lines 32 - 35 public function configureOptions(OptionsResolver ) { {dollar_sign}resolver->setDefaults(array( 'compound' => false, )); } ... lines 42 - 45 public function getBlockPrefix() { return 'text'; } ... lines 50 - 53 public function transform({dollar_sign}data) { // Model data should not be transformed return {dollar_sign}data; } ... lines 59 - 63 public function reverseTransform({dollar_sign}data) { return null === {dollar_sign}data ? '' : {dollar_sign}data; } } Now, unlike variables, there is a specific set of valid options for a field. If you pass an option that doesn't exist, Symfony will scream at you. The valid options for a field are determined by this configureOptions() method: 69 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/TextType.php ... lines 1 - 16 use Symfony\Component\OptionsResolver\OptionsResolver; class TextType extends AbstractType implements DataTransformerInterface { ... lines 21 - 35 public function configureOptions(OptionsResolver {dollar_sign}resolver) { {dollar_sign}resolver->setDefaults(array( 'compound' => false, )); } ... lines 42 - 67 } Apparently the TextType has a compound option, and it defaults to false. orm Type Inheritance Earlier, when we talked about form theme blocks, I mentioned that the field types have a built-in inheritance system. Well, technically, TextType extends AbstractType, but behind-the-scenes, the TextType's parent type is FormType. In fact, every field ultimately inherits options from FormType. Open that class: 195 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/FormType.php ... lines 1 - 11 namespace Symfony\Component\Form\Extension\Core\Type; ... lines 13 - 21 use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; class FormType extends BaseType { /** * @var PropertyAccessorInterface */ private {dollar_sign}propertyAccessor; public function __construct(PropertyAccessorInterface {dollar_sign}propertyAccessor = null) { {dollar_sign}this->propertyAccessor = {dollar_sign}propertyAccessor ?: PropertyAccess::createPropertyAccessor(); } ... lines 36 - 193 } Tip Wondering how you would know what the 'parent' type of a field is? Each *Type class has a getParent() method that will tell you. If you don't see one, then it's defaulting to FormType. This is cool because it also has a configureOptions() method that adds a bunch of options: 195 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/FormType.php ... lines 1 - 24 class FormType extends BaseType { ... lines 27 - 120 public function configureOptions(OptionsResolver {dollar_sign}resolver) { parent::configureOptions({dollar_sign}resolver); // Derive data_class option from passed data object {dollar_sign}dataClass = function (Options {dollar_sign}options) { return isset({dollar_sign}options['data']) && is_object(['data']) ? get_class({dollar_sign}options['data']) : null; }; // Derive empty_data closure from data_class option {dollar_sign}emptyData = function (Options {dollar_sign}options) { {dollar_sign}class = {dollar_sign}options['data_class']; if (null !== {dollar_sign}class) { return function (FormInterface {dollar_sign}form) use ({dollar_sign}class) { }; } return function (FormInterface {dollar_sign}form) { return {dollar_sign}form->getConfig()->getCompound() ? array() : ''; }; }; // For any form that is not represented by a single HTML control, // errors should bubble up by default {dollar_sign}errorBubbling = function (Options {dollar_sign}options) { return {dollar_sign}options['compound']; }; // If data is given, the form is locked to that data // (independent of its value) {dollar_sign}resolver->setDefined(array( 'data', )); {dollar_sign}resolver->setDefaults(array( 'data_class' => {dollar_sign}dataClass, 'empty_data' => {dollar_sign}emptyData, 'trim' => true, 'required' => true, 'property_path' => null, 'mapped' => true, 'by_reference' => true, 'error_bubbling' => {dollar_sign}errorBubbling, 'label_attr' => array(), 'inherit_data' => false, 'compound' => true, 'method' => 'POST', // According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt) // section 4.2., empty URIs are considered same-document references 'action' => '', 'attr' => array(), 'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.', )); {dollar_sign}resolver->setAllowedTypes('label_attr', 'array'); } ... lines 179 - 193 } These are the options that are available to every field type. And actually, the parent class - BaseType - has even more: 125 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php ... lines 1 - 11 namespace Symfony\Component\Form\Extension\Core\Type; ... lines 13 - 27 abstract class BaseType extends AbstractType { ... lines 30 - 109 public function configureOptions(OptionsResolver {dollar_sign}resolver) { {dollar_sign}resolver->setDefaults(array( 'block_name' => null, 'disabled' => false, 'label' => null, 'label_format' => null, 'attr' => array(), 'translation_domain' => null, 'auto_initialize' => true, )); {dollar_sign}resolver->setAllowedTypes('attr', 'array'); } } The Form to FormView Transition That's interesting! Let's see if we can figure out how the option and variable work together. Scroll up in BaseType. These classes also have another function called buildView(): 125 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php ... lines 1 - 27 abstract class BaseType extends AbstractType { ... lines 30 - 41 public function buildView(FormView {dollar_sign}view, FormInterface {dollar_sign}form, array {dollar_sign}options) { {dollar_sign}name = {dollar_sign}form->getName(); {dollar_sign}blockName = {dollar_sign}options['block_name'] ?: {dollar_sign}form->getName(); {dollar_sign}translationDomain = {dollar_sign}options['translation_domain']; {dollar_sign}labelFormat = {dollar_sign}options['label_format']; if ({dollar_sign}view->parent) { if ('' !== ({dollar_sign}parentFullName = {dollar_sign}view->parent->vars['full_name'])) { {dollar_sign}id = sprintf('%s_%s', {dollar_sign}view->parent->vars['id'], {dollar_sign}name); {dollar_sign}fullName = sprintf('%s[%s]', {dollar_sign}parentFullName, {dollar_sign}name); {dollar_sign}uniqueBlockPrefix = sprintf('%s_%s', {dollar_sign}view->parent->vars['unique_block_prefix'], {dollar_sign}blockName); } else { {dollar_sign}id = {dollar_sign}name; {dollar_sign}fullName = {dollar_sign}name; {dollar_sign}uniqueBlockPrefix = '_'.{dollar_sign}blockName; } if (null === {dollar_sign}translationDomain) { {dollar_sign}translationDomain = {dollar_sign}view->parent->vars['translation_domain']; } {dollar_sign}labelFormat = {dollar_sign}view->parent->vars['label_format']; } } else { {dollar_sign}id = {dollar_sign}name; {dollar_sign}fullName = {dollar_sign}name; {dollar_sign}uniqueBlockPrefix = '_'.{dollar_sign}blockName; // Strip leading underscores and digits. These are allowed in // form names, but not in HTML4 ID attributes. // http://www.w3.org/TR/html401/struct/global.html#adef-id {dollar_sign}id = ltrim({dollar_sign}id, '_0123456789'); } {dollar_sign}blockPrefixes = array(); for ({dollar_sign}type = {dollar_sign}form->getConfig()->getType(); null !== {dollar_sign}type; {dollar_sign}type = {dollar_sign}type->getParent()) { array_unshift({dollar_sign}blockPrefixes, {dollar_sign}type->getBlockPrefix()); } {dollar_sign}blockPrefixes[] = {dollar_sign}uniqueBlockPrefix; {dollar_sign}view->vars = array_replace(->vars, array( 'form' => {dollar_sign}view, 'id' => {dollar_sign}id, 'name' => {dollar_sign}name, 'full_name' => {dollar_sign}fullName, 'disabled' => {dollar_sign}form->isDisabled(), 'label' => {dollar_sign}options['label'], 'label_format' => {dollar_sign}labelFormat, 'multipart' => false, 'attr' => {dollar_sign}options['attr'], 'block_prefixes' => {dollar_sign}blockPrefixes, 'unique_block_prefix' => {dollar_sign}uniqueBlockPrefix, 'translation_domain' => {dollar_sign}translationDomain, // Using the block name here speeds up performance in collection // forms, where each entry has the same full block name. // Including the type is important too, because if rows of a // collection form have different types (dynamically), they should // be rendered differently. // symfony/symfony#5038 'cache_key' => {dollar_sign}uniqueBlockPrefix.'_'.{dollar_sign}form->getConfig()->getType()->getBlockPrefix(), )); } ... lines 106 - 123 } In a controller, when you pass your form into a template, you always call createView() on it first: 86 lines src/AppBundle/Controller/Admin/GenusAdminController.php ... lines 1 - 15 class GenusAdminController extends Controller { ... lines 18 - 63 public function editAction(Request {dollar_sign}request, Genus {dollar_sign}genus) { ... lines 66 - 81 return {dollar_sign}this->render('admin/genus/edit.html.twig', [ 'genusForm' => {dollar_sign}form->createView() ]); } } That line turns out to be very important: it transforms your Form object into a FormView object. In fact, your form is a big tree, with a Form on top and fields below it, which themselves are also Form objects. When you call createView(), all of the Form objects are transformed into FormView objects. To do that, the buildView() method is called on each individual field. And one of the arguments to buildView() is an array of the final options passed to this field. 125 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php ... lines 1 - 15 use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormView; ... lines 18 - 27 abstract class BaseType extends AbstractType { ... lines 30 - 41 public function buildView(FormView {dollar_sign}view, FormInterface {dollar_sign}form, array {dollar_sign}options) { ... lines 44 - 104 } ... lines 106 - 123 } For example, for subFamily, we're passing three options: 50 lines src/AppBundle/Form/GenusFormType.php ... lines 1 - 13 class GenusFormType extends AbstractType { public function buildForm(FormBuilderInterface {dollar_sign}builder, array {dollar_sign}options) { ... line 19 ->add('subFamily', EntityType::class, [ 'placeholder' => 'Choose a Sub Family', 'class' => SubFamily::class, 'query_builder' => function(SubFamilyRepository {dollar_sign}repo) { return {dollar_sign}repo->createAlphabeticalQueryBuilder(); } ]) ... lines 27 - 39 ; } ... lines 42 - 48 } We could also pass a label option here. These values - merged with any other default values set in configureOptions() - are then passed to buildView() and... if you scroll down a little bit, many of them are used to populate the vars on the FormView object of this field. Yep, these are the same vars that become so important when rendering that field: 125 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php ... lines 1 - 27 abstract class BaseType extends AbstractType { ... lines 30 - 41 public function buildView(FormView {dollar_sign}view, FormInterface {dollar_sign}form, array {dollar_sign}options) { ... lines 44 - 83 {dollar_sign}view->vars = array_replace({dollar_sign}view->vars, array( 'form' => {dollar_sign}view, 'id' => {dollar_sign}id, 'name' => {dollar_sign}name, 'full_name' => {dollar_sign}fullName, 'disabled' => {dollar_sign}form->isDisabled(), 'label' => {dollar_sign}options['label'], 'label_format' => {dollar_sign}labelFormat, 'multipart' => false, 'attr' => {dollar_sign}options['attr'], 'block_prefixes' => {dollar_sign}blockPrefixes, 'unique_block_prefix' => {dollar_sign}uniqueBlockPrefix, 'translation_domain' => {dollar_sign}translationDomain, // Using the block name here speeds up performance in collection // forms, where each entry has the same full block name. // Including the type is important too, because if rows of a // collection form have different types (dynamically), they should // be rendered differently. / 10000 / symfony/symfony#5038 'cache_key' => {dollar_sign}uniqueBlockPrefix.'_'.{dollar_sign}form->getConfig()->getType()->getBlockPrefix(), )); } ... lines 106 - 123 } To put it simply: every field has options and sometimes these options are used to set the form variables that control how the field is rendered: 125 lines vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php ... lines 1 - 27 abstract class BaseType extends AbstractType { ... lines 30 - 41 public function buildView(FormView {dollar_sign}view, FormInterface {dollar_sign}form, array {dollar_sign}options) { ... lines 44 - 83 ->vars = array_replace({dollar_sign}view->vars, array( ... lines 85 - 89 'label' => {dollar_sign}options['label'], ... lines 91 - 92 'attr' => {dollar_sign}options['attr'], ... lines 94 - 103 )); } ... lines 106 - 123 } Symfony gives us a label option as a convenient way to ultimately set the label variable. Close up all those classes. Here's a question: we know how to set the help variable from inside of a Twig template. But could we somehow set this variable from inside of the GenusFormType class? Yes, and there are actually two cool ways to do it. Let's check them out.
Is this issue possibly fixed in 3.2? |
Thanks @HeahDude, I missed it in the changelog.. Until version update, I'm going with a form extension. |
When setting the "empty_data" option of a text field to
""
, the submitted value is still converted tonull
.Actual result:
Expected result:
The text was updated successfully, but these errors were encountered: