django-salesman icon indicating copy to clipboard operation
django-salesman copied to clipboard

Allow non-integer primary keys

Open thenewguy opened this issue 1 year ago • 2 comments

The basket serializer expects an Integer primary key for product_id.

From an initial search it looks like this is the only spot requiring Integer primary keys. UUID primary keys are common and for consistency it is nice to use them here.

Please consider adding support for UUID (or more generically, non-integer) primary keys.

Thanks!

thenewguy avatar May 06 '24 15:05 thenewguy

Also - I would be happy to provide PRs for the issues that I have raised. Please let me know if you will accept suitable PRs for any of them. Thanks!

thenewguy avatar May 06 '24 15:05 thenewguy

I worked through this. The required changes would be:

  • modify salesman.basket.serializers.BasketItemCreateSerializer.product_id to be a CharField or allow the BasketItemCreateSerializer to be swapped. Currently this one has to be monkey patched

The generally useful changes would be:

  • modifying BasketItem.product_id and OrderItem.product_id to use a CharField so the generic relation works with IntegerFields or UUIDFields out of the box. Currently, this one can be overridden on the swapped models.

The documentation changes would be:

  • cast the basket id in PaymentMethod.basket_payment like order.extra["basket_id"] = str(basket.id)
  • unless basket id stored on session is cast to str, depending on the deployment, may also need to provide a session serializer with settings.SESSION_SERIALIZER like:

from rest_framework.utils.encoders import JSONEncoder


class JSONSerializer:
    """
    Simple wrapper around json to be used in signing.dumps and
    signing.loads.
    """
    def dumps(self, obj):
        return json.dumps(obj, separators=(',', ':'), cls=JSONEncoder).encode('latin-1')

    def loads(self, data):
        return json.loads(data.decode('latin-1'), cls=JSONEncoder)

So far those seem to be the only required changes.

thenewguy avatar May 06 '24 18:05 thenewguy