#4553 closed (invalid)
newsforms.models.save_instance doesn't save references to related correctly
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Keywords: | ||
Cc: | shaun@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The current line 36:
setattr(instance, f.name, cleaned_data[f.name])
will save integer primary key to field for relation *object* not the underlying field for the id
Works if replaced with:
attname = opts.get_field( f.name ).get_attname()
setattr(instance, attname, cleaned_data[f.name] )
Attachments (1)
Change History (4)
by , 17 years ago
Attachment: | save_instance-patch.patch added |
---|
follow-up: 2 comment:1 by , 17 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
That's because cleaned_data
should contain the object itself, not the primary key value.
comment:2 by , 17 years ago
Cc: | added |
---|
Replying to SmileyChris:
That's because
cleaned_data
should contain the object itself, not the primary key value.
Why? This could easily be done automatically and without this functionality I need to write clean methods for each of my foreign key fields. I don't think it's bad to assume that, given an integer in a foreign key field, it is in fact a foreign key to the object I'm looking for.
comment:3 by , 17 years ago
Cc: | removed |
---|
In newforms.models
, ModelChoiceField
's clean()
method cleans to the object itself - this is why the assumption is made.
There's the possibility I'm not getting why this change is necessary. Perhaps an example (or even better, tests, because that's what the patch would need anyway) would help me understand your point of view. Alternatively, bring this up in django-dev (mentioning this ticket) and try to clarify your point.
Thanks! :)
PS: you don't need to cc me, I get emailed anyway
patch w/ fix