Skip to content

Add list_length to connection returned by default_resolver in MongoengineConnectionField #72

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

Merged
merged 3 commits into from
Feb 13, 2019

Conversation

riverfr0zen
Copy link
Contributor

The recent changes to MongoengineConnectionField are great; however, one change was that the length of the queryset results is no longer applied to the connection object returned by the default_resolver.

This means that if someone were to implement a special Connection class such as below that provides a totalCount field (pretty common use case), the count() method has to be called a second time.

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

    total_count = graphene.Int()

    def resolve_total_count(self, info, **kwargs):
        # To get the totalCount with the current impl. we have to query again
        return self.iterable.count()

        # Would be better if we could return this, right?
        # return self.list_length

I'm not sure how much worse having to call count() a second time actually is (it might depend on various things), but I'd like opinions on whether it makes sense to just append the list_length to the connection so that we can avoid calling twice (as in this PR). Is there a reason not to append it?

Otherwise, would welcome suggestions on another way to avoid the second call.

@coveralls
Copy link

coveralls commented Feb 12, 2019

Pull Request Test Coverage Report for Build 224

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 96.939%

Totals Coverage Status
Change from base Build 219: 0.008%
Covered Lines: 380
Relevant Lines: 392

💛 - Coveralls

@zrlay
Copy link
Contributor

zrlay commented Feb 12, 2019

Makes a lot of sense. Calling .count() after the slicing that occurs in the connection resolver would not return the total count anyways, only the count of the queryset at that state. Good call to attach it when list_length is known.

@abawchen
Copy link
Collaborator

@riverfr0zen : Looks good 👍 , and it's better if you could provide corresponding test case :)
If this one is urgent for you, I can merge it first and put test case in another pr.
Please let me know, thanks.

@riverfr0zen
Copy link
Contributor Author

@abawchen There you go, I think that should cover it, let me know if otherwise.

@abawchen
Copy link
Collaborator

@riverfr0zen 👍 , merged.

@abawchen abawchen merged commit d6bd56f into graphql-python:master Feb 13, 2019
arunsureshkumar pushed a commit to arunsureshkumar/graphene-mongo that referenced this pull request Nov 2, 2020
Add list_length to connection returned by default_resolver in MongoengineConnectionField
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.

4 participants