Code

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#4553 closed (invalid)

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

Reported by: shaunc <shaun@…> Owned by: adrian
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@…> 7 years ago.
patch w/ fix

Download all attachments as: .zip

Change History (4)

Changed 7 years ago by shaunc <shaun@…>

patch w/ fix

comment:1 follow-up: Changed 7 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

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

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

  • Cc SmileyChris 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 6 years ago by SmileyChris

  • Cc SmileyChris 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

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.