Opened 17 years ago

Closed 17 years ago

Last modified 17 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: 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)

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

Download all attachments as: .zip

Change History (4)

by shaunc <shaun@…>, 17 years ago

Attachment: save_instance-patch.patch added

patch w/ fix

comment:1 by Chris Beaven, 17 years ago

Resolution: invalid
Status: newclosed

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

in reply to:  1 comment:2 by tobias@…, 17 years ago

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 by Chris Beaven, 17 years ago

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