djangonauts / django-hstore

PostgreSQL HStore support for Django.
http://django-hstore.readthedocs.io/
Other
517 stars 142 forks source link

create() fails (using model with slug PK) with passed dict #111

Closed stringfellow closed 9 years ago

stringfellow commented 9 years ago

With SerializedDictionaryField using create(data={'foo': 1}) fails if using a model with a non auto-id (e.g. slug)

nemesifier commented 9 years ago

@alukach this issue was related to SerializedDictionaryField, I had to introduce this little change to fix it: https://github.com/djangonauts/django-hstore/commit/c3f3142e6bf9d82d5e91e4353033b09b712f0397

Could you check it out when you have time and let me know if all is ok?

alukach commented 9 years ago

Absolutely, will take a look shortly. On 28 Jun 2015 10:33 am, "Federico Capoano" notifications@github.com wrote:

@alukach https://github.com/alukach this issue was related to SerializedDictionaryField, I had to introduce this little change to fix it: c3f3142 https://github.com/djangonauts/django-hstore/commit/c3f3142e6bf9d82d5e91e4353033b09b712f0397

Could you check it out when you have time and let me know if all is ok?

— Reply to this email directly or view it on GitHub https://github.com/djangonauts/django-hstore/issues/111#issuecomment-116296151 .

alukach commented 9 years ago

Have we been able to verify this bug? Looking at this test:

    def test_create_directly(self):
        SerializedDataBag.objects.create(name='abc', data={'num': 1})
        SerializedDataBagNoID.objects.create(slug='123', name='abc', data={'num': 1})

The models are as such:

class SerializedDataBag(HStoreModel):
    name = models.CharField(max_length=32)
    data = hstore.SerializedDictionaryField()

class SerializedDataBagNoID(HStoreModel):
    slug = models.SlugField(primary_key=True)
    name = models.CharField(max_length=32)
    data = hstore.SerializedDictionaryField()

Wouldn't this condition be caught by SerializedDataBag.objects.create(name='abc', data={'num': 1})? What's different?

alukach commented 9 years ago

Ah, okay, I now see that the test_create_directly test was created to illustrate this bug and that @nemesisdesign's fix was merged.

All looks good to me!

nemesifier commented 9 years ago

thx @alukach