Fix ACL Forms #3746
Fix ACL Forms #3746
Conversation
FormBuilder expects a string and not an object
| @@ -272,7 +272,7 @@ protected function buildForm(AdminObjectAclData $data, FormBuilderInterface $for | |||
| ); | |||
| } | |||
|
|
|||
| $formBuilder->add($key, new AclMatrixType(), array('permissions' => $permissions, 'acl_value' => $aclValue)); | |||
| $formBuilder->add($key, AclMatrixType::class, array('permissions' => $permissions, 'acl_value' => $aclValue)); | |||
There was a problem hiding this comment.
This won't work with old PHP versions.
There was a problem hiding this comment.
@core23 Wouldn't it be time to stop supporting 5.4? It's already long gone...
There was a problem hiding this comment.
Bye bye array(array(array( 😆 )))
There was a problem hiding this comment.
I am the last person who would support these old crap... But we use semver, so we must support it until the next major release.
There was a problem hiding this comment.
Don't use ::class and this should be good to merge.
|
@Turko : would you like to make this in a BC way ? Or do I mark this as "major" and schedule it for 4.0 ? EDIT: this is totally doable in a BC way, I'm marking this as "patch". |
|
Hi, thank you for the fast feedback. I change it to be compatible to php 5.3. Greetings |
| @@ -272,7 +271,7 @@ protected function buildForm(AdminObjectAclData $data, FormBuilderInterface $for | |||
| ); | |||
| } | |||
|
|
|||
| $formBuilder->add($key, new AclMatrixType(), array('permissions' => $permissions, 'acl_value' => $aclValue)); | |||
| $formBuilder->add($key, 'Sonata\\AdminBundle\\Form\\Type\\AclMatrixType', array('permissions' => $permissions, 'acl_value' => $aclValue)); | |||
There was a problem hiding this comment.
Double \ is not really necessary IMO and not used on the rest of the project.
The goal is not only to make it compatible to php 5.3 but compatible with older version of Symfony. |
|
I add some line breaks and a check auf sf major_version. Greetings. |
| @@ -271,8 +271,19 @@ protected function buildForm(AdminObjectAclData $data, FormBuilderInterface $for | |||
| 'attr' => $attr, | |||
| ); | |||
| } | |||
|
|
|||
| $formBuilder->add($key, new AclMatrixType(), array('permissions' => $permissions, 'acl_value' => $aclValue)); | |||
| if (3 < Kernel::MAJOR_VERSION) { | |||
There was a problem hiding this comment.
Is the kernel version safe enough as a check?
There was a problem hiding this comment.
Absolutly not. This is from the Kernel component and we are working with the form one.
@Turko Please use method_exists.
|
Please also rebase to have StyleCI green. |
|
According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning. If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch. So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one). Regards. |
|
Hi Guys, sorry, but I don't see a way to do this check with out checking the kernel. I understand, that symfony forms are not depending on the kernel, but I didn't find a better way. We are talking about a breaking change in Symfony Forms that now expects a string where before it expected an Object (as a second param). I don't see how method_exists can help me on this one. I really believe that you should carefully evaluate if you want to support in version 3.x symfony 2.3. |
Here a sample of
See discussion here: sonata-project/dev-kit#51
Don't forget to rebase and open another PR for |
FormBuilder expects a string and not an object