Fix ManyToMany schema mapping

[spockNinja]

Issues
closes #155
closes #157
closes #172

The Problem

ManyToMany fields were not mapping correctly and required some less than ideal workarounds.

This was because the get_reverse_fields did not differentiate between the original instantiation of the ManyToMany and the automatic reverse version. There was then two copies of the field coming back from get_model_fields, the one at the end being incorrect.

The Solution

Unfortunately, the reverse flag on the "reverse" fields has not always been around. To maintain backwards compatibility, get_reverse_fields is doing the best it can.

Instead, we can make sure that local fields are not overridden by reverse fields. So a few lines in get_model_fields limits get_reverse_fields to skip local fields.

Tests

Before fix:

After fix:

Example

Before fix:

After fix:

[coveralls]

Coverage decreased (-0.1%) to 93.684% when pulling cfe38ae on spockNinja:fix_many_to_many into afec960 on graphql-python:master.

[coveralls]

Coverage remained the same at 93.801% when pulling ca06d74 on spockNinja:fix_many_to_many into afec960 on graphql-python:master.

[spockNinja]

@syrusakbary

It appears that Django 1.8 does not have the reverse flag. We could simply use getattr and this fix will then not work for <1.8. Also open to other suggestions now that the root of the problem is clear.

[spockNinja]

Another option might be to add logic to get_model_fields that ignores any reverse_fields that are already in all_fields. I'll give that a shot and see if it is more friendly to Django 1.8.

[coveralls]

Coverage increased (+0.01%) to 93.816% when pulling 74e4e1a on spockNinja:fix_many_to_many into afec960 on graphql-python:master.

[spockNinja]

That did the trick locally, and is passing all the tests. I'll update the PR description with the new solution.

[coveralls]

Coverage increased (+0.02%) to 93.823% when pulling 9551098 on spockNinja:fix_many_to_many into afec960 on graphql-python:master.

[LegoStormtroopr]

a. @spockNinja you are amazing
b. I think Django <= 1.7 is officially deprecated, so I wouldn't worry about that.

Considering how bleeding edge GraphQL is, my recommendation would be if 1.8 doesn't have the fields needed the focus on 1.9 and above. LTS is a stability release, and one of the downsides to that is not having every new piece of tech. But I'm not a maintainer of this repo, so I'm not sure what weight that advice holds here.

[spockNinja]

I have changed the solution to not use the "new" reverse flag. The solution now is to simply ignore "local" fields when building the list of reverse fields. Tests are passing and it's working for me. Any feedback would be appreciated.

[spockNinja]

@syrusakbary I believe this PR is ready for review. I have tested it with our data, added automatic tests, and all tests are passing. Let me know if there is anything I need to improve to get this merged. Thanks! 😄

[syrusakbary]

Great PR, and great analysis on how to make it in a compatible way.
Merging :)

[clodal]

In case anyone needs it now, I left a workaround for graphql m2m connections in #172