Fix for #6702: ModelForm now checks instance on __init__ #296

Closed
wants to merge 2 commits into from

2 participants

@selwin

Hi there,

Attached is a patch and testcase that fixes https://code.djangoproject.com/ticket/6702 . Let me know if there's anything else that needs changing.

Best,
Selwin

@apollo13 apollo13 and 1 other commented on an outdated diff Aug 31, 2012
tests/regressiontests/forms/tests/models.py
@@ -114,6 +114,10 @@ def test_unicode_filename(self):
self.assertEqual(m.file.name, 'tests/\u6211\u96bb\u6c23\u588a\u8239\u88dd\u6eff\u6652\u9c54.txt')
m.delete()
+ def test_incorrect_instance_raises_exception(self):
+ # ModelForm with incorrect instance raises ValueError on init ##########
@apollo13
Django member

What's with the hashes (#) at the end?

@selwin
selwin added a note Aug 31, 2012

The tests surrounding it have comments ending with multiple hashes so I was just following those (Lines 109 and 122). Want me to remove them?

@apollo13
Django member

Yes that would be great (also the other occurences in that file). Btw Malcom said the assert was okay: https://code.djangoproject.com/ticket/6702#comment:3 -- Why do you use an insinstance check in your patch?

@selwin
selwin added a note Aug 31, 2012

@apollo13 I just updated my branch to clean the extra hashes. I raised ValueError because it's more consistent with the rest of ModelForm (raises ValueError when no model is specified and when save is called when the form doesn't validate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member

Ticket has been marked as "won't fix."

@timgraham timgraham closed this May 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment