Skip to content
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

[Form] [DataMapper] Read only form datamapper fix #4280

Merged
merged 1 commit into from
May 15, 2012

Conversation

Romain-Geissler
Copy link
Contributor

The current 2.0.13 Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper enables to overwrite data from form values, no matter the form fields are read only or not.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

@travisbot
Copy link

This pull request passes (merged 47605f6 into 72b2f69).

@webmozart
Copy link
Contributor

Forms don't bind when they are read only, so why is this change necessary?

@stof
Copy link
Member

stof commented May 14, 2012

@bschussek A read-only child will not be bound but the setter will still be called on the parent object for this field (with the old value), making it mandatory to define setters for read-only fields.

@Romain-Geissler
Copy link
Contributor Author

In my case, the property is still set through a setter even if the field for this property is read only. The problem is the setter is not called with the legacy value it held, but with the value given by the form. In my case the value is transformed from a string to an object by a DataMapper, which returns null for an empty string/value. Thus, the setter is called with null instead of the previous non null value (and not always the same) it held.

This PR just prevent the setter for an object property marked as read only in the form definition from being called.

@webmozart
Copy link
Contributor

Ok, 👍 then

fabpot added a commit that referenced this pull request May 15, 2012
Commits
-------

47605f6 [Form][DataMapper] Do not update form to data when form is read only

Discussion
----------

[Form] [DataMapper] Read only form datamapper fix

The current 2.0.13 ``Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper`` enables to overwrite data from form values, no matter the form fields are read only or not.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

---------------------------------------------------------------------------

by travisbot at 2012-05-14T15:50:02Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1328279) (merged 47605f6 into 72b2f69).

---------------------------------------------------------------------------

by bschussek at 2012-05-14T18:06:41Z

Forms don't bind when they are read only, so why is this change necessary?

---------------------------------------------------------------------------

by stof at 2012-05-14T19:29:45Z

@bschussek A read-only child will not be bound but the setter will still be called on the parent object for this field (with the old value), making it mandatory to define setters for read-only fields.

---------------------------------------------------------------------------

by Romain-Geissler at 2012-05-14T19:43:11Z

In my case, the property is still set through a setter even if the field for this property is read only. The problem is the setter is not called with the legacy value it held, but with the value given by the form. In my case the value is transformed from a string to an object by a ``DataMapper``, which returns ``null`` for an empty string/value. Thus, the setter is called with ``null`` instead of the previous non ``null`` value (and not always the same) it held.

This PR just prevent the setter for an object property marked as read only in the form definition from being called.

---------------------------------------------------------------------------

by bschussek at 2012-05-15T08:20:28Z

Ok, :+1: then
@fabpot fabpot merged commit 47605f6 into symfony:2.0 May 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants