search

contributte / forms-multiplier

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

Fixes for nested multipliers #59

Closed Kocicak closed 8 months ago

Kocicak commented 3 years ago

Hello, there is a problem with nested multipliers, if the top-one has minCopies=0 and the nested one has minCopies>0. Then it will not create any copies, because form has been submitted (condition on line 260). With this, there is also a bug - when removing copies, counter is not decreased.

With this two changes, nested multipliers with minimum copies does work.

Kocicak commented 3 years ago

The second commit fixes setting default values for nested multipliers.

f3l1x commented 2 years ago

Can you please @Kocicak add some tests? I am not aware of this problem and need to be sure it's working. :-)

Kocicak commented 2 years ago

Can you please @Kocicak add some tests? I am not aware of this problem and need to be sure it's working. :-)

Unfortunately not anytime soon. It is one year from this merge request.

I remember that debugging this was not easy. The main understanding is that Multiplier creates minimum copies only if form is not submitted (condition on line 260). But this is not entirely true for nested multipliers if top-level one has 0 minimum and nested one has 1 minimum copy. When creating first copy of top-level multiplier, it assumes form is submitted (because it is) and because of that condition on line 260, nested multiplier does not create minimum copies.

We are using forked multipler successfully for a year now and it is working fine, also with default values. That is all I can say about this, but I understand that test should be needed. When I get to it, I will write them, but it may take another year :)

f3l1x commented 2 years ago

No problem. Do it right takes time. :-) If you need my help, just let me know.

Stancek commented 1 year ago

This BUG is still in package. And this fix helped. Problem is when nested multiplier have > 0 min copies and root has min = 0. On adding new root multiplier then nested is not created with min copies.

Firstly tested on v3.2.0 then tryied with some changes on 3.3.1 still working correctly.

Kocicak commented 1 year ago

Hi @Stancek, thank you for confirming that this pull request works.

If you would like to write tests so @f3l1x can merge this pull request to the original package, it would be really apprecieted. But I understand if not, we can always use our forks.

f3l1x commented 8 months ago

Hey @Kocicak and @Stancek. Can you please rebase this PR? I tried a lot but I still failing. After that I will merge it without tests.

Kocicak commented 8 months ago

Hi, I have rebased source branch (bugfix/...). It was indeed not easy, there were many changes and code refactoring, but I think I got it all. After some testing everything seems to be working (PHP 8.1 / Latte 3).

I don't know about unit tests, if they pass or not.

f3l1x commented 8 months ago

Thank you.