fix(GraphQL): Linking of xids for deep mutations

[pawanrawal]

For nested add mutations, the linking of nodes for xids wasn't working properly. Level 1 and 2 are linked properly but level 2 and beyond were not. This PR aims to fix that.
See https://discuss.dgraph.io/t/grandchild-is-not-attached-to-child-when-using-addtype/9224 for more details.

TODO

• Needs more testing for different combination of deeply nested id and xids.

---

This change is 

Docs Preview:

[harshil-goel]

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @pawanrawal)

---

graphql/resolve/mutation_rewriter.go, line 987 at r1 (raw file):

	var parentFrags []*mutationFragment

	// TODO - Find out when is withAdditionalDeletes true?

it's true when you do a "set" update mutation or add mutation.

---

graphql/resolve/mutation_rewriter.go, line 996 at r1 (raw file):

			if deepXID > 2 {
				// TODO - Find out why do we do this only when deepXID > 2. Also, this check can be
				// moved above with the xid != nil check.

We don't have to do this for if deepXID is 1, as it is a part of firstPass itself, no need to create new frags.

---

graphql/resolve/mutation_rewriter.go, line 1107 at r1 (raw file):

			res := make(map[string]interface{}, 1)
			res["uid"] = srcUID
			this := fmt.Sprintf("_:%s", variable)

Can we reuse addInverseLink function? That would check if parentTyp is nil too.

---

graphql/resolve/mutation_rewriter.go, line 1109 at r1 (raw file):

			this := fmt.Sprintf("_:%s", variable)
			if srcField.Type().ListType() != nil {
				res[parentTyp.DgraphPredicate(srcField.Name())] =

parentTyp can be nil, can cause an error.

---

graphql/resolve/mutation_rewriter.go, line 1115 at r1 (raw file):

					map[string]interface{}{"uid": this}
			}
			// addInverseLink(res, srcField.Inverse(), this)

No need.

[abhimanyusinghgaur]

We should add some e2e test for this too. Maybe a 4 level deep mutation with xids, and lists of xids with multiple children in each list, and hasInverses.

Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)

---

graphql/resolve/add_mutation_test.yaml, line 2280 at r2 (raw file):

- setjson: |
        {"uid":"uid(Comment16)"}
      cond: "@if(eq(len(Comment16), 1) AND eq(len(Comment14), 0))"

This condition seems repetitive, but doesn't seem to do anything anyways.

[pawanrawal]

I'll add the e2e tests in a separate PR.

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @abhimanyusinghgaur, @harshil-goel, and @MichaelJCompton)

---

graphql/resolve/add_mutation_test.yaml, line 2280 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

- setjson: |
        {"uid":"uid(Comment16)"}
      cond: "@if(eq(len(Comment16), 1) AND eq(len(Comment14), 0))"

This condition seems repetitive, but doesn't seem to do anything anyways.

Yeah, will have to have a look at it separately and if we can avoid it.

---

graphql/resolve/mutation_rewriter.go, line 987 at r1 (raw file):

Previously, harshil-goel (Harshil Goel) wrote…

it's true when you do a "set" update mutation or add mutation.

Done.

---

graphql/resolve/mutation_rewriter.go, line 996 at r1 (raw file):

Previously, harshil-goel (Harshil Goel) wrote…

We don't have to do this for if deepXID is 1, as it is a part of firstPass itself, no need to create new frags.

Done.

---

graphql/resolve/mutation_rewriter.go, line 1107 at r1 (raw file):

Previously, harshil-goel (Harshil Goel) wrote…

Can we reuse addInverseLink function? That would check if parentTyp is nil too.

Done.

---

graphql/resolve/mutation_rewriter.go, line 1109 at r1 (raw file):

Previously, harshil-goel (Harshil Goel) wrote…

parentTyp can be nil, can cause an error.

Done.

---

graphql/resolve/mutation_rewriter.go, line 1115 at r1 (raw file):

Previously, harshil-goel (Harshil Goel) wrote…

No need.

Done.