Filtering not working correctly with 2.6.0

[lassesteffen]

Related to #750. Appreciate the fix for this problem!!

Problem

When using filter_fields I get an error about using wrong types which started appearing in 2.4.0.
Variable "userEmail" of type "String" used in position expecting type "ID". The error does not occur with graphene-django 2.3.2

Context

• using django-filter 2.2.0
• django 2.4.0
• graphene-django 2.6.0

model.py

class Membership(TimeStampedModel):
    user = models.ForeignKey(User, on_delete=models.CASCADE)
    tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE)



class User(TimeStampedModel, AbstractBaseUser, PermissionsMixin):
    email = EmailField(unique=True, verbose_name=_('email'))

    USERNAME_FIELD = 'email'
    REQUIRED_FIELDS = []

    objects = UserManager()

Schema.py

class MembershipNode(DjangoObjectType):
    class Meta:
        model = Membership
        filter_fields = {
            'id': ['exact'],
            'user__email': ['exact'],
        }
        interfaces = (MembershipNodeInterface,)

Query:

QUERY_MEMBERSHIPS = '''
    query memberships($tenant: String!, $userEmail: String) {
        memberships(tenant: $tenant, user_Email: $userEmail) {
            edges {
                node {
                    id
                    isFitter
                    isMonitor
                    isAdmin
                    isStaff
                }
            }
        }
    }
'''

Result:
Variable "userEmail" of type "String" used in position expecting type "ID".

Solution

Should be related to #750. Might be a special case due to the email being the identifying field of the User

I am confident it is related to this PR: https://github.com/graphql-python/graphene-django/pull/682/files . In graphene_django/filter/utils.py the way how to retrieve the Type of a field was changed.

Keep on rocking :)

[jkimbo]

Damn, you fix one thing and another issue pops up! Any idea what is going wrong here @lassesteffen ?

[lassesteffen]

Hey I updated the issue, something was not correctly saved by me. The actual issue is: Variable "userEmail" of type "String" used in position expecting type "ID".

[lassesteffen]

I see why it's doing what it does. It is not coded to work with filtering on foreign keys, which is the problem here. Because the user field on Membership is indeed of type ID. But the problem is I am actually trying to filter by the email of the User model (which the user field is linking to.)
The split in line 21 in utils.py breaks it, since it is written under the assumption that there are no accesses to foreign keys, and the email part of the name is just ignored. There should actually be some kind of loop to work with filtering on foreign keys.
Or am I missing something?

[curlywurlycraig]

@lassesteffen I'm seeing the same thing, but only when I have a single relation. As soon as I use two relations (as in filter_fields = ['user__friend__email'], for example) the type is appropriately inferred as a String.

I haven't had a chance to dive into the code or set up an example repo, but I can corroborate your findings at least partially.

[jkimbo]

@lassesteffen @curlywurlycraig sorry I haven't followed up on this. Have either of you got to a point where you could make a PR to fix this bug? (I'm pushed for time at the moment)

[zbyte64]

Looked at utils.get_filtering_args_from_filterset ; it does not recurse over foreign members when resolving field types. IE: this works: user__email , this does not: user__friend__email. The method looks at user__friend__email as filtering on a method user__friend which is not found thus falling back to what is provided as part of filterset_class.

Fortunately this is a common pattern and django-filter has implemented this as utils.get_model_field(model, field_name): https://github.com/carltongibson/django-filter/blob/master/django_filters/utils.py#L130
I suggest we import and replace our logic.

[jkimbo]

That sounds good @zbyte64 . Do you have time to make a PR with the changes?

[lassesteffen]

Any updates over here?

[lassesteffen]

Hey @jkimbo I would assist if there is anything to do to get the fixing PR merged! Just let me know

[jkimbo]

The fix for this has been released: https://github.com/graphql-python/graphene-django/releases/tag/v2.7.0