search

derek-schaefer / django-json-field

Generic JSON model and form fields.
BSD 3-Clause "New" or "Revised" License
122 stars 86 forks source link

JSONField breaks with model_to_dict #3

Closed mjtorn closed 12 years ago

mjtorn commented 12 years ago

Hi!

I've been using a JSONField implementation I found floating around, but it's not as good as this project.

The one bug is, though, when I use _django.forms.models.model_todict.

I'm confused about _value_forobject here, because it returns the json representation, while surely it could return the decoded json object?

My suggested fix would be to remove the method; the default superclass functionality seems good. Or what do you think?

mjtorn commented 12 years ago

Or wait. Is that _value_forobject and/or _model_todict used in form validation? I might have misread the code; the clean method tries also to call loads on the given value, so it might actually fail it if it's given a real data structure.

Wouldn't the first dumps then always be ok?

It might be best not to introduce the change, and I could make another pull request with only the contribute_to_class, although I would have to hack around at my code (a preferrable choice to either maintaining a fork or using something less established than your project)

Comments?

mjtorn commented 12 years ago

Yeah, let's close this, the form gets too fishy otherwise.