LookupChannel.get_objects fails for inherited models

[vil-s]

Given a these models:

from django.db import models

class Base(models.Model):
    foo = models.TextField()

class Inherited(Base):
    bar = models.TextField()

class Other(models.Model):
    foobar = models.ForeignKey(Inherited)

This implicitly creates a base_ptr field on Inherited with the type models.OneToOneField.

We then add the InheritedLookupChannel and add it to Other's admin. When trying to submit the form, LookupChannel.get_objects fails, because the pk_type resolves to models.OneToOneField.to_python, which just returns the value it got as an argument.

The problem is that ids is a list of strings, but the in_bulk method returns the objects as {<id>: <object>}, but the id is an integer, not a string, which makes get_objects return an empty list.

The return line seems a bit redundant though, can't it just return something like things.values(), or would that break some things?

[pkaczynski]

This is a real blocker for me. Any chance on fixing that? Any workaround?

[vil-s]

@pkaczynski I have used this as a workaround, but I haven't submitted a PR for it, because I'm not sure if it is a 100% fix:

def get_objects(self, ids):
    pk_type = self.model._meta.pk.to_python
    ids = [pk_type(pk) for pk in ids]
    things = self.model.objects.in_bulk(ids)
    return things.values()

Just override the get_objects method with that one on the affected model's LookupChannel.

[pkaczynski]

@unklphil Thanks for the info. I really tried to make a PR, but after an hour or so, I dropped it since I didn't manage to run the tests properly :(

Maybe I'll be back with a cleanup proposal in a PR soon. Nevertheless, thanks for the workaround.

[crucialfelix]

The problem is that ids is a list of strings

ids was a list of strings when passed in, but they are then cast to the pk_type. So in most cases they are integers (database pks).

If this:

things = self.model.objects.in_bulk(ids)

is returning a dict of Base objects then you might need to convert them to the Inherited. self.model is Base so it is going to return Base objects. It's going to query the Base table.

to convert those to leaf classes add this to Base:

    def as_leaf_class(self):
        """
            if you have an Base object this will reload it and return it as the subclass model
        """
        if not type(self) is Base:
            return self
        content_type = self.content_type
        model = content_type.model_class()
        if(model is Base):
            # Base is abstract although it has a db table
            raise Exception("Found instance of Base.")
        return model.objects.get(id=self.id)

and now you can convert each object to it's inherited class by calling thing.as_leaf_class(). This will result in many db lookups, so be warned.

[vil-s]

ids was a list of strings when passed in, but they are then cast to the pk_type

In the case of inherited models (through concrete inheritance), the pk_type does not change the strings to integers, because the primary key field is a OneToOneField which uses the base Field's to_python, which returns the input value as-is. (See here)

The in_bulk method does return model objects of the correct type (in this case the Inherited model), so there is no need to convert them.

To illustrate the problem better, here's what's happening in the current LookupChannel.get_objects method:

>>> ids = ['1', '2']
>>> pk_type = self.model._meta.pk.to_python  # django.db.models.fields.Field.to_python
>>> ids = [pk_type(pk) for pk in ids]
>>> ids
['1', '2']
>>> things = self.model.objects.in_bulk(ids)
>>> things
{1: <Inherited object 1>, 2: <Inherited object 2>}
>>> # Here is where the problem comes in:
>>> return_val = [things[aid] for aid in ids if aid in things]
>>> return_val
[]
>>> # What was expected is: [<Inherited object 1>, <Inherited object 2>]
>>> # This discrepancy is because after the ids went through pk_type, they're still strings
>>> ids
['1', '2']
>>> list(things.keys())
[1, 2]
>>> assert ids == list(things.keys())
AssertionError
>>> assert ids[0] in things
AssertionError

Hope that makes it clear what the issue is.

[crucialfelix]

So pk_type is not reliably polymorphic. To me it looks like self.model._meta.pk.to_python is broken if the pk types are different in the base classes. Why anybody would want a different pk type in the base class is something I don't understand.

LookupChannel is under your (the developer's) control so you can implement get_objects as you need to for special cases.

This can be documented in LookupChannel itself to note when you might want to implement that method yourself (eg. for inherited classes or any place you have OneToOne fields)

Your suggestion above would remove the sorting - which I suppose some people might think that's not important, but for a user they expect it to be shown in the same order that they added them.

[crucialfelix]

The code from #203 is merged and released now. I have a test for the normal pk situation but did not setup a test for the inherited situation.