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

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

Closed
jakubvojacek opened this issue Mar 10, 2021 · 9 comments
Labels

Comments

@jakubvojacek
Copy link
Contributor

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
Copy link
Member

f3l1x commented Mar 10, 2021

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

@jakubvojacek
Copy link
Contributor Author

I did some digging and it was actually commit 45ed76a, method validate (45ed76a#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
Copy link
Member

f3l1x commented Mar 16, 2021

Could you please send a PR? Excellent digging.

@jakubvojacek
Copy link
Contributor Author

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

@solcik
Copy link
Member

solcik commented Mar 21, 2021

@f3l1x @jakubvojacek

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

It was introduced in this commit as mentioned.

45ed76a#diff-bbd929c6f77fe505e5255934192003ffef1ca43d2ecd1b908083449269892acaR206

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

#69 (review)

@f3l1x
Copy link
Member

f3l1x commented Mar 31, 2021

Can you guys test dev-master? Thx.

@solcik
Copy link
Member

solcik commented Mar 31, 2021

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

@jakubvojacek
Copy link
Contributor Author

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

@jtojnar
Copy link
Collaborator

jtojnar commented Mar 21, 2024

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 #103 switching to the correct filter.

@jtojnar jtojnar closed this as completed Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants