wagtail / django-modelcluster

Django extension to allow working with 'clusters' of models as a single unit, independently of the database
BSD 3-Clause "New" or "Revised" License
485 stars 66 forks source link

<Foo object at ...> is not JSON serializable #39

Open thenewguy opened 9 years ago

thenewguy commented 9 years ago

Model cluster does not support primary keys that are not of simple types.

If you have a primary key that uses something like this CashField from django's tests that doesn't inherit from one of the basic types known to the DjangoJSONEncoder then you get <Foo object at ...> is not JSON serializable.

The Cash object I linked to inherits from decimal.Decimal but that is because it is only a simple example for testing.

ModelCluster needs to do some sort of is_serializable check at https://github.com/torchbox/django-modelcluster/blob/c72110e7789415196336584c70b2706fe74dbc2d/modelcluster/models.py#L44 and use Field.value_to_string or the like if the object isn't serializable

thenewguy commented 9 years ago

I just submitted a pull request. All tests pass for me locally. This issue isn't just for primary keys. That is just what I ran into first. It would be for any field that returns a value that cannot be directly converted to json. Not sure how you would want to test it. I can put together a quick test if you want with a custom field like the CashField in the django tests if you like