Code

Opened 8 years ago

Closed 7 years ago

#2415 closed defect (fixed)

AutomaticManipulator with foreign keys and unique_together fails

Reported by: aurynn@… Owned by: adrian
Component: Core (Other) Version: master
Severity: normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

When attempting to use model.AddManipulator() on a model with both unique_together and a foreign key reference, the AddManipulator fails with the following traceback.
Removing either the foreign key, or the unique_together, and .AddManipulator() proceeds without issue.

>>> import notes.models as models
b>>> blah = models.Note.AddManipulator()
Traceback (most recent call last):
  File "<console>", line 1, in ?
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.95-py2.4.egg/django/db/models/manipulators.py", line 75, in __init__
    self.fields.extend(f.get_manipulator_fields(self.opts, self, self.change, fol))
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/python2.4/site-packages/Django-0.95-py2.4.egg/django/db/models/related.py", line 116, in get_manipulator_fields
    if follow.get(f.name, False):
AttributeError: 'bool' object has no attribute 'get'
>>> 

The model in question is :

class Note(models.Model):
    
    id = models.AutoField(primary_key=True)
    parent = models.ForeignKey("self", blank=True, null=True, help_text='If left blank, a new top \
        level note will be created', related_name='self_parent' 
    )
    created = models.DateField(auto_now=True) # Each revision will be Now-Ified.
    
    company = models.ForeignKey(Company, blank=True, core=True, edit_inline=models.STACKED, num_in_admin=1, null=True, help_text='A company may be blank',validator_list=[ \
        myValidators.RequiredIfOtherFieldsNotGiven('parent','people')  ])
    
    people = models.ForeignKey(People, blank=True, null=True, help_text="People may be blank.",
        validator_list=[
            myValidators.RequiredIfOtherFieldsNotGiven('parent','company'),
        ]
    )
    
#   topic = models.CharField('Topic',maxlength=250, help_text='Short description of the note')
    note = models.TextField('Note', help_text='Bring on the info', validator_list = [validators.isNotEmpty('note',None),] )
    
    auth_user = models.ForeignKey(auth_models.User) # The creator of this note.
    
    sticky = models.BooleanField(default=False) # Doesn't need to be anything special
    
    note_id = models.IntegerField() # part of the revision ID field. Will generally be populated.
    revision = models.IntegerField() # the revision number. 
   
    def __repr__(self):
        return str(self.created) + ' - ' + self.note[35:] # A slice of the note, as a description.
    
    class _custom_meta:
        revise_on = ['note_id','revision']

    class Meta:
        db_table = 'note'
        unique_together = (('note_id','revision'),)

    class Admin:
        list_display = ('parent','created','company','people')
        js = (
            '/media/js/tinymce/jscripts/tiny_mce/tiny_mce.js',
            '/media/js/admin/textarea.js',
        )

Attachments (0)

Change History (7)

comment:1 Changed 8 years ago by aurynn@…

Further investigation reveals that the self-referential key is triggering the bug.

comment:2 Changed 7 years ago by jeroen@…

  • Resolution set to wontfix
  • Status changed from new to closed

This ticket is related to the "old forms" package and has not seen activity in 7 months. In the light of the migration to "new forms" it is unlikely that any resources are going to be spent on this one, so I close it as 'wontfix'.

comment:3 Changed 7 years ago by Ramiro Morales <rm0 _at_ gmx.net>

I get your point but I'm not sure closing the ticket is the right thing to do if you aren't the original submitter, a member of the triaging crew or a core developer.

See #2536 for a similar report, same symptoms but with mutually referential FKs among two models.

comment:4 Changed 7 years ago by ubernostrum

(leaving closed, but pointing out that I agree with the rationale for closing and with the notion that it probably would have been better for one of the triagers to do that)

comment:5 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Needs tests set
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to Accepted

Even though this probably won't get fixed for manipulators (unless someone really wants it and submits a patch, in which case we will gladly accept), we do need to make sure that the bug doesn't also exist in newforms. At the very least, we should make a test for this using newforms to be assured it has been fixed.

So as a general rule, tickets that are requesting new features for Manipulators (like #1679) can be closed as wontfix, but tickets that are bugs should be left open to make certain that the same problem doesn't exist in newforms.

comment:6 Changed 7 years ago by ubernostrum

(In [4673]) 0.91-bugfixes: Fixed #999 by resolving name clash in the metasystem which could confuse manipulators about which fields they should follow. Refs #1808, #1826, #1839 and #2415, which are variations of this that persist in trunk.

comment:7 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4676]) Fixed #1839, #2415, #2536 -- Fixed a generated name clash that was common in
self-referential and circular relations. A lot of community debugging went into
this fix, so thanks to bmurdock@…, Marek Kubica, ramiro, Michael
Radziej (the last two giving test cases showing the problem) and James Bennett
(who did the hard work to actually diagnose the true problem and fix it).

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.