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

fix(fragment): merge the nested fragments fields (#8075) #8435

Merged
merged 6 commits into from
Nov 23, 2022

Conversation

dshekhar95
Copy link
Contributor

@dshekhar95 dshekhar95 commented Nov 16, 2022

@CLAassistant
Copy link

CLAassistant commented Nov 16, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ dshekhar95
❌ NamanJain8
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Nov 16, 2022
@dshekhar95
Copy link
Contributor Author

Have tested locally, fix works. Need to add unit test case still

@coveralls
Copy link

coveralls commented Nov 16, 2022

Coverage Status

Coverage increased (+0.01%) to 37.195% when pulling bd797b9 on dshekhar95-cherry-pick-8075 into bc5f584 on main.

@dshekhar95 dshekhar95 added the cherry-pick-22.0.2 Label for all cherry-picks for 22.0.2 Release label Nov 16, 2022
@MichelDiz
Copy link
Contributor

MichelDiz commented Nov 21, 2022

@dshekhar95 Do you have a test proof? that I can run in my end. Looks like you have fixed and send it to the user PoolShark correct? So his example could be a test.

BTW, can you sign the CLA?

@matthewmcneely
Copy link
Member

This one definitely needs tests.

@dshekhar95
Copy link
Contributor Author

@dshekhar95 Do you have a test proof? that I can run in my end. Looks like you have fixed and send it to the user PoolShark correct? So his example could be a test.

BTW, can you sign the CLA?

@dshekhar95 dshekhar95 closed this Nov 21, 2022
@dshekhar95 dshekhar95 reopened this Nov 21, 2022
Copy link
Contributor

@meghalims meghalims left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding test. Please go ahead and merge it

@dshekhar95 dshekhar95 merged commit 409d30c into main Nov 23, 2022
@dshekhar95 dshekhar95 deleted the dshekhar95-cherry-pick-8075 branch November 23, 2022 23:50
@damonfeldman
Copy link

Note this fixes a bug where a fragment is used in a query but data is not returned as required by the GraphQL spec. (See discuss link above for details)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph. cherry-pick-22.0.2 Label for all cherry-picks for 22.0.2 Release
Development

Successfully merging this pull request may close these issues.

8 participants