Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4553 closed (invalid)

newsforms.models.save_instance doesn't save references to related correctly

Reported by: shaunc <shaun@…> Owned by: Adrian Holovaty
Component: Forms Version: master
Severity: Keywords:
Cc: shaun@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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)

save_instance-patch.patch (508 bytes) - added by shaunc <shaun@…> 9 years ago.
patch w/ fix

Download all attachments as: .zip

Change History (4)

Changed 9 years ago by shaunc <shaun@…>

Attachment: save_instance-patch.patch added

patch w/ fix

comment:1 Changed 9 years ago by Chris Beaven

Resolution: invalid
Status: newclosed

That's because cleaned_data should contain the object itself, not the primary key value.

comment:2 in reply to:  1 Changed 9 years ago by tobias@…

Cc: Chris Beaven 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 Changed 9 years ago by Chris Beaven

Cc: Chris Beaven 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

Note: See TracTickets for help on using tickets.
Back to Top