[fix](notification) use model field JSONField from django.contrib

[areski]

Using django-jsonfield doesn't work as it still use SubfieldBase which has been deprecated and removed in in Django 1.10.
This patch fallback on new JSONField from django.contrib when django version is >= 1.9.

[coveralls]

Coverage decreased (-0.1%) to 89.268% when pulling 8e885ae on areski:master into 0e658dc on django-notifications:master.

[coveralls]

Coverage decreased (-0.7%) to 88.691% when pulling 5723a2e on areski:master into 0e658dc on django-notifications:master.

[yangyubo]

@areski thanks for the contribution. But Travis check was failed, could you please take a look at it?

[areski]

I have done some more digging on this and the test break because the patch force the import of a Postgresql only field from django.contrib.postgres.fields import JSONField when Django >= 1.9 and postgres.JSONField requires that we have psycopg2 installed.

We could add psycopg2 but then my worry is how will this work for peoples not using PostgreSQL.

A solution would be to add this before the import, if settings.DATABASES['default']['ENGINE'] == 'django.db.backends.postgresql_psycopg2' but that hurt my eyes so much...

FYI, there is also a Mysql json field that could be used http://stackoverflow.com/questions/37007109/django-1-9-jsonfield-in-models but that means we increase a lot the number of dependencies and I'm not keen to that.

Any recommendation?

[yangyubo]

How about use django.db.connection.vendor to test if the database engine postgresql or mysql, and fallback to django-jsonfield for other engines

I'm worry about after switching to PostgreSQL or MySQL version of JSONField, will it break deployed production project for peoples who upgrade the package?

[areski]

it's important to try to be backward compatible, I just added an extra line, see if that helps

[coveralls]

Coverage decreased (-0.7%) to 88.691% when pulling b97821d on areski:master into 0e658dc on django-notifications:master.

[psychok7]

Hey guys.. any updates on this?? Would be nice to have this merged

[yangyubo]

@psychok7 This patch still has a couple of problems:

1. As @areski said django-jsonfield doesn't work on Django 1.10, but the patch only deal with Postgres and MySQL. It will false if use SQLite or other database engine
2. How about the project already in production with django-jsonfield deployed? Will switch to JSONField lead to break existing project?

Hope someone can find a better solution.

[viperfx]

Guys django-jsonfield and django 1.9> cannot be used together. We are using this package, and we are facing some issues because this project still uses django-jsonfield.

See here for more info:

https://bitbucket.org/schinckel/django-jsonfield/issues/57/cannot-use-in-the-same-project-as-djangos

https://code.djangoproject.com/ticket/27675

[areski]

What about the current status of jsonfield, it seems they fixed their compatibility issue with Django 1.10 https://github.com/dmkoch/django-jsonfield/

[AlvaroLQueiroz]

Apparently the bug has been fixed. But they removed support for Django <= 1.7. I think we should do the same.

[hdoupe]

What's the status of #160? I'm upgrading Django from 1.8 to 1.9 right now. Solving the issue of moving from django-jsonfield to the postgres JSON field is pretty tricky. As @yangyubo commented above, there may be some unexpected things happening under the django/postgres hood. Basically, if you have a postgres database of version less than 9.4, then the underlying postgres column is saved as type text instead of json or jsonb. See rpkilby/jsonfield#140 for more information.

That means that you have to do add a new migration that specifies that you want to use django.contrib.postgres.fields.JSONField in order to actually change the underlying postgres type tojson or jsonb.

The other concern is what to do with the old migration file that imports the django-jsonfield package. Since the data is saved as type text in postgres anyways, perhaps you can just change the type in the old migration file to models.TextField.

I hope there's a better solution. I'm interested in hearing your thoughts on this.

PS the relevant issue that I'm working on is ospc-org/ospc.org#822.

[nemesifier]

IMHO the best thing would be to give the possibility to enable postgres jsonfield explicitly with a setting, otherwise lots of users of this package may experience unexpected behaviour by this change.

[chokosabe]

What is holding this up? It seems like a really straightforward improvement.

[AlvaroLQueiroz]

Fixed on version 2