Model Inheritance doesn't work in the admin.

[anonymous]

Under QS-RF, inherited models don't work in the admin. See Malcolm's message: ​http://groups.google.com/group/django-developers/msg/c0fb00dabb70c660

[Jacob]

We're going to need a lot more information to be able to debug this one. Please read our ​guidelines for submitting bugs, and especially this:

Do write complete, reproducible, specific bug reports. Include as much information as you possibly can, complete with code snippets, test cases, etc. This means including a clear, concise description of the problem, and a clear set of instructions for replicating the problem. A minimal example that illustrates the bug in a nice small test case is the best possible bug report.

Feel free to reopen with more info if you've got it.

[anonymous]

Sorry, I hope that this comment helps you to reproduce the behaviour that I think that is a bug.

Reproducing de bug

1. Suppose a django app with this model:

   class Place(models.Model):
       name = models.CharField(max_length=50)
       address = models.CharField(max_length=80)
   
       def __unicode__(self):
           return u"%s the place" % self.name
   
       class Admin:
           pass
   
   class Restaurant(Place):
       serves_hot_dogs = models.BooleanField()
       serves_pizza = models.BooleanField()
   
       def __unicode__(self):
           return u"%s the restaurant" % self.name
   
       class Admin:
           pass
   

2. Now, go to the admin site (<address to your app>/admin) and create a new Restaurant.
3. Then, try edit the restaurant created in the previous step. Modify some field value and press "save". Now when go to the Restaurant list appears TWO restaurants: the save on tha previous step creates a new restaurant instead of edit the first.

NOTE

If define the primary key of the parent class(Place) explicitly the edition functionality works well:

class Place(models.Model):
    id = models.AutoField('id', primary_key=True, editable=False)
    name = models.CharField(max_length=50)
    address = models.CharField(max_length=80)

    def __unicode__(self):
        return u"%s the place" % self.name

    class Admin:
        pass

class Restaurant(Place):
    serves_hot_dogs = models.BooleanField()
    serves_pizza = models.BooleanField()

    def __unicode__(self):
        return u"%s the restaurant" % self.name

    class Admin:
        pass

[Karen Tracey <kmtracey@…>]

I believe this is a known problem. You are using queryset-refactor, model inheritance, and admin. Per Malcolm's note yesterday on the status of queryset-refactor:

​http://groups.google.com/group/django-developers/msg/c0fb00dabb70c660

"model inheritance is not expected to work in admin at the moment".

[Jacob]

As Karen said, MI doesn't yet work in the admin; updating the ticket to indicate that.

IMO, also, we could simply wait for newforms-admin to fix this; dunno how much that'll annoy people.

[Alex Gaynor]

I don't think we want to merge half working stuff into the trunk, what I have heard suggested is merging qs-rf into nfa, and then when nfa merges we get both into trunk.

[Malcolm Tredinnick]

Alex, that isn't what is going to happen. Describing something as "half working" isn't accurate. It's fully working for the features implemented. The extra feature of being able to use it via admin is not implemented (making it completely equivalent to, say, generic relations). Since model inheritance is an entirely new feature, it's not going to hurt existing code for it not to be part of admin yet. Given the amount of work involved, I'm intending to make the newforms-admin changes required after queryset-refactor has been merged to trunk.

[Malcolm Tredinnick]

Note that I'm not pushing the development burden here onto the newforms-admin guys. I'll do the work for this when I get some time. My last change was just to make sure it shows up in the right list of things that need doing.

[Ramiro Morales]

#7138 (list_filter ignores foreign keys to inherited models) was closed as duplicate of this.

[sloonz]

We just ran into this problem today. Here is a small patch to fix it, rather self-explanatory I think, but you're welcome if you need some explanations...

[sloonz]

Fix (just a try...)

[Etienne Robillard]

Hi,

Here's a simple use case (and traceback below)

from django.contrib.auth.models import User
class Member(User):
 # a "multi-table" model, inheriting from the User model
 ...

Traceback:
File "/home/erob/work/mos_australia/contrib/django/core/handlers/base.py" in get_response
  82.                 response = callback(request, *callback_args, **callback_kwargs)
File "/home/erob/work/mos_australia/contrib/django/contrib/admin/views/decorators.py" in _checklogin
  62.             return view_func(request, *args, **kwargs)
File "/home/erob/work/mos_australia/contrib/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/home/erob/work/mos_australia/contrib/django/contrib/admin/views/main.py" in change_list
  757.         cl = ChangeList(request, model)
File "/home/erob/work/mos_australia/contrib/django/contrib/admin/views/main.py" in __init__
  582.         self.get_results(request)
File "/home/erob/work/mos_australia/contrib/django/contrib/admin/views/main.py" in get_results
  640.             result_list = list(self.query_set)
File "/home/erob/work/mos_australia/contrib/django/db/models/query.py" in __len__
  55.             self._result_cache.extend(list(self._iter))
File "/home/erob/work/mos_australia/contrib/django/db/models/query.py" in iterator
  162.         for row in self.query.results_iter():
File "/home/erob/work/mos_australia/contrib/django/db/models/sql/query.py" in results_iter
  200.         for rows in self.execute_sql(MULTI):
File "/home/erob/work/mos_australia/contrib/django/db/models/sql/query.py" in execute_sql
  1466.         cursor.execute(sql, params)
File "/home/erob/work/mos_australia/contrib/django/db/backends/util.py" in execute
  18.             return self.cursor.execute(sql, params)
File "/home/erob/work/mos_australia/contrib/django/db/backends/sqlite3/base.py" in execute
  136.         return Database.Cursor.execute(self, query, params)

Exception Type: OperationalError at /admin/accounts/memberprofile/
Exception Value: no such column: accounts_member.id

Please note that I've applied the patch outlined above but it fails to solve
this problem. As a side note, if I add the following line to my Member class::

id = AutoField('id', primary_key=True, ...)

The exception value would express a different (but still apparently missing) column.

I did not tried with the newforms-admin branch, only trunk for now.

Any suggestions besides posting this on a mailing-list ?

Thanks,

Etienne

[ekellner <ekellner@…>]

I encountered this as well, and I have a unit test that for it. The unit test identifies the problem I am having, and the patch does indeed fix it.

The problem is when you use a OneToOne field with the parent link option, so that the primary key has a different name in the parent and child tables. if the object you're working with has an existing primary key (object.pk is set), I don't think the sql shouldgenerate an insert instead of an update on the parent table.

class Place(models.Model):
    name = models.CharField(max_length=50, core=True)
    address = models.CharField(max_length=80, null=True)

class Restaurant(Place):
    """
    >>> p = Place.objects.create(name="Alice's", address="1 Main St", pk=1)
    >>> p.id
    1
    >>> r = Restaurant(serves_hot_dogs=True, serves_pizza=True)
    >>> r.pk = 1
    >>> r.save()
    >>> r.pk
    1
    >>> r.place_id
    1
    """
    serves_hot_dogs = models.BooleanField()
    serves_pizza = models.BooleanField()
    place = models.OneToOneField(Place, parent_link=True)

[sloonz]

Etienne, I can't reproduce your problem (r7569 from trunk + my patch), with the following (and simple) models.py:

from django.contrib.auth.models import User

class Member(User):
    class Admin:
        pass

# Automatically filled, don't have to bother the user with this
Member._meta.get_field('user_ptr').blank = True

Can you provide the full snippet causing the problem ?

[sloonz]

ekellner, I don't think you're testing the problem stated here. You're testing "what if the user create only the child class and manually make the association with the already existing parent class", or, in other words, "can we create the base and the child separately and make the link ad-hoc ?". Honestly, I don't know what's the "right thing to do" for this corner case. IMO, creating the base and the parent at different transactions is just *ugly*, and should be discouraged by stating that "behavior for this is not guaranteed and may change for the sake of django internal matters". You don't have to create a Place if you want to create a Restaurant, just create a Restaurant and django will create the Place for you. But I'm not a django dev, and they'll obviously have the last word, so I'd like to have their opinion : my patch don't allow to make this ad-hoc association, and I hardly see how to fix this bug, with allowing this corner-case and without making substantial core changes (far too much for my superficial knowledge of the django code, anyway). So if they think that the behavior change caused by my patch is just unacceptable, I just have to give up on this problem -- and sooner is better ;).

This problem is just "if base PK is unknown, it should be guessed from child PK, but instead django say 'no PK ? No problem, I'll just create a new record'"

Anyway, I'll try to make a test-case for this problem tomorrow.

[Etienne Robillard]

Replying to sloonz:

Etienne, I can't reproduce your problem (r7569 from trunk + my patch), with the following (and simple) models.py:

from django.contrib.auth.models import User

class Member(User):
    class Admin:
        pass

# Automatically filled, don't have to bother the user with this
Member._meta.get_field('user_ptr').blank = True

Can you provide the full snippet causing the problem ?

Sloonz, here's the full snippet:

class Member(User):
    """The Member model"""
    objects = MemberManager()

    def __repr__(self):
        return "<Member: %s>" % self.username
    def __unicode__(self):
        return u"%s" % self.username
        
    def get_profile(self):
        """
        This method shall return the ``MemberProfile`` object for
        the current (``Member``) instance.
        """
        try:
            obj = MemberProfile._default_manager.get(user__id=self.id)
        except MemberProfile.DoesNotExist:
            raise Http404
        else:
            if obj is not None:
                return obj
    class Meta:
        verbose_name = 'member'
        verbose_name_plural = 'members'
        app_label = 'accounts'
        db_table = 'accounts_member'

    class Admin:
        pass

class MemberProfile(models.Model):
    user = models.ForeignKey(Member, unique=True, raw_id_admin=True)
    # more profile fields here

Notice that we overrided get_profile in the Member class, since this
is what we want to do. Then I can call get_profile as usual and it should returns
the related profile for a given Member instance:

>>> auth_user = Member(username='sloonz')
>>> profile_data = MemberProfile(user=auth_user)
>>> profile_data.save()
>>> # Try retrieving the member data with get_profile 
>>> self.failUnlessEqual(profile_data == auth_user.get_profile(), True)

[ekellner <ekellner@…>]

Replying to sloonz:

This problem is just "if base PK is unknown, it should be guessed from child PK, but instead django say 'no PK ? No problem, I'll just create a new record'"

I think you're right sloonz, in that this pinpoints the problem, but I think I am leaning towards a stronger statement. I don't think it's possible for the base PK to be unknown. It's model inheritence, and it has to be the same as the child PK. I could understand the viewpoint that says, if the code has gotten to this point, there's already something wrong.

[sloonz]

@erob
Still can't reproduce your problem, but:

• I don't have the definition of MemberManager, I just commented out this line
• profile_data != auth_user.get_profile() because you didn't defined a cmp method, so your test is equivalent to "profile_data is auth_user.get_profile()". Since django doesn't reuse previous instances, this test will always fail
• with your code, I have a strange bug: adding a member in an empty database (only with the first user) overrides the first user...

Can you describe the exact steps to reproduce your bug ? Have you tried with a shiny new DB ?

@ekellner

It's model inheritence, and it has to be the same as the child PK

No, not in the case of multiple inheritance (yet the current implementation seems to be bogus on multiple inheritance) : if C inherits from A and B, since we can have A.pk != B.pk we can't have A.pk = C.pk and C.pk = B.pk.

[sloonz]

Regression test

[Russell Keith-Magee]

Making this an NFA-blocker, as per ​http://groups.google.com/group/django-developers/msg/02b044140a5f759f

[Karen Tracey <kmtracey@…>]

I read Jacob's message to mean this should be nfa-someday, not nfa-blocker. If it can be done after the merge to trunk, then it shouldn't be nfa-blocker.

[Karen Tracey <kmtracey@…>]

(unless the meanings have been changed?)

[Russell Keith-Magee]

@Karen - You are correct. However, nfa-someday marks lot of things that _wont_ be on the list for v1.0, so to make sure it doesn't get forgotten, I stuck it in nfa-blocker. If it ends up happening post-merge, so be it.

[Brian Rosner]

I have tested the first use case in newforms-admin it the problem doesn't seem to exist there. Can people start banging against newforms-admin and report some specific issues they see. It will help get this done faster. I have attached a patch that would make sense for ModelForms and model inheritance.

[Michael Placentra II <someone@…>]

sloonz's, compatible with newer revisions (currently 7634)

[anonymous]

Although sloonz patch to prevent duplication works fine but was not merged with trunk, i don't understand the last line of code:

# Automatically filled, don't have to bother the user with this
Member._meta.get_field('user_ptr').blank = True

Shouldn't it prevent the admin site to show the "User ptr" field for this model? It doesn't here with rev 7928. Note that i used the same exact proof-code from sloonz.

[Brian Rosner]

Removing the nfa-blocker keyword since this isn't a blocker, but since we have tagged it with 1.0 alpha that will be enough to ensure it is not forgotten. This should be dealt with once newforms-admin is merged to trunk.

[anonymous]

Works with both brosner's and sloonz patchs

[Jacob]

(In [8033]) Fixed #6755: model inheritance now works in the admin. Thanks, sloonz and Michael Placentra.

[Malcolm Tredinnick]

The patch doesn't generate valid SQL on MySQL or either PostgreSQL backend. I'm fixing it now, but reopening in case anybody else notices and wonders if we've seen it.

[Malcolm Tredinnick]

(In [8051]) Fixed the tests from [8033] to work for the PostgreSQL backends (the primary
key value isn't guaranteed to be the same across all backends). Still some
breakage on MySQL because of the way it treats booleans, but that's another
issue.

Refs #6755.

[Malcolm Tredinnick]

Problem turned out to be that the tests were bad, not the patch content itself. That was fixed (for reasonably large values of "fixed") by [8050] and [8051].

[anonymous]

Not working on SQLite3 backend yet.

[Jacob]

Mmm, it is indeed working on sqlite3 for at least the buildbot and me locally. Please open a separate ticket with more information about what "not working" means to you.

[Jacob]

Milestone 1.0 alpha deleted