Revert unneeded changes from "Fixes for nested multipliers (#59)"

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

Revert unneeded changes from "Fixes for nested multipliers (#59)" #92

Closed jtojnar closed 2 months ago

jtojnar commented 5 months ago

This partially reverts commit 39725d333355b2a1ef21dac3948909cf1e5d4744, only keeping the part relevant for nested support and a PHPUnit deprecation fix.

The commit introduced a regression in testGroupManualRenderWithButtons so this will minimize the changes that could affect it.

Latte 2 macros are trivial to migrate and unclearly named, no need to keep them for BC.
The onSuccess handlers were already fixed in ed96ba0276a2db878841836c403f60568c7a3a98.
testGroupManualRenderWithButtons look like remnants of debugging effort, they do not fix the test.

For reference, the reverted change is the following commit that was squashed into 39725d333355b2a1ef21dac3948909cf1e5d4744, except for the change to testSendNested: https://github.com/contributte/forms-multiplier/pull/59/commits/2c33de22b8343ba08210764711e4776a5bf58227

f3l1x commented 5 months ago

Tests are still failing. :(

jtojnar commented 5 months ago

Yes, the primary reason is that webchemistry/testing-helpers is broken with latest Nette.

If you merge https://github.com/contributte/forms-multiplier/pull/96, that will fix it, and only two test failures will remain:

the one introduced by merging https://github.com/contributte/forms-multiplier/pull/83 before fix has been implemented
one broken by https://github.com/contributte/forms-multiplier/pull/59, as mentioned in the opening post

This pull request does not fix any tests, only cleans the code a bit. But spending most of the last weekend trying to decipher the changes in #59, I am now convinced the PR should be reverted wholesale, and the fix reimplemented.

I will try looking into it and into fixing #83 this weekend.

f3l1x commented 5 months ago

Thanks. The best thing would be drop webchemistry/testing-helpers at all. :-/

jtojnar commented 5 months ago

Do you have some replacement in mind? Otherwise, the simplest solution would probably be copying the used subset of it into the multiple tree and fixing it locally.

On Fri, 23 Feb 2024, 17:32 Milan Felix Šulc, @.***> wrote:

Thanks. The best thing would be drop webchemistry/testing-helpers at all. :-/

— Reply to this email directly, view it on GitHub https://github.com/contributte/forms-multiplier/pull/92#issuecomment-1961638008, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFMEYYO6ONP32IYB4OKR7TYVDACNAVCNFSM6AAAAABDNXPRP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRRGYZTQMBQHA . You are receiving this because you authored the thread.Message ID: @.***>

jtojnar commented 2 months ago

I became a maintainer of testing-helpers and fixed it there.

As for #59, I think it will be easier to revert it completely and re-implement it from scratch. Doing that in https://github.com/contributte/forms-multiplier/pull/110.