Code

#19189 closed New feature (wontfix)

FormWizard done method can't revisit forms

Reported by: kenth Owned by: nobody
Component: contrib.formtools Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The FormWizard done method is used for processing information collected by the form steps. However, it is not currently possible to revisit the forms to correct a problem which occurs in the done step. To extend the documentation example, it is possible to collect and validate a credit card (or other payment method) in a form step, but have the card be 'declined' (eg exceeds limit) when processing in done. A reasonable processing flow in this case would be to display the error, and revisit the payment page for a different payment method. However, the current implementation of FormWizard unconditionally clears its storage after calling done.

I propose that the done method be allowed to raise a (newly defined) RevalidationError to cause the Wizard to revisit a form step instead of clearing storage.

Attachments (2)

ticket_19189.diff (5.7 KB) - added by kenth 21 months ago.
Patch with tests
wizard-subclass.py (932 bytes) - added by kenth 21 months ago.
implementation w/o patching Django

Download all attachments as: .zip

Change History (8)

comment:1 Changed 21 months ago by kenth

  • Component changed from Uncategorized to contrib.formtools
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Type changed from Uncategorized to New feature
  • Version changed from 1.4 to master

comment:2 Changed 21 months ago by russellm

  • Resolution set to needsinfo
  • Status changed from new to closed

The intended contract for done() is that at point of entry, all the forms are valid. If there is validation to be performed, it should be performed in the clean methods for the previous forms.

To follow through your example -- if one of the things you need to do for a valid form is to check the credit card, why isn't this being done as part of the cleaning for the form that took the credit card details? By your own admission - the right response if the card is declined is to re-show the credit card page, so why should the already existing clean method handle this?

Closing needsinfo; feel free to reopen if you can explain why this *can't* (not just "I don't want to") be handled as part of the form's clean method.

Changed 21 months ago by kenth

Patch with tests

comment:3 Changed 21 months ago by kenth

  • Has patch set
  • Resolution needsinfo deleted
  • Status changed from closed to reopened

Thanks for reviewing the ticket. I found the need to perform "revalidation" in the done step in an e-commerce project incorporating validation using stripe.com which separates validation (performed in the form's clean method) and charging (which I perform in done). However, I think the problem is more general than this specific case.

I had rejected performing a 'charge' in the clean of a confirmation step, instead preferring the done method for two reasons:

  1. The wizard validates each form at least twice (when the form is initially validated and again in render_done). To ensure an action is performed only once, the clean method needs to leave a token to not repeat the action. This can be fragile.
  1. The wizard does not revalidate earlier steps when first validating a form. Thus, if a confirmation step's clean charges when first validated, and an earlier step's form fails revalidation in render_done (say stock on an item is no longer available, or a discount window has expired), the charge has already occurred and the done step will not be processed. The transaction is a mess at that point.

An alternate solution would be to have earlier forms check if the charge has occurred and always return valid() in that case. I'm not sure how that would interact with tampering detection and the like.

Accordingly actions which must not be duplicated are best performed in the done method. The done method guarantees all forms validate & the wizard state is deleted upon completion. The current wizard allows for forms which do not revalidate, but does not allow for a done method which may not complete successfully. The proposed exception handles this case.

An implementation I currently use defines the Exception in my wizard subclass & then overrides render_done to catch the exception and step back into the FormWizard as needed. Django being open-source and Python, the workaround is quite functional. However I thought the problem and solution general enough for inclusion.

I have attached a patch w/ tests. I can work up the docs if it would help.

Thanks. Kent

Changed 21 months ago by kenth

implementation w/o patching Django

comment:4 Changed 20 months ago by russellm

  • Triage Stage changed from Unreviewed to Design decision needed

#19285 was a duplicate.

As I noted on that ticket, part of the design contract for the Form Wizard is that once validated, the form will be able to reliably save without error.

However, your use case is a bit more complex than the username case from #19285. However, I'm still not 100% convinced that this isn't a documentation/"fix it another way" issue. While your use case is certianly valid, I'm not sure hacking the wizard workflow is the appropriate fix.

Marking this as design decision needed, because it's a pretty big change to the way wizards work that needs some serious discussion.

comment:5 Changed 16 months ago by aaugustin

  • Status changed from reopened to new

comment:6 Changed 16 months ago by jacob

  • Resolution set to wontfix
  • Status changed from new to closed

Marking closed again; see Russ's comments. Kenth: if you'd like to revisit this further, please take it to the django-dev mailing list, thanks!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.