#5780 closed (fixed)
pass the created/updated object to formsets in edit_inline for validation
Reported by: | Honza Král | Owned by: | Brian Rosner |
---|---|---|---|
Component: | contrib.admin | Version: | newforms-admin |
Severity: | Keywords: | ep2008 | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When validating InlineFormset
it is essential that you have access to the created/updated object. I created a patch that handles exactly that - it creates the object (without saving it) and passes it to the FormSet
.
Attachments (3)
Change History (16)
by , 17 years ago
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 17 years ago
Keywords: | nfa-blocker added |
---|
Not sure if this is critical enough prior to a merge, but marking it nfa-blocker anyway. Joseph, please correct that if it can be deferred until later.
comment:3 by , 16 years ago
milestone: | → 1.0 alpha |
---|
by , 16 years ago
Attachment: | 5780-against-7875.patch added |
---|
comment:4 by , 16 years ago
Keywords: | ep2008 added |
---|
comment:5 by , 16 years ago
there is problem with models.ImageField
and this patch
=> file is saved twice on storage (one original name and one with underscore ;))
follow-up: 8 comment:6 by , 16 years ago
Hey Honza... I may be dense here, but if you're using commit=False when saving the forms, where do the objects get saved and will m2m fields be saved? I wonder if we could use transaction.commit_manually here and just roll the transaction back if any of the formsets fail validation. Thoughts?
comment:7 by , 16 years ago
Replying to jkocherhans:
Hey Honza... I may be dense here, but if you're using commit=False when saving the forms, where do the objects get saved and will m2m fields be saved? I wonder if we could use transaction.commit_manually here and just roll the transaction back if any of the formsets fail validation. Thoughts?
Hi Joseph, the transaction thing should work for the main issue, but there is still the issue of some custom non db-related validation that could be done in formsets.
The object is saved in .save_add()
and .save_change()
- there is
new_object = form.save(commit=True)
I haven't touched that but we could pass in the obj
as well and just call
obj.save() form.save_m2m()
follow-up: 9 comment:8 by , 16 years ago
Replying to jkocherhans:
Hey Honza... I may be dense here, but if you're using commit=False when saving the forms, where do the objects get saved and will m2m fields be saved? I wonder if we could use transaction.commit_manually here and just roll the transaction back if any of the formsets fail validation. Thoughts?
I am the dense one here - the transaction-based approach would work (with no problems), if you are willing to sacrifice MySQL MyISAM to get there ;). That's your call
by , 16 years ago
Attachment: | 5780-against-7925.patch added |
---|
comment:9 by , 16 years ago
Replying to Honza_Kral:
Replying to jkocherhans:
Hey Honza... I may be dense here, but if you're using commit=False when saving the forms, where do the objects get saved and will m2m fields be saved? I wonder if we could use transaction.commit_manually here and just roll the transaction back if any of the formsets fail validation. Thoughts?
I am the dense one here - the transaction-based approach would work (with no problems), if you are willing to sacrifice MySQL MyISAM to get there ;). That's your call
Grrr... if only I had a time machine so I could go back and... No. I didn't think about that. It isn't an acceptable tradeoff. Damn my nice tools altering my perception of reality! :)
comment:10 by , 16 years ago
Keywords: | nfa-blocker removed |
---|---|
milestone: | 1.0 alpha → 1.0 beta |
Dropping nfa-blocker as this will be dealt with once merged with trunk. Also pushing to 1.0 beta since this will likely change the API it needs to be looked at pre-1.0. See http://groups.google.com/group/django-developers/browse_thread/thread/a2d8baf9b6846649
comment:11 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Sounds reasonable. I'll take this into account when I'm rewriting the admin save code (which I'll need to do at some point.)