Skip to content
This repository has been archived by the owner on Jul 8, 2023. It is now read-only.

Need help on customize relay.Connection #108

Closed
sahmadreza opened this issue Aug 29, 2022 · 9 comments
Closed

Need help on customize relay.Connection #108

sahmadreza opened this issue Aug 29, 2022 · 9 comments

Comments

@sahmadreza
Copy link

sahmadreza commented Aug 29, 2022

I have types like this and I want the CollectionNode to show photos with filter is_public=1 not all photos

types:

from strawberry_django_plus import gql
from strawberry_django_plus.gql import relay

@gql.django.type(Photo, filters=PhotoFilter, order=PhotoOrder)
class PhotoNode(relay.Node):
    id: gql.auto
    title: gql.auto

    @classmethod
    def get_queryset(cls, queryset: QuerySet[Photo], _) -> QuerySet[Photo]:
        return queryset.filter(is_public=True)

@gql.django.type(Collection, filters=CollectionFilter, order=CollectionOrder)
class CollectionNode(relay.Node):
    id: gql.auto
    photos: relay.Connection["PhotoNode"] = relay.connection()

schema :

@gql.type
class Query:
    """All available queries for this schema."""

    # Photos
    photo_list: relay.Connection[PhotoNode] = gql.django.connection(description='Return Photo connection with pagination information.')

    # Collection
    collection_list: relay.Connection[CollectionNode] = gql.django.connection(description='Return Collection connection with pagination information.')

schema = gql.Schema(
    query=Query,
    extensions=[
        SchemaDirectiveExtension,
        DjangoOptimizerExtension,
    ],
)

photo_list query is fine and shows public photos but collection_list -> photos not working and show all photos

@bellini666
Copy link
Member

Hi @sahmadreza ,

First of all, did you gql.django.type your CollectionNode? Maybe you just forgot in the examples, but that is important.

Also, can you try replacing photos: relay.Connection["PhotoNode"] = relay.connection() by photos: relay.Connection["PhotoNode"] = gql.django.connection() and try again?

@sahmadreza
Copy link
Author

sahmadreza commented Aug 30, 2022

hey @bellini666

Thanks for your reply.

First of all, did you gql.django.type your CollectionNode? Maybe you just forgot in the examples, but that is important.

Yes, I just forgot to add gql.django.type in the examples.

Also, can you try replacing photos: relay.Connection["PhotoNode"] = relay.connection() by photos: relay.Connection["PhotoNode"] = gql.django.connection() and try again?

I tried it but it didn't work and all the photos without any filter in collection_list were still displayed.

I found a solution which is like this:

...

@gql.django.type(Collection, filters=CollectionFilter, order=CollectionOrder)
class CollectionNode(relay.Node):
    ...

    @gql.django.connection
    def photos(self: Collection) -> List["PhotoNode"]:
        return self.photos.filter(is_public=True)

@sahmadreza
Copy link
Author

sahmadreza commented Aug 30, 2022

Now my question is, how can I put a filter for a single gql.django.node query

for example, I want to show just a photo with a filter is_public=True

@gql.type
class Query:
    """All available queries for this schema."""
    
    photo: Optional[PhotoNode] = gql.django.node(description='Get a single Photo detail.')

Query

photo(id: "UGhvdG9Ob2RlOjM2"){
    id
    title
    status
  }

@bellini666
Copy link
Member

I found a solution which is like this:

Interesting... I'll have to check why your previous solution did not work, since in theory there's not much difference between your previous code and your workaround (except from the fact that you filtered the query)

Now my question is, how can I put a filter for a single gql.django.node query

node is actually just a shortcut for defining a resolver that receives an ID and returns the object with the given node. It is not supposed to take any other parameters.

You can easily define your own custom field in this case and add extra parameters to it

@Nimov
Copy link

Nimov commented Oct 28, 2022

@bellini666 I'm having a very similar issue. Did you manage to check that?

Interesting... I'll have to check why your previous solution did not work, since in theory there's not much difference between your previous code and your workaround (except from the fact that you filtered the query)

@bellini666
Copy link
Member

Hey @Nimov ,

Not yet, but in theory this should be working.

Basically the relay resolver will call the type's resolve_node method, which in the case of a django type is defined to call this function:

def resolve_model_node(source, node_id, *, info: Optional[Info] = None, required=False):
. In there it checks if the type has a get_queryset attribute and calls it.

You can try checking if any of those steps are failing, that will bring us closer to the real issue

frleb pushed a commit to frleb/strawberry-django-plus that referenced this issue Feb 28, 2023
@LucasPickering
Copy link
Contributor

@bellini666 I'm running into this same issue. My schema looks similar to the original post: I have a connection as a subfield on a top-level node, and get_queryset isn't being called on the connection. I traced through the call stack and here's what I found:

In that last function, root will be the Django object for the root field (e.g. an instance of Collection), while name will be the name of the connection field (e.g. photos). So it's just grabbing whatever field is defined on the Django instance, and never checking for the get_queryset method. Then the resulting related queryset is used to instantiate a connection here: https://github.com/blb-ventures/strawberry-django-plus/blob/v2.0.5/strawberry_django_plus/relay.py#L1037

I'm not sure where in this chain the get_queryset check should be done, but it's definitely not happening right now. Let me know if you need any more info to debug. I'm going to start adding some code in places and see if I can get it to work.

@LucasPickering
Copy link
Contributor

LucasPickering commented Mar 3, 2023

Ok I got it to work pretty easily, with this change: main...LucasPickering:strawberry-django-plus:108-connection-fix

The trouble with this is that I'm afraid that removing the .all() will break other stuff that doesn't use get_queryset. @bellini666 I can put this up as a PR if you want though and we can work on it there.

@bellini666
Copy link
Member

@LucasPickering thanks for taking the time to look at that.

After seeing your PR, I wonder if it would be enough to do something similar to resolve_model_notes(https://github.com/blb-ventures/strawberry-django-plus/blob/main/strawberry_django_plus/utils/resolvers.py#L307), which is the resolver that gets called to resolve root connections.

You have the origin there and you can also add an info to the function to receive it. With that info you get_queryset if it exists in the origin and also call the optimizer on it if it is enabled.

The .filter() you did worked because .all() was probably cached by the optimizer. Probably we also want to skip the optimizer in that case to avoid it doing 2 queries (one for the cache, that is going to be thrown away anyway, and other for your new connection pagination)

Feel free to open a PR and we can continue the implementation discussion in there :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants