Skip to content

Added support for total count on relay connections #104

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

Closed
wants to merge 1 commit into from

Conversation

CaselIT
Copy link
Contributor

@CaselIT CaselIT commented Dec 18, 2017

This PR adds support for returning the length of a ConnectionField under the argument totalCount
Example query:
{ allArticles { totalCount } }
will return
{ "allArticles": { "totalCount": <number of articles> } }

This option is added by default, but can be disabled by adding total_count = False to the Meta of a node line as in the example below:

class Role(SQLAlchemyObjectType):
    class Meta:
        model = RoleModel
        interfaces = (relay.Node, )
        # Disable the total count on this connection
        total_count = False

I've added the tests for this, but I do not know where should I add the documentation. What's your advice @syrusakbary
Closes #58

@coveralls
Copy link

coveralls commented Dec 18, 2017

Coverage Status

Coverage increased (+0.2%) to 91.186% when pulling 99ec6b4 on CaselIT:total_count into 4827ce2 on graphql-python:master.

@CaselIT
Copy link
Contributor Author

CaselIT commented Jan 5, 2018

@syrusakbary have you had a chance to review this pr?

Thanks

@CaselIT
Copy link
Contributor Author

CaselIT commented Jan 29, 2018

@syrusakbary sorry to bother you again. It's been more than a month without any feedback and I just wanted if you are still supporting this library or if I should consider creating another package
Thank you

@CaselIT CaselIT mentioned this pull request Jan 29, 2018
@microidea
Copy link

microidea commented Feb 1, 2018

@CaselIT I've already merged your code into my project 😂, can't wait that long.

@exit99
Copy link

exit99 commented Mar 8, 2018

Can we merge this in? Highly needed

@MichaelWashburnJr
Copy link

Why the hell was this never merged in?

@exit99
Copy link

exit99 commented Mar 13, 2018

@syrusakbary I'm assuming this was not merged in, as you can just override the classes yourself?
Or is there another reason?

@CaselIT
Copy link
Contributor Author

CaselIT commented Mar 13, 2018

@Kazanz you can override the class, but for it to work everywhere you need to patch the import in the library before creating any types with something along the lines of

from graphene_sqlalchemy import types
types.Connection = ConnectionWithCount

which is clearly not the best way to achieve this.

I'm not really sure why it has not been merged.

@nikordaris
Copy link

I've submitted a more general solution that will enable you to define your own Connection with count #120.

@CaselIT
Copy link
Contributor Author

CaselIT commented Apr 6, 2018

@nikordaris Nice. I'll close this one in favor of your implementation. Hopefully it will be merged..

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

Successfully merging this pull request may close these issues.

6 participants