Class based view update for FormPreview

[Ryan Kaskel]

I used the FormPreview class in django.contrib.formtools.preview for the first time this week. After working with class based views for a while, I really missed methods like get_form for custom form instantiation. Recently, the form wizard machinery (which is much more complex) got a class-based views update (#9200) so I decided to update the FormPreview class as well. It uses django.views.generic.FormView and it's on a Django branch on my Github account. (​https://github.com/ryankask/django/blob/cbv-formpreview/django/contrib/formtools/preview.py -- I've also attached a diff based on the Django Github mirror which I am pretty sure is acceptable.

In order to maintain 100% compatibility, there are some issues to note:

• The original class used get_context whereas the class based views use get_context_data. I've made it so both can be used but if backwards compatibility can be broken, it would be better to only use get_context_data.
• The original class used __call__ so it could be easily instantiated with a form in a urlconf. I moved the important code to dispatch and __call__ just returns a call to dispatch.
• The failed_hash method, which isn't documented, falls under a section in the code which says "METHODS SUBCLASSES MIGHT OVERRIDE IF APPROPRIATE." I left the ability to call that method and it just wraps post and sets the current stage to preview.
• get_initial, process_preview, and security_hash, which all fall under the "override if you need to" section only documented in the code, used to take a request as an argument. They all still do. The methods that were marked "don't touch" now use the instance's request.

If backwards compatibility doesn't need to be maintained, the code can be cleaned up a little more.

Anyway, any comments on the patch would be much appreciated and I'd update the docs if devs think it's okay.

By the way, if none of this is acceptable, the test formtools.PreviewTests.test_form_submit_bad_hash needs to be fixed (it is in my patch). self.client.post('/previewpreview/', self.test_data) results in a 404 so the next assertion that some text isn't in the template passes when it shouldn't.

[AndrewIngram]

I'm wondering whether it's more suitable for this to be implemented as a mixin class that can be easily used with CreateView and UpdateView, eg:

class MyCreateView(CreateView,PreviewMixin):
    pass

[Ryan Kaskel]

I've updated the patch to remove the unneeded post override.

A mixin is an interesting idea. The only issue is getting the form_valid methods the play well with each other as those views are based on the ModelFormMixin. I'd love to hear some thoughts on that.

[Ryan Kaskel]

I've updated the patch again. Because our site needs it now, I've created a package for it if anyone is interested (django-cbv-formpreview - ​https://github.com/ryankask/django-cbv-formpreview).

I will write improved documentation this weekend.

[Ryan Kaskel]

I've updated the docs.

The basic question is does backwards compatibility need to be maintained? And if so, to what extent.

[Ryan Kaskel]

The more I use this in production, the more I realize that backwards compatibility would need to break. For example, the done method takes cleaned_data but it would be much better if it took the form object itself so you could, for example, call save.

I am going to break backwards compatibility in my fork (django-cbv-formpreview - ​https://github.com/ryankask/django-cbv-formpreview) and see where that takes me.

[Jacob]

Milestone 1.4 deleted

[Jannis Leidel]

FWIW, I think we should consider providing a class based form preview view that has the ability to take advantage of as much class based views API as possible. In that sense it's possible to introduce it without keeping in line with the previously shipped form preview feature. The simplest way to do this is to ship it next to the current implementation like we are going to do with the form wizard. Deprecation of the current form preview would start as soon as the new class based form preview is included.

[Ryan Kaskel]

That approach is definitely best.

I think some sort of mixin as AndrewIngram mentioned would be ideal.

My initial thoughts are thoughts are that a mixin would be a bit brittle because its post method would always need to be resolved first with respect to the inheritance tree.

Another option would be to replace the ProcessFormView and provide new bases for the create, update, and delete views in the new form preview package.

Or maybe I'm just not thinking outside the box ;) I will have a look this weekend and report back.

[Greg Chapple]

formtools has been extracted into its own repository (​​​​https://github.com/django/django-formtools/). Because of this, the issue tracking for this package has been moved to GitHub issues. I'm going to close this ticket, but I've created a GitHub issue to replace it where the conversation can continue: ​​​​https://github.com/django/django-formtools/issues/19. Thanks!