3

I would like to display all pet owners (Clients) using list_display and for each owner a comma-separate list of all of their pets (Patients).

The foreign key is in the Patient table, such that an owner can have many pets, but a pet can only have one owner.

I've got the following to work but would like some advise as to whether this is an acceptable approach.

from .models import Client, Patient

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'patients')

    def patients(self,obj):
        p = Patient.objects.filter(client_id=obj.pk)
        return list(p)

This is what it looks like: enter image description here

Thanks for any guidance.

UPDATE: Here's where I'm at so far:

Here's what I've managed to get working so far

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'getpatients')
    def getpatients(self, request):
        c = Client.objects.get(pk=1)
        p = c.patient_fk.all()
        return p

This is following the docs re: following relationships backwards.

Of course, the above example 'fixes' the number of client objects to just one (pk=1) so I'm not sure how I'd get the results for all of the Clients.

@pleasedontbelong - I've tried your code, thank you very much. I'm almost certainly doing something wrong as I'm getting an error.but So you know the FK now has

 related_name = 'patient_fk'

which explains why I'm not using patient_set (since FOO_set is overriden)

So here's what I have:

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'getpatients')

    def get_queryset(self, request):
        qs = super(ClientAdmin, self).get_queryset(request)
        return qs.prefetch_related('patient_fk') 

    def getpatients(self, obj):
        return self.patient_fk.all()

The error I get is "'ClientAdmin' object has no attribute 'patient_fk'" and relates to the last line of the code above.

Any ideas?

Thanks!

EDIT

I've tried Brian's code:

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'getpatients')

    def getpatients(self, obj):
        p = obj.patient_pk.all()
        return list(p)

...and am getting error 'Client' object has no attribute 'patient_fk'

If I run my original code, it still works ok:

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'getpatients')

    def getpatients(self, obj):
        p = Patient.objects.filter(client_id=obj.pk)
        return list(p)

For reference, here are my classes:

class Client(TimeStampedModel):
    first_name = models.CharField(max_length=30)
    last_name = models.CharField(max_length=30)
    ....

class Patient(TimeStampedModel):
    client = models.ForeignKey(Client, on_delete=models.CASCADE, related_name='patient_fk')
    name = models.CharField(max_length=30)
    ....
James Agnew
  • 375
  • 1
  • 4
  • 17

3 Answers3

9

This now works:

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'get_patients')

    def get_queryset(self, obj):
        qs = super(ClientAdmin, self).get_queryset(obj)
        return qs.prefetch_related('patient_fk')

    def get_patients(self, obj):
        return list(obj.patient_fk.all())

This page only needed 6 queries to display...

enter image description here

...compared to my original code (below) which was running a separate query to retrieve the patients for each client (100 clients per page)

from .models import Client, Patient

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'patients')

    def patients(self,obj):
        p = Patient.objects.filter(client_id=obj.pk)
        return list(p)

enter image description here

Here's my understanding of how and why this works (feel free to point out any errors):

Every model has a Manager whose default name is objects allowing us to access the database records. To pull all records from a model, we us SomeModel.objects.all() which - under the hood - is just the QuerySet returned by the get_queryset method of the Manager class.

So if we need to tweak what is returned from a Model - i.e. the QuerySet - then we need to override the method that grabs it, namely get_queryset. Our new method has same name as the method we want to override:

 def get_queryset(self, obj):

Now, the above method knows nothing about how to get access to the modes data. It contains no code. To get access to the data we need to call the 'real' get_queryset method (the one we're overriding) so that we can actually get data back, tweak it (add some extra patient info), then return it.

To access the 'original' get_queryset method and get a QuerySet object (containing all Model data, no patients) then we use super().

super() gives us access to a method on a parent class.

For example:

enter image description here

In our case it lets us grab ClientAdmin's get_queryset() method.

def get_queryset(self, obj):
    qs = super(ClientAdmin, self).get_queryset(obj)

qs hold all the data in the Model in a QuerySet object.

To 'add in' all of the Patients objects that lie at the end of the one-to-many relationship (a Client can have many Patients) we use prefetch_related():

return qs.prefetch_related('patient_fk')'

This performs a lookup for each Client and returns any Patient objects by following the 'patient_fk' foreign key. This is performed under the hood by Python (not SQL) such that the end result is a new QuerySet - generated by a single database lookup - containing all of the data we need to not only list all of the objects in our main Model but also include related objets from other Models.

So, what happens if we do not override Manager.get_queryset() method? Well, then we just get the data that is in the specific table (Clients), no info about Patients (...and 100 extra database hits):

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'get_patients')
    #do not override Manager.get_queryset()
    #def get_queryset(self, obj):
    #    qs = super(ClientAdmin, self).get_queryset(obj)
    #    return qs.prefetch_related('patient_fk')

def get_patients(self, obj):
    return list(obj.patient_fk.all())
    #forces extra per-client query by following patient_fk

I hope this helps someone out there. Any errors in my explanation let me know and I'll correct.

James Agnew
  • 375
  • 1
  • 4
  • 17
3

if it works :+1: !!

few notes however: it will execute one query for each Client, so if you display 100 clients on the admin, django will execute 100 queries

You could maybe improve it by changing the main queryset (like this) on the admin and using prefetch_related('patients')

should be something like:

class ClientAdmin(admin.ModelAdmin):
    list_display = ('first_name', 'last_name', 'mobile', 'patients')

    def get_queryset(self, request):
        qs = super(ClientAdmin, self).get_queryset(request)
        return qs.prefetch_related('patients')  # do read the doc, maybe 'patients' is not the correct lookup for you

    def patients(self,obj):
        return self.patients_set.all()  # since you have prefetched the patients I think it wont hit the database, to be tested

Hope this helps

Note:

you can get all the Patients related to a Client using the related object reference, something like:

# get one client
client = Client.objects.last()
# get all the client's patient
patients = client.patient_set.all()

the last line is similar to:

patients = Patient.objects.get(client=client)

finally you can override the patient_set name and make it prettier, read https://docs.djangoproject.com/en/1.9/topics/db/queries/#following-relationships-backward

I haven't tested it, It would be nice to have a feedback to see if this will prevent the n+1 problem

Community
  • 1
  • 1
pleasedontbelong
  • 19,542
  • 12
  • 53
  • 77
  • Thanks, I'm still a bit confused though, so bear with me :) Can you describe how I would use the queryset and prefetch_related()? My understanding is that from the Client object I'm able to make a 'reverse' lookup of a one-to-many relationship i.e. all of the related Patient fields for each of the given Client objects. As it stands, I've tried dozens of combinations and still don't quite get it, so a nudge in the right direction would be much appreciated. – James Agnew Aug 08 '16 at 13:46
0
def patients(self,obj):
        p = obj.patients.all()
        return list(p)

this is assuming that in your ForeignKey you set related_name='patients'

EDIT: fixed mistake EDIT2: changed reverse_name to related_name and added '.all()'

Brian H.
  • 854
  • 8
  • 16
  • Thanks. I guess you mean `related_name` which I have indeed set. I'm still struggling with understanding how the Client class is able to 'see' into the various Patient `name` values without needing to instantiate a Patient object. I guess this is why I need to have both `related_name` and `prefetch_related` but for the life of me I'm not getting the reverse lookup working. – James Agnew Aug 08 '16 at 13:54
  • yes, i meant related_name, fixed my answer, also, i forgot to add the call to the all() method, which i've added now too – Brian H. Aug 09 '16 at 07:26
  • Thanks very much Brian. I've edited my post with my result, still getting missing 'patient_pk' attribute on Client object... any ideas? – James Agnew Aug 09 '16 at 14:32