Django

Code

Ticket #9218 (closed: fixed)

Opened 4 months ago

Last modified 2 months ago

KeyError in validate_unique when primary_key is in fields and it's empty in form

Reported by: romke <rombar@gmail.com> Assigned to: kmtracey
Milestone: Component: Forms
Version: SVN Keywords: KeyError, validate_unique, custom primary_key
Cc: rombar@gmail.com, sime Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

1. create model with custom primary_key:

class Test(models.Model):
  testing = models.CharField(max_length=20, primary_key=True)

2. create form:

class TestForm(forms.ModelForm):
  class Meta:
    model = Test
    fields = ('testing')

3. POST EMPTY testing field

KeyError @ forms/models.py in validate_unique, line 235 will pop up

Attachments

key-error-on-empty-pk-field-in-forms-fix.patch (0.6 kB) - added by romke <rombar@gmail.com> on 09/26/08 13:17:18.
patch for this one
key-error-on-empty-pk-field-in-forms-fix-using-get.patch (0.6 kB) - added by romke <rombar@gmail.com> on 09/30/08 07:37:05.
patch using get(name) as suggested
runtests.log (13.5 kB) - added by romke <rombar@gmail.com> on 09/30/08 07:40:46.
tested as suggested
key-error-on-empty-pk-field-in-forms-fix-test.patch (0.8 kB) - added by romke <rombar@gmail.com> on 09/30/08 08:59:27.
test for fix

Change History

09/26/08 13:17:18 changed by romke <rombar@gmail.com>

  • attachment key-error-on-empty-pk-field-in-forms-fix.patch added.

patch for this one

09/26/08 13:30:53 changed by romke <rombar@gmail.com>

  • needs_better_patch changed.
  • has_patch set to 1.
  • needs_tests changed.
  • needs_docs changed.

09/29/08 21:55:20 changed by mtredinnick

  • needs_better_patch set to 1.
  • needs_tests set to 1.

I think this could be better written as self.cleaned_data.get(name), since that will return None when it's missing (and the following line won't execute if the name is missing from cleaned_data. So the code is a bit simpler.

Also, this should really have a test to make sure we don't reintroduce the problem. There's a regression tests file in the forms test directory.

09/29/08 21:55:32 changed by mtredinnick

  • stage changed from Unreviewed to Accepted.

09/30/08 07:37:05 changed by romke <rombar@gmail.com>

  • attachment key-error-on-empty-pk-field-in-forms-fix-using-get.patch added.

patch using get(name) as suggested

09/30/08 07:40:46 changed by romke <rombar@gmail.com>

  • attachment runtests.log added.

tested as suggested

09/30/08 07:46:52 changed by romke <rombar@gmail.com>

Sorry, Did not understood part about creating test for it. I'll attach test for that in few hours.

09/30/08 08:59:27 changed by romke <rombar@gmail.com>

  • attachment key-error-on-empty-pk-field-in-forms-fix-test.patch added.

test for fix

09/30/08 21:37:14 changed by romke <rombar@gmail.com>

  • needs_better_patch deleted.
  • needs_tests deleted.

better patch added, tests added

10/24/08 04:50:18 changed by julianb

My form didn't work at all. I can confirm that this patch fixes it. Thanks!

10/29/08 23:12:18 changed by sime

  • cc changed from rombar@gmail.com to rombar@gmail.com, sime.

11/05/08 13:22:49 changed by kmtracey

  • owner changed from nobody to kmtracey.
  • status changed from new to assigned.

Minor, but the attached patch uses 2-space indents while the rest of the code in the file uses 4-space -- in the future please code patches in the style of the surrounding code.

This error was actually fixed by the fix for #9039 but that fix didn't use the simpler-code get approach suggested above. I'll fix that and add a test for this to ensure it doesn't get broken again in the future.

11/05/08 13:55:51 changed by kmtracey

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [9342]) [1.0.X] Fixed #9218 -- Simplified the fix from #9039 and added tests to ensure this case doesn't break again (and that the simplification didn't break anything).

[9341] from trunk. Also updated svnmerge metadata.


Add/Change #9218 (KeyError in validate_unique when primary_key is in fields and it's empty in form)




Change Properties
Action