Django

Code

Ticket #7682 (closed: fixed)

Opened 5 months ago

Last modified 5 months ago

edit_inline issues error(s) incorrectly when going from revision 7710 to TRUNK

Reported by: jleingang Assigned to: mtredinnick
Milestone: Component: django.contrib.admin
Version: SVN Keywords: edit_inline validation
Cc: ristretto.rb@gmail.com, puyb@puyb.net Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

models referenced here: http://dpaste.com/61474/

Users edit the memberpledge in the admin interface; when saving this validation error occurs:

{'billinginfo.0.id': [u'Billing Information with this ID already exists.']}

Attachments

7682.diff (0.8 kB) - added by Karen Tracey <kmtracey@gmail.com> on 07/09/08 10:30:09.

Change History

07/09/08 00:02:57 changed by anonymous

  • cc set to ristretto.rb@gmail.com.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

07/09/08 01:42:50 changed by Karen Tracey <kmtracey@gmail.com>

  • stage changed from Unreviewed to Accepted.

This bug seems to have been introduced in r7790. Specifically I think the changes to how unique is calculated in django.db.models.__init__.py around line 170 is causing the manipulator_validator_unique validator to be associated with the hidden input primary key fields for inline-edited objects, but that validator looks up the primary key in the parent object's table, not the inline-edited object's table, so its results are incorrect.

07/09/08 01:43:19 changed by Karen Tracey <kmtracey@gmail.com>

(Prior to r7790 these fields had no validators associated with them.)

07/09/08 04:58:00 changed by puyb@puyb.net

  • cc changed from ristretto.rb@gmail.com to ristretto.rb@gmail.com, puyb@puyb.net.

As Karen Tracey say, the problem seems to be in db/models/fields/init.py at line 329

Before r7790, on the id field, the unique attribute return false even if the field was a primary key. r7790 correct this, but introduce the bug. Am i wrong ?

I changed the condition on line 329 from :

if self.unique or (self.primary_key and not rel):

to :

if not rel and (self.unique or self.primary_key):

It work for me, but, i don't know if this may not create some problems when you have a related object with a unique field (other than the primary key). This may need more tests.

07/09/08 09:28:24 changed by Karen Tracey <kmtracey@gmail.com>

Having an inline-edited object with unique=True specified on a field is a known problem. See #565 and at least six other tickets marked as dups.

Another solution to this problem is to switch to newforms-admin, which doesn't use the old manipulator/validator framework. Ordinarily I'd say old admin problems aren't worth fixing but this is a recently introduced bug that entirely breaks inline editing on trunk, so unless newforms-admin is merging Today (which it might be, I thought the merge target was before the first sprint, which is coming up this weekend, right?) this seems worth fixing.

07/09/08 09:42:48 changed by mtredinnick

  • owner changed from nobody to mtredinnick.

I'll look at this (although it will be 12 - 24 hours before I get to it), since it was my change that did it. My inclination, though, is that given the change in question fixed some major problems and given that newforms-admin is so close, people really needing edit_inline support can manage to not "svn update" for a few days without the universe grinding to a halt. So if it's not trivial to fix, it might just be hard cheese until the merge. That said, I will look at it. It might be as easy as indicated in the above comment.

07/09/08 10:30:09 changed by Karen Tracey <kmtracey@gmail.com>

  • attachment 7682.diff added.

07/09/08 10:43:19 changed by Karen Tracey <kmtracey@gmail.com>

  • has_patch set to 1.

So I probably overstated how soon nfa needs to land to make this worthwhile fixing, but I'm impatient for that merge to happen and a bit at a loss to know how to help it along. FWIW the oneliner change (which I've attached as a patch) posted by puyb fixes the case I tracked down and allows me to freely switch my own app (which uses inlines and non-default-named ID fields, which caused a different error when the validator was attached to the hidden fields) between nfa and trunk admin as I've been doing for testing purposes for the last several months. It seems harmless but I'll admit it's in code that I've assiduously avoided learning anything about since it's supposed to be going away real soon now.

07/11/08 02:34:11 changed by mtredinnick

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

(In [7882]) Fixed #7682 -- Added a work around to keep existing admin alive until newforms-admin merges (to handle changes in [7790]).

This code becomes irrelevant in a non-oldforms world, so even if it's not quite correct, it will do for now. Based on a patch from puyb@puyb.net and Karen Tracey.


Add/Change #7682 (edit_inline issues error(s) incorrectly when going from revision 7710 to TRUNK)




Change Properties
Action