twoscoops / two-scoops-of-django-1.8

Tracking thoughts and feature requests for Two Scoops of Django 1.8 in the issue tracker. And the book's code examples are here.
400 stars 81 forks source link

Chapter 10.6 - incorrect get_object_or_404 parameter #65

Closed bogdal closed 9 years ago

bogdal commented 9 years ago

Hi there,

In examples 10.13 and 10.14 we have the following line:

flavor = get_object_or_404(Flavor, pk=kwargs['slug'])

But slug field in the definition of model Flavor (example 10.7) is not a primary key, hence instead of pk in the above line we should use slug:

flavor = get_object_or_404(Flavor, slug=kwargs['slug'])
pydanny commented 9 years ago

Thanks for the catch. Changed example 0.7 to define the slugfield thus: slug = models.SlugField(unique=True).

:ship:

bogdal commented 9 years ago

@pydanny unique=True is not good enough in our case, we should rather use primary_key=True.

Generally I see here two options:

# models.py
slug = models.SlugField(unique=True)

# views.py
flavor = get_object_or_404(Flavor, slug=kwargs['slug'])
# models.py
slug = models.SlugField(primary_key=True)

# views.py
flavor = get_object_or_404(Flavor, pk=kwargs['slug'])

Right now we mix both options, therefore current solution still doesn't work.

pydanny commented 9 years ago

unique=True adds an index. Which makes it fine to use slug as the primary identifier.

Reference: https://docs.djangoproject.com/en/1.8/ref/models/fields/#unique

bogdal commented 9 years ago

I agree with you, but in the following line we should use slug instead of pk: flavor = get_object_or_404(Flavor, pk=kwargs['slug'])

Currently this line says: give me the flavor which id (default primary key field) is equal kwargs['slug'], what is incorrect.

pydanny commented 9 years ago

Ha ha ha!. Thank you so much for having a good eye. :smile_cat:

:ship: