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

Is totalCount not implemented on Relay? #162

Closed
mraak opened this issue Apr 28, 2017 · 24 comments
Closed

Is totalCount not implemented on Relay? #162

mraak opened this issue Apr 28, 2017 · 24 comments

Comments

@mraak
Copy link

mraak commented Apr 28, 2017

This gives me an error in /graphql. Rest works like a charm though.

{
  "errors": [
    {
      "message": "Cannot query field \"totalCount\" on type \"ProductNodeConnection\".",
      "locations": [
        {
          "line": 3,
          "column": 5
        }
      ]
    }
  ]
}
query{
  allProducts(first:2 after:"YXJyYXljb25uZWN0aW9uOjM="){
    totalCount 
    pageInfo {
      hasNextPage
      hasPreviousPage
      startCursor
      endCursor
    }
  
    edges{  
      node{
        id
        name
      }
      cursor
    }
  }
}
@arianon
Copy link
Contributor

arianon commented Apr 28, 2017

Connections don't expose their length attribute by default. But you can do this:

class Product(graphene_django.DjangoObjectType):
    ...
    @classmethod
    def get_connection(cls):
        class CountableConnection(graphene.relay.Connection):
            total_count = graphene.Int()

            class Meta:
                name = '{}Connection'.format(cls._meta.name)
                node = cls

            @staticmethod
            def resolve_total_count(root, args, context, info):
                return root.length

        return CountableConnection

@mraak
Copy link
Author

mraak commented Apr 28, 2017

Not sure what you did there. If you could put it in the context of my code it would be more understandable, and if you provide some explanations, we could upgrade this question to the doc status :) Since Relay has the concept of totalCount, developers would expect it available in the query.

from graphene import relay, ObjectType, AbstractType, List
from graphene_django.types import DjangoObjectType
from graphene_django.filter import DjangoFilterConnectionField
from .models import Product

class ProductNode(DjangoObjectType):
    class Meta:
        model = Product
        filter_fields = {
            'name': ['exact', 'icontains', 'istartswith'],
        }
        interfaces = (relay.Node, )


class Query(AbstractType):
    all_products = DjangoFilterConnectionField(ProductNode)

    def resolve_all_products(self, args, context, info):
        return Product.objects.all()

@arianon
Copy link
Contributor

arianon commented Apr 28, 2017

Ok so here's what happens: when you implement the relay.Node interface, the Node.implements class method is called on the ObjectType you are defining, in your case it would be ProductNode.

Look at the method

def implements(cls, objecttype):
    get_connection = getattr(objecttype, 'get_connection', None)
    if not get_connection:
        get_connection = partial(get_default_connection, objecttype)

    objecttype.Connection = get_connection()

you'll notice it attemps to find a get_connection method from your ObjectType to define your Node's Connection custom implementation, and if it doesn't, it just uses the default Connection class, so if we take advantage of this then you can create a Connection that inherits from the default class and implement the totalCount attribute and resolver. (By the way, keep in mind that DjangoObjectType inherits from ObjectType)

...
from graphene import Int

class ProductNode(DjangoObjectType):
    class Meta:
        model = Product
        filter_fields = {
            'name': ['exact', 'icontains', 'istartswith'],
        }
        interfaces = (relay.Node, )

    @classmethod
    def get_connection(cls):
        class CountableConnection(relay.Connection):
            total_count = Int()

            class Meta:
                name = '{}Connection'.format(cls._meta.name)
                node = cls
            
            @staticmethod  # Redundant since Graphene kinda does this automatically for all resolve_ methods.
            def resolve_total_count(root, args, context, info):
                return root.length

        return CountableConnection

That's it... I hope. Also, thanks to how Python's scoping works, the outer cls method argument is passed to the class you define, which is why we are defining this subclass of Connection inside the ProductNode.get_connection method.

Also, the root.length attribute is defined here from here

Hope I cleared any doubts

@mraak
Copy link
Author

mraak commented Apr 28, 2017

Will try, but wouldn't it be easier to just stick is somewhere here in the Connection?

@arianon
Copy link
Contributor

arianon commented Apr 28, 2017

Yes, it definitely would. But I am merely proposing a solution that works with the current version of Graphene, though :)

@mraak
Copy link
Author

mraak commented Apr 28, 2017

I can wait for few days no worries :)

@Fercho191
Copy link

The solution worked for me.

Because this is scensial to paginate, I've done it as a Mixin to add it to multiple DjangoObjectType like this:

class TotalCountMixin(ObjectType):
    @classmethod
    def get_connection(cls):
        class CountableConnection(relay.Connection):
            total_count = Int()

            class Meta:
                name = '{}Connection'.format(cls._meta.name)
                node = cls

            @staticmethod
            def resolve_total_count(root, args, context, info):
                return root.length

        return CountableConnection

Then I added it to the DjangoObjectType, in your case it would be:

class ProductNode(DjangoObjectType, TotalCountMixin):
    class Meta:
        model = Product
        filter_fields = {
            'name': ['exact', 'icontains', 'istartswith'],
        }
        interfaces = (relay.Node, )

Test it!

@Eraldo
Copy link

Eraldo commented Sep 28, 2017

@Fercho191 Thank you for sharing your Mixin! :D

@dperetti
Copy link

The get_connection is no more in v.2 🤔

@aminland
Copy link

This is how to do it in version 2 (graphene is woefully undocumented) :(

class CountableConnectionBase(relay.Connection):
	class Meta:
		abstract = True

	total_count = Int()
	def resolve_total_count(self, info, **kwargs):
		return self.iterable.count()


class ProductNode(DjangoObjectType):
	class Meta:
		model = MyModel
		interfaces = (relay.Node, )
		connection_class = CountableConnectionBase

@igo
Copy link

igo commented Dec 2, 2017

Thanks @aminland. Unfortunately when I try make it using abstract class I get TypeError: Error when calling the metaclass bases __init_subclass_with_meta__() got an unexpected keyword argument 'connection_class' Any ideas?
Meanwhile I was able to make it work with

class EventType(DjangoObjectType):
    class Meta:
        model = Event
        interfaces = (graphene.relay.Node, )

class CountableConnection(graphene.relay.Connection):
    total_count = graphene.Int()

    class Meta:
        node = EventType

    def resolve_total_count(self, info):
        return self.iterable.count()

class UserProfilType(DjangoObjectType):
    events = graphene.relay.ConnectionField(CountableConnection)
    class Meta:
        model = UserProfil
        interfaces = (graphene.relay.Node, )

@oharlem
Copy link

oharlem commented Dec 4, 2017

@igo
Thank you, this works!
However, looks like it does not respect the first argument,
i.e. if I'm requesting just first 3 items, it will still get the full set.
Thoughts?

@aminland
Copy link

aminland commented Dec 4, 2017 via email

@oharlem
Copy link

oharlem commented Dec 4, 2017

@aminland
Sorry, got confused about the role of first. Working as expected.
(first should not be limiting the count value ).

@dvreed77
Copy link

@igo where does the Event model come from?

@nuschk
Copy link

nuschk commented Jan 4, 2018

@igo This worked fine, for relay.ConnectionField, but didn't for DjangoConnectionField, because it fails an assert in DjangoConnectionField.type() (it expects the type argument to the constructor (to which you pass CountableConnection) to be a subclass of DjangoObjectType). I think that assert is wrong, but honestly don't know for sure. It works though when you override and skip the assert like this:

class CustomDjangoConnectionField(DjangoConnectionField):
    @property
    def type(self):
        from .types import DjangoObjectType
        _type = super(ConnectionField, self).type
        # assert issubclass(_type, DjangoObjectType), "DjangoConnectionField only accepts DjangoObjectType types"
        assert _type._meta.connection, "The type {} doesn't have a connection".format(_type.__name__)
        return _type._meta.connection

Note the wonky call to super() to skip the baseclass.

I ended up implementing the connection type directly in type(), see my comment in #320 , as this is a common case in our code base.

@dani0805
Copy link

@aminland I think the version on pypi (https://pypi.python.org/pypi/graphene-django/2.0.0) does not expose connection_class

@aminland
Copy link

@dani0805 good point, I didn't realize that the change in 2a39f5d wasn't pushed to pypy yet. You can just change your requirements file and add -e git+https://github.com/graphql-python/graphene-django#egg=graphene-django to get the latest version.

@dani0805
Copy link

Yep that’s what we did but would be nice to have it in a stable release as this is necessary for pagination.

jole78 referenced this issue Feb 13, 2018
referred to as connection_class, it will instantiate the connection from the provided class or default to graphene.Connection if not supplied
@danpalmer
Copy link
Collaborator

Any reason why this hasn't been released on PyPI yet?

2a39f5d#diff-6926ea790e42fa924302b75462dfef72

It's been over 8 months, would be great to be able to use this sort of functionality.

@phalt
Copy link
Contributor

phalt commented May 3, 2019

This should have been released now so closing.

@phalt phalt closed this as completed May 3, 2019
@zbyte64
Copy link
Collaborator

zbyte64 commented Oct 13, 2020

IMHO totalCount should be part of the codebase

@artofhuman
Copy link
Contributor

@zbyte64 +1. Maybe we should reopen this issue?

@PabloCastellano
Copy link

Why is this issue closed? As far as I'm concerned, the issue is still there in v2.15.0 because totalCount is not implemented.

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

No branches or pull requests