gathering / unicorn-backend

Backend/API for UNICORN
https://competitions.gathering.org
MIT License
2 stars 1 forks source link

Fix issue where serialization of user relations failed #21

Closed niccofyren closed 1 year ago

niccofyren commented 2 years ago

Noticed this while trying to post data to create a model that related to a user object.

For example; Posting this user data to /api/achievements/awards, results in the Primary key must be an integer being thrown

{
    "user": "0cc5dda4-7cc3-4847-b4e1-152e20af65ca",
    "achievement": "2"
}

The new version of this checks still tries integer first, but re-tries as undefined type (can it ever be anything else than string on second try?) if that fails.

eriktm commented 2 years ago

Based on a few quick tests we should probably stop trying to cast as integer, and rather give a more generic error paired with the text/value of the underlying exception when it fails.

Fetching an object with UUID-based PK

>>> from accounts.models import User
>>> User.objects.get(pk="test")
ValueError: badly formed hexadecimal UUID string
django.core.exceptions.ValidationError: ['“test” is not a valid UUID.']

>>> User.objects.get(pk=1)
accounts.models.DoesNotExist: User matching query does not exist.

Fetching an object with int-based PK

>>> from competitions.models import Competition
>>> Competition.objects.get(pk="test")
ValueError: invalid literal for int() with base 10: 'test'
ValueError: Field 'id' expected a number but got 'test'.
niccofyren commented 2 years ago

Right, so just rip out the existing lookup and replace with the fallback one introduced in this PR? Plus some prettier/more relevant error messages?

eriktm commented 2 years ago

Yes. It doesn't seem like we've used the achievements module for a while, as it wasn't up to date with the current account structure in general (see 49d4d16129aaa185e8266db3f4f2f6db136f688d)