dominno / django-moderation

django-moderation is reusable application for Django framework, that allows to moderate any model objects.
BSD 3-Clause "New" or "Revised" License
269 stars 90 forks source link

Django 1.10+ Support #155

Closed blag closed 7 years ago

blag commented 7 years ago

Redo Django 1.10 support

blag commented 7 years ago

@dominno Do you have any idea why loading the JSON is failing for Django 1.10+? I haven't the foggiest idea...

miigotu commented 7 years ago

Looks like building the egg for linecache2 fails on py2, try adding a pip install linecache2 just after the pip install $DJANGO. On my machine installing it that way works since it uses a whl https://travis-ci.org/dominno/django-moderation/jobs/248920557#L283

dominno commented 7 years ago

@blag No idea, didn't had this problem. Maybe it would be good to switch Django fixtures to http://factoryboy.readthedocs.io/en/latest/index.html ?

miigotu commented 7 years ago

@dominno switching the fixtures out for another method doesnt really fix the problem =P

blag commented 7 years ago

I figured out the fixture problem - Django 1.10+ tightened up the requirements for m2m relation serializations, so even though there were zero m2m relations specified:

[
    {
        "pk": 1, 
        "model": "auth.user", 
        "fields": {
            "username": "moderator", 
            "first_name": "", 
            "last_name": "", 
            "is_active": true, 
            "is_superuser": false, 
            "is_staff": true, 
            "last_login": "2009-11-20 13:34:03", 
            "groups": [], 
            "user_permissions": [], 
            "password": "...", 
            "email": "...", 
            "date_joined": "2009-11-20 13:34:03"
        }
    },

I had to remove the m2m relations entirely:

[
    {
        "pk": 1, 
        "model": "auth.user", 
        "fields": {
            "username": "moderator", 
            "first_name": "", 
            "last_name": "", 
            "is_active": true, 
            "is_superuser": false, 
            "is_staff": true, 
            "last_login": "2009-11-20 13:34:03", 
            "password": "...", 
            "email": "...", 
            "date_joined": "2009-11-20 13:34:03"
        }
    },

That problem is solved, but I have a few more issues to work out now.

blag commented 7 years ago

Alright, now all of the tests are passing! 😄

I tried to break everything up into smaller, more manageable chunks. Every commit except d616de1f40c234800141efe8c6969a8ba695f38b should be pretty straightforward to review.

For d616de1f40c234800141efe8c6969a8ba695f38b, I would check out the changes I made to tests/ to make sure they still look proper. Other than that, the changes to

contain the bulk of interesting changes.

Let me know if there's anything I should redo.

dominno commented 7 years ago

@blag Thank you, PR is merged

blag commented 7 years ago

I meant to update the README to indicate support for Django 1.10+, but I forgot to. I'm on mobile right now, can somebody else take care of that? Should be pretty quick and simple. If not I'll try to get to it on Monday.

blag commented 7 years ago

Nevermind, I got around to updating the README earlier than I thought I would: #156.

blag commented 7 years ago

@dominno Can we get a new PyPI release with this? It would simplify my deployment.

I'm working on a PR for #153, but I think that will be a breaking change, so a major version bump would probably be better for that.