search

jazzband / django-formtools

A set of high-level abstractions for Django forms
https://django-formtools.readthedocs.io
BSD 3-Clause "New" or "Revised" License
794 stars 135 forks source link

Fixing #220 (by reverting the breaking change) #222

Closed violuke closed 1 year ago

violuke commented 1 year ago

Please see #220, this is causing a lot of problems as a breaking change was accidentally introduced in 2.4.

It might be that there is a "better" fix in #221 or via another solution, but I think the priority should be to release 2.4.1 with this PR in and the original functionality fixed.

This would buy time to find a better solution that allows for the request of #168 to be implemented without causing the problems that the original change caused.

Thank you for this fantastic package.

spapas commented 1 year ago

Since there already is a solution of the problem (#221) I think the correct approach would be to go with that solution instead of reverting the change (which was on the correct direction). Peopl that are experiencing problems should not upgrade to 2.4 for now.

schinckel commented 1 year ago

I'm going to recommend merging this, because I don't have the time to commit to a proper solution (and it's very likely that nobody else is using the feature other than me at this stage).

robd003 commented 1 year ago

Any chance this can get merged in and cut a 2.4.1 release?

@schinckel @violuke

schinckel commented 1 year ago

I can't merge PRs.

violuke commented 1 year ago

I'd also love this merged, but I also cannot merge PRs. @claudep, @felixxm and @hramezani can you help?

hramezani commented 1 year ago

Sorry for the late response. I am not very familiar with the project TBH.

Agree that the fix is not a proper fix and cause some problem. Reverting the fixing might not be good because the other people who are using the feature might be hurt. I would suggest finding a better solution and meanwhile, you can pin the version to 2.3

Is there any important feature in 2.4 that this bug blocks you to upgrade and use it?

spapas commented 1 year ago

I would agree with @hramezani on this; releasing a new version without a proper fix would result to people that are using the new behavior having problems! In the previous version the bug was slipped under the rag and was introduced without anybody noticing.

Also, please notice that if you want to upgrade and have the old behavior you can simply override the get_form method of you WizardView to use self.form_list like already proposed in this comment:

https://github.com/jazzband/django-formtools/issues/220#issuecomment-1325485752

I know this is not ideal however it should allow you to upgrade until #220 is actually fixed.

claudep commented 1 year ago

Thanks for the patch, however hopefully 604a8ec5488c90c should fix the regression.