Fix select_related with filtering

[camd]

I must admit, I don't totally see the need for this union between the default_queryset and queryset but through experimentation, I have found that if the order of the union is reversed, it will preserve the select_related and prefetch_related. It has the same effect of finding the union of records returned. The list is identical.

So I hope this change doesn't break something I'm not aware of. :)

This is an attempt to fix #179

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[coveralls]

Coverage remained the same at 92.721% when pulling 146ce58 on camd:fix-select-related into 0588f89 on graphql-python:master.

[spockNinja]

@syrusakbary Same fix as #194, just specific to filter fields. Looks fine to me.

[jancespivo]

Hi, IMO this code violates DRY principle and should be refactorized. The DjangoFilterConnectionField is inherited from DjangoConnectionField where the code is ok (in classmethod merge_queryset). I agree with this fix (it's the same fix as mine in duplicate #272 ) but I think it is definitely not a good final solution.

[camd]

Thanks for accepting the fix! I'm not able to merge it, so I'll leave that to the project collaborators. Thanks everyone. :)

[syrusakbary]

I agree with @jancespivo that the code needs to be refactored, but we can do it in a separate PR.

The change looks good, but I think before merging the PR should include tests ensuring that the issue is not repeated in future releases.

[coveralls]

Coverage decreased (-0.2%) to 92.79% when pulling 7a110ab on camd:fix-select-related into f93251b on graphql-python:master.

[spockNinja]

7a110ab should address everyone's concerns. Using super to use DjangoConnectionField's default implementation. Since the base class's implementation was tested in #194 then it is tested here.

Tests are failing, but it's unrelated. I'm looking into those failures in a separate PR.

[coveralls]

Coverage remained the same at 92.999% when pulling 1d76db8 on camd:fix-select-related into 283bccf on graphql-python:master.

[spockNinja]

Now that the failing test in master is fixed, this PR is ✅.

@syrusakbary Ready for merge, and I can close several duplicate issues/PRs for this bug.

[coveralls]

Coverage remained the same at 93.014% when pulling 95e28ba on camd:fix-select-related into dfa0a06 on graphql-python:master.

[coveralls]

Coverage remained the same at 93.014% when pulling 95e28ba on camd:fix-select-related into dfa0a06 on graphql-python:master.

[coveralls]

Coverage remained the same at 93.014% when pulling 95e28ba on camd:fix-select-related into dfa0a06 on graphql-python:master.

[camd]

Thanks for the merge! :)

[urbandove]

There's an issue with this merge - if a class is subclassing DjangoFilterConnectionField and a queryset is returned from resolve_schema_field the super(cls,cls) refers back to DjangoFilterConnectionField and has an endless loop.

other people fixed this issue by copying the code from the merge_queryset
queryset = queryset & default_queryset

but it seems we can keep this change and have only one place for that code by replacing
queryset = super(cls, cls).merge_querysets(default_queryset, queryset)
with
queryset = super(DjangoFilterConnectionField, cls).merge_querysets(default_queryset, queryset)