Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

BC break introduced in 2.8.4 #102

Closed
davidwindell opened this issue Jul 8, 2016 · 12 comments
Closed

BC break introduced in 2.8.4 #102

davidwindell opened this issue Jul 8, 2016 · 12 comments

Comments

@davidwindell
Copy link
Contributor

davidwindell commented Jul 8, 2016

I've just been embarrassed on live site (yes, test case added now ;) by a BC break that was introduced in 2.8.4.

This was caused by #19.

Because empty array's are now sent to the hydrator for a Collection (even when the collection's key is not in the validationGroup) all values in that collection are being wiped out. This was not the case in 2.8.3.

I'm not sure at the moment what the solution is as I'm still trying to get my head around why #19 was done.

Thoughts please @weierophinney @svycka @bakura10

@davidwindell
Copy link
Contributor Author

What you are saying here is that because nothing was posted for a collection element, you should pass that to the hydrator as an empty array.

I have many forms where we don't display the collections on the same page (and thus they are not posted - but should never be deleted like this).

How should we work around this?

@svycka
Copy link
Contributor

svycka commented Jul 8, 2016

@davidwindell can you show example what does not work because I not fully understand where is the problem I thought that inputFitler is responsible for validationGroup but I guess I was wrong :(

@davidwindell
Copy link
Contributor Author

@svycka the idea of the validation group is that if you omit a field (or say a collection) then that data won't be validated or then consequently hydrated.

What's happening now is even when a collection is explicitly not included in the validationGroup it is being sent through with the data to the hydrator as empty.

I'll push up a breaking test.

@svycka
Copy link
Contributor

svycka commented Jul 8, 2016

@davidwindell you are right this really does not work with validationGroup, but I am not sure how to fix it yet.
@weierophinney any suggestions? maybe we can pass validation group as optional second param in bindValues()? would it be BC break?

@davidwindell
Copy link
Contributor Author

Please see #103 for a test case that passes in 2.8.3 but not 2.8.4+

@zluiten
Copy link
Contributor

zluiten commented Jul 11, 2016

I'm having the same issue.

weierophinney added a commit that referenced this issue Sep 14, 2016
…-of-collections-fix

Hotfix for #102 BC break validationGroup ignored for collections
weierophinney added a commit that referenced this issue Sep 14, 2016
@popovserhii
Copy link

popovserhii commented Sep 17, 2016

New problem is raised after this fix.
I have big form with nested fieldset collections
InvoiceForm->InvoiceFieldset (base) -> InvoiceProductFieldset (collection) -> QuantityItemFieldset (collection)

I set group validation like this

$form->setValidationGroup([
    'invoice' => [
        'invoiceProducts' => [
            'quantityItems' => [
                'id',
                'quantity'
            ]
        ]
    ]
]);

When I called $form->isValid() post data hydrate to entity successfully.
This scenario has been worked correct to 2.9.1 release.
Now, when I call this code it simply iterate through base fieldset elements and nothing to do with nested fieldsets.

@michalbundyra
Copy link
Member

@popovsergiy Could you open separate issue and provide a test, which pass on version <2.9.1 and fails on 2.9.1? Thanks!

@davidwindell
Copy link
Contributor Author

Yes, I'm experiencing this too.

@svycka
Copy link
Contributor

svycka commented Sep 19, 2016

can someone create a test case for this? @davidwindell ?

@popovserhii
Copy link

@svycka, I will try create test but not sure what to do after. Simply create PR?

@svycka
Copy link
Contributor

svycka commented Sep 19, 2016

yep, I would like to see where it fails and maybe we can fix it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants