Code

Opened 6 years ago

Closed 3 years ago

#7222 closed New feature (wontfix)

FormPreview should pass the form to done()

Reported by: Samuel Cormier-Iijima <sciyoshi@…> Owned by: nobody
Component: contrib.formtools Version: master
Severity: Normal Keywords: formpreview done cleaned_data form
Cc: sciyoshi@…, bthomas@… 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

Currently, FormPreview's done() gets passed the request and the form's cleaned_data, but it'd be much nicer if done() got passed the actual form instead. This way, for ModelForms, one could just do form.save(). You can still get the cleaned_data with form.cleaned_data. Attaching a patch that breaks backwards compatibility, but I think its nicer in the long run...

Attachments (2)

preview.diff (2.3 KB) - added by Samuel Cormier-Iijima <sciyoshi@…> 6 years ago.
preview.2.diff (2.7 KB) - added by Samuel Cormier-Iijima <sciyoshi@…> 6 years ago.
forgot to change done() parameter

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by Samuel Cormier-Iijima <sciyoshi@…>

Changed 6 years ago by Samuel Cormier-Iijima <sciyoshi@…>

forgot to change done() parameter

comment:1 Changed 6 years ago by brosner

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from newforms-admin to SVN

This has nothing to do with newforms-admin. :)

comment:2 Changed 6 years ago by programmerq

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 6 years ago by bthomas

  • Cc bthomas@… added

I agree that this would be useful. In comparison, the form wizard passes a list of form instances to its done() method. This would bring a little more consistency to formtools.

I had some issues with using cleaned_data instead of the form, but I was able to hack around them by putting some completely inappropriate data in cleaned_data.

If this is accepted, should it go into 1.0? It seems that it would be better to break backwards compatibility now rather than after 1.0.

comment:4 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 3 years ago by jacob

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed
  • UI/UX unset

Yeah, we clearly missed getting this in before 1.0!

Adding it now would indeed introduce a backwards-incompatible change, and I can't see a good way around that. Thus, marking wontfix: the slight improvement in usability doesn't warrant breaking everyone's existing code.

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.