Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter pages with page model field #219

Open
dilonne opened this issue Mar 1, 2022 · 10 comments · May be fixed by #225
Open

Filter pages with page model field #219

dilonne opened this issue Mar 1, 2022 · 10 comments · May be fixed by #225
Labels
bug Something isn't working

Comments

@dilonne
Copy link

dilonne commented Mar 1, 2022

How can one use a page model field besides ID to filter pages? Decorating a page model with @register_query_field allows custom query parameters to be used with the singular query but not with the plural query.

@zerolab
Copy link
Member

zerolab commented Mar 1, 2022

Have you tried @register_paginated_query_field ?

@dilonne
Copy link
Author

dilonne commented Mar 1, 2022

Let me give it a try

@dilonne
Copy link
Author

dilonne commented Mar 1, 2022

@register_paginated_query_field only adds two more arguments(page and per page) to the plural query

@zerolab
Copy link
Member

zerolab commented Mar 1, 2022

The documentation says:

You can add custom query parameters like so:

@register_paginated_query_field(
    "advert",
    "adverts",
    {
        "id": graphene.Int(),
        "url": graphene.String(),
    },
)

then use it in your query:

{
    # Get a specific advert
    advert(url: "some-unique-url") {
        url
        text
    }
}

so you should be able to replace url with you field in the definition

@dilonne
Copy link
Author

dilonne commented Mar 2, 2022

"advert" is the singular query in this case so that should work, my question is how do you get to specify such on the plural query "adverts", suppose we had a non unique field 'category' field on the model Advert and we want to query all adverts with that particular category

@register_paginated_query_field(
    "advert",
    "adverts",
    {
        "id": graphene.Int(),
        "url": graphene.String(),
        "category": graphene.Int()
    },
)
adverts(category: 2){
      url
}

The above wont work with the plural query which is supposed to return a list, adverts won't recognise category as an argument

advert(category:  2){
    url
}

The above will recognize category as an argument but isnt for returning lists.

My question is how do we get plural queries to recognize custom query arguments

@dilonne
Copy link
Author

dilonne commented Mar 2, 2022

Ended up overriding @register_query_field to allow the plural query to be able to use custom query params just like the singular query

@dilonne dilonne closed this as completed Mar 2, 2022
@zerolab
Copy link
Member

zerolab commented Mar 2, 2022

Hey @dilonne,

Can you share your overrides? I don't have lots of time this week, but keen to improve the developer experience, or documentation where possible

@flyte
Copy link

flyte commented Apr 8, 2022

Sorry to do this twice in one day, but this is also still an issue in the latest version! I'm struggling to work out how to add filters to the plural query. @zerolab perhaps this should be reopened too?

flyte added a commit to flyte/wagtail-grapple that referenced this issue Apr 9, 2022
@zerolab zerolab reopened this Apr 23, 2022
@zerolab zerolab linked a pull request Apr 23, 2022 that will close this issue
@zerolab zerolab linked a pull request Sep 8, 2022 that will close this issue
@dopry
Copy link
Collaborator

dopry commented May 12, 2023

@flyte @zerolab I think the underlying issue here may be that query_params isn't documented... looking at the code in register_singular_query_field it looks like the query params are passed to the filter function in the resolver. But in register_paginated_query_field ti doesn't seem to be, and this will likely break if search_query is set, since .search() handles filters differently...

@dopry dopry added the bug Something isn't working label May 12, 2023
@dopry
Copy link
Collaborator

dopry commented May 12, 2023

dug a little more and...

register_paginated_query_field suffers from the same limitation where kwargs is passed to get in resolve_singular, and to resolve_paginated_queryset in resolve_plural, but then resolve_paginated_queryset never passes **kwargs to filter.

There are a few routes a fix could probably go.

  1. register_query_field.inner.Mixin.resolve_plural could pass qs.filter(**kwargs) to resolve_queryset instead of qs.all()
  2. resolve_queryset could be modified to call qs.filter(**kwargs).

It may be better to apply kwargs in resolve_queryset so that any special conditions around search filters can be properly handled.

It looks like the same change in both resolve_queryset and resolve_paginated_queryset would address this...

- qs = qs.all()
+ qs = qs.filter(**kwargs)

At the same time we fix this we should probably document that the syntax for these argument name should use django field lookup syntax ex) field: value for exact match, field__gt: value for a greater than match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants