nshafer / django-hashid-field

Django Model Field that uses Hashids to obscure the value
MIT License
370 stars 40 forks source link

TypeError when primary key is an OneToOneField #10

Closed OskarPersson closed 7 years ago

OskarPersson commented 7 years ago

With the models and serializers below I get an exception when listing the books:

TypeError: '1' value must be a valid Hashids string.

Removing the nested serialization of authors in the book serialization gets rid of the exception but displays an invalid id for the author of each book.

Models

# books/models.py
from django.db import models
from hashid_field import HashidAutoField

class Book(models.Model):
    id = HashidAutoField(primary_key=True)
    name = models.CharField(max_length=255)
    author = models.ForeignKey('authors.Author', on_delete=models.SET_NULL, related_name='books', null=True)
# customauth/models.py
from django.contrib.auth.models import AbstractUser
from django.db import models
from hashid_field import HashidAutoField

class User(AbstractUser):
    id = HashidAutoField(primary_key=True)
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)
# authors/models.py

from django.conf import settings
from django.db import models

class Author(models.Model):
    user = models.OneToOneField(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, primary_key=True)
    description = models.TextField(max_length=1000)

Serializers

# authors/serializers.py
from django.conf import settings
from django.contrib.auth import get_user_model
from hashid_field.rest import HashidSerializerCharField
from rest_framework import serializers
from .models import Author

class UserSerializer(serializers.HyperlinkedModelSerializer):
    id = HashidSerializerCharField(source_field='%s.id' % settings.AUTH_USER_MODEL)

    class Meta:
        model = get_user_model()
        fields = ('url', 'id', 'username')

class AuthorSerializer(serializers.HyperlinkedModelSerializer):
    user = UserSerializer()

    class Meta:
        model = Author
        fields = ('url', 'user', 'description',)
# books/serializers.py
from hashid_field.rest import HashidSerializerCharField
from rest_framework import serializers
from authors.serializers import AuthorSerializer
from .models import Book

class BookSerializer(serializers.HyperlinkedModelSerializer):
    id = HashidSerializerCharField(source_field='books.Book.id')
    author = AuthorSerializer()

    class Meta:
        model = Book
        fields = ('url', 'id', 'name', 'author',)
nshafer commented 7 years ago

I took a quick look into this, but I couldn't replicate it, and unfortunately I don't have time to fully set up a duplicate test case. Do you happen to have a repo I can clone with this issue set up? Thanks!

OskarPersson commented 7 years ago

I created a repo for you to test here. You should only have to create a book, an author and a user that are all connected to get the error described above when you visit /api/books

nshafer commented 7 years ago

Just an update, after digging deep into DRF and Django internals, I've tracked down the source of this issue. It's the hack I put in to disallow lookups of a Hashid*Field by integer, and force lookups only by string. I have an idea of how to fix it and will be working on it soon. I'm going to have to set up custom Lookup classes for the different kinds of possible lookups that I want to support, and check for integer lookups there, so that get_prep_value can take either an int or a string. In the meantime, turning off HASHID_FIELD_ALLOW_INT or setting allow_int=True on the particular field will get around the issue. Obviously not a great fix, so I will be attempting to fix this properly soon.

Thanks for the repo, it was a huge help in being able to debug.

nshafer commented 7 years ago

This is now fixed in version 1.3.0.