django-formtools: Infinite Recursion possible in 2.4

Please revert 533a83053, which introduced a major bug and was not a proper solution for the new feature request (to dynamically add forms that aren’t in the initial form_list). For those that require this new feature, please provide a working example of your use cases and how you were doing this prior to 2.4. A proper API needs to be created and tested for this new feature request, without breaking existing documented functionality. That has not been done yet. A proper API shouldn’t require any hacks around infinite recursion errors, it should be designed so these errors are impossible.

I’ve just submitted a PR that attempts a more complete fix for this problem. Please have a look and give it a test if you’ve got time.

I’m confused about why people are advocating for this change to be reverted “ASAP”. Is there a reason why affected people can’t stick with 2.3 until a comprehensive solution for this issue is found?

A reason could be that version 2.4.0 added official support for django v4

Maybe a version 2.3.1 could be released with the old behaviour and official django v4 support? So we can pin to <2.4 until the issue is somehow resolved and upgrade/use django v4.

The latter - to come up with a proper solution.

@martey, it should be reverted because 2.4 is a broken release that breaks real projects and costs people valuable time and money. It’s not very reasonable to expect everyone to find out the hard way that 2.4 is broken, find this issue, and then downgrade to 2.3, when reverting could avoid all of that. To be clear, we don’t need to come up with a comprehensive solution for the infinite recursion bug. The changeset that introduced the bug simply needs to be reverted. Once that is done, a comprehensive solution to the feature request (to be able to dynamically add forms that are not present in form_list) would need to be developed, documented, and tested before being included in a future release.

@schinckel, are you saying you don’t have time for reverting the commit or just that you do not have time to work on a proper solution for the feature request? There is already #222 for reverting the change. In my opinion, 2.4 should be yanked from PyPI in favor of the next release. I could submit a PR to update the changelog. Let us know how we can help. Thanks!

The fix of #168 has caused this problem. My understanding is that you should be using the conditiion_dict to vary which steps are in the wizard and which are not. I can see why overriding get_form_list() would seem like a nice alternative (and maybe simpler to use), but by changing get_form() to use get_form_list() and solve the request of #168, this makes it impossible to have conditions which look at prior form steps as part of their conditions and therefore breaks what is probably one of the most common use-cases of this package.

My opinion is that 533a83053deab2adeb72de079e813aae78b03c1a should be reverted as it tries to allow the same problem to be solved in a new, previously-not-possible, way and breaks the original way of solving the same problem that’s been part of this great package for years.

Thanks for your help, we’ve also had to pin 2.3 due to this being a breaking change for our use of the package.

Yeah, that looks okay to me.

The original use case was to dynamically inject forms: specifically given the input from step A, run n instances of step B with different data, as “sub-steps”.

And in an ideal world, SemVer would be used so people could understand version numbers and use modern package managers like Poetry properly. This being a breaking change could then be safely introduced using version 3.0.0 instead of 2.4.0, if it was wanted (not that I’m sure it should be tbh). Having come from other languages to Python in the last few years, the Python community seems quite out of sync with the rest on the SemVer front and SemVer is really damn handy when managing dependencies.

I’ll have a stare at this later + try and understand it (and invite everyone else to) - I (or someone else), should add a test to this for modifying get_form_list() as well so we have both cases.

EDIT: Thanks for making an actual test; I’m pressed for free time, so it’s really helpful to have a working example to start from.

I agree that a project-wide Django setting is too broad, but this could be fixed by adding a dynamic_form_list variable to WizardView that defaults to False.