contributte / forms-multiplier

:repeat: Form multiplier & replicator for Nette Framework
https://contributte.org/packages/contributte/forms-multiplier.html
MIT License
26 stars 20 forks source link

Issue with the input type returned from multiplier since d51e6d4fb1d05fa01a6c2e1cb6d6f150c4e7e756 #68

Closed jakubvojacek closed 4 months ago

jakubvojacek commented 3 years ago

Hello

we were using master#d51e6d4fb1d05fa01a6c2e1cb6d6f150c4e7e756 without problems, today, after upgrading to 3.2.0, multiplier started to return integers as strings (when there is an integer input added via $form->addInteger). I quickly looked at the code but did not find the place where this bug was introduced.

Can you please check?

Thank you Jakub

f3l1x commented 3 years ago

Hi, please provide some code or better failing tests to reproduce it. Thanks.

jakubvojacek commented 3 years ago

I did some digging and it was actually commit https://github.com/contributte/forms-multiplier/commit/45ed76a7ed22c9b97048441eccd4a7fb2f8f2d6f, method validate (https://github.com/contributte/forms-multiplier/commit/45ed76a7ed22c9b97048441eccd4a7fb2f8f2d6f#diff-bbd929c6f77fe505e5255934192003ffef1ca43d2ecd1b908083449269892acaR206)

by changing line

$controls = $controls ?? iterator_to_array($this->getComponents(false, Control::class));

to

$controls = $controls ?? iterator_to_array($this->getComponents(true, Control::class));

or 

$controls = $controls ?? iterator_to_array($this->getComponents());

it starts to work as expected

f3l1x commented 3 years ago

Could you please send a PR? Excellent digging.

jakubvojacek commented 3 years ago

certainly, which of the two options I found that work you prefer?

solcik commented 3 years ago

@f3l1x @jakubvojacek

Yeah, I am on the master and I have this problem also.

It was introduced in this commit as mentioned.

https://github.com/contributte/forms-multiplier/commit/45ed76a7ed22c9b97048441eccd4a7fb2f8f2d6f#diff-bbd929c6f77fe505e5255934192003ffef1ca43d2ecd1b908083449269892acaR206

My comment - I think there should be C variant without parameter as it was.

https://github.com/contributte/forms-multiplier/pull/69#pullrequestreview-617062684

f3l1x commented 3 years ago

Can you guys test dev-master? Thx.

solcik commented 3 years ago

@f3l1x afk, I am going to deploy it on Friday.

jakubvojacek commented 3 years ago

@f3l1x all tests passed with dev-master, seems OK, thanks 🙏

jtojnar commented 4 months ago

Turns out this was because the Multiplier component’s direct subcomponents are actually Containers, the Controls (other than Add Submitter) are nested. I have opened https://github.com/contributte/forms-multiplier/pull/103 switching to the correct filter.