Filter by id causes other filters to be ignored

[claradaia]

Hello, I find that if I make a query with a filter by id, other filters are ignored. For example, in the following query:

query players {{
    players(
        id: "UmVwb3J0ZXJOb2RlOjE=",  # id of some player with first_name == "Larry"
        firstName: "Michael"
    ) {{
        edges {{
            node {{
                id
                firstName
            }}
        }}
    }}
}}

I expected an empty list, because the filters should "cancel", but it actually retrieves the "Larry" player.

I created a PR with a test case that reproduces this -- only in relay, but I can check if it happens without relay too.

Is it supposed to be this way? If not, I can try and fix it next week.

[claradaia]

Ok, so I checked the commit history and apparently this commit introduces this new "if" that uses get_node_from_global_id and returns the node if id is present in the filters. I believe this excerpt:

        _id = args.pop('id', None)

        if _id is not None:
            iterables = [get_node_from_global_id(self.node_type, info, _id)]
            list_length = 1
        elif callable(getattr(self.model, "objects", None)):
            iterables = self.get_queryset(self.model, info, **args)
            list_length = iterables.count()
        else:
            iterables = []
            list_length = 0

should be changed to

        _id = args.pop('id', None)
        # code that existed before to replace the id filter with 'pk'
        if _id is not None:
                args['pk'] = global_id_via_node(self.node_type, _id)[-1]

        if callable(getattr(self.model, "objects", None)):
            iterables = self.get_queryset(self.model, info, **args)
            list_length = iterables.count()
        else:
            iterables = []
            list_length = 0

Thoughts?

[claradaia]

I updated the PR with something similar and my test case passed, please let me know if there's a better way to do it.

[abawchen]

@claradaia : Thanks for the PR, and it looks good to me. Merged.