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

Feat(GraphQL): Add count query feature at root to GraphQL #6786

Merged
merged 5 commits into from
Oct 29, 2020

Conversation

vmrajas
Copy link
Contributor

@vmrajas vmrajas commented Oct 22, 2020

Motivation:
The PR adds

  1. aggregateTypeName query and AggregateResultTypeName Type to GraphQL schema. This is used for adding support for Count Queries at root to GraphQL.
  2. Adds functionality for count queries at root in GraphQL.
  3. Added tests in graphql/resolve/
    Related Discuss Issue: https://discuss.dgraph.io/t/count-queries-in-graphql/10714/9
    This PR will be merged to rajas/count_queries_main which will then be merged to master branch once all features for handling count queries are present.

Fixes GRAPHQL-746


This change is Reviewable

@vmrajas vmrajas changed the title Add schema for aggregate queries Feat(GraphQL): Add count query feature at root to GraphQL Oct 27, 2020
@vmrajas vmrajas changed the base branch from rajas/count_queries_main to master October 28, 2020 11:49
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 52 of 52 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MichaelJCompton and @vmrajas)


graphql/e2e/auth/auth_test.go, line 895 at r1 (raw file):

	for _, tcase := range testCases {
		t.Run(tcase.role+tcase.user, func(t *testing.T) {
			getUserParams := &common.GraphQLParams{

Just params is enough instead of getUserParams


graphql/e2e/auth/auth_test.go, line 956 at r1 (raw file):

	for _, tcase := range testCases {
		t.Run(tcase.role+tcase.user, func(t *testing.T) {
			getUserParams := &common.GraphQLParams{

params

Copy link
Contributor Author

@vmrajas vmrajas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)


graphql/e2e/auth/auth_test.go, line 895 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Just params is enough instead of getUserParams

Done.


graphql/e2e/auth/auth_test.go, line 956 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

params

Done.

@vmrajas vmrajas merged commit 4f4cf47 into master Oct 29, 2020
@vmrajas vmrajas deleted the rajas/count_queries_schema branch October 29, 2020 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants