Fix MutationHandlers on field with args

Summary:
This should fix #3457

The bug is in calling `record.getLinkedRecord(payload.fieldKey, payload.args)`.  `getLinkedRecord` would append the serialized `args` to `fieldKey` as the final field name. However, `payload.fieldKey` already contains serialized arguments when it's populated using `getStorageKey` in `RelayRepsonseNormalizer`. We were duplicating the arguments in the field name.
Removing the `payload.args` to `record.getLinkedRecord` fixes the issue.

Reviewed By: voideanvalue

Differential Revision: D37732466

fbshipit-source-id: 9fc5a46643e463f265633b8390998c01e2f70b36

packages/relay-runtime/handlers/connection/MutationHandlers.js

@@ -104,11 +104,11 @@ function edgeUpdater(
     );
     let singleServerEdge, serverEdges;
     try {
-      singleServerEdge = record.getLinkedRecord(payload.fieldKey, payload.args);
+      singleServerEdge = record.getLinkedRecord(payload.fieldKey);
     } catch {}
     if (!singleServerEdge) {
       try {
-        serverEdges = record.getLinkedRecords(payload.fieldKey, payload.args);
+        serverEdges = record.getLinkedRecords(payload.fieldKey);
       } catch {}
     }
     if (singleServerEdge == null && serverEdges == null) {
@@ -181,11 +181,11 @@ function nodeUpdater(
     let singleServerNode;
     let serverNodes;
     try {
-      singleServerNode = record.getLinkedRecord(payload.fieldKey, payload.args);
+      singleServerNode = record.getLinkedRecord(payload.fieldKey);
     } catch {}
     if (!singleServerNode) {
       try {
-        serverNodes = record.getLinkedRecords(payload.fieldKey, payload.args);
+        serverNodes = record.getLinkedRecords(payload.fieldKey);
       } catch {}
     }
     if (singleServerNode == null && serverNodes == null) {

packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteMutationWithDeclarativeMutation-test.js

@@ -25,11 +25,13 @@ const {
 const RelayModernStore = require('../RelayModernStore');
 const RelayRecordSource = require('../RelayRecordSource');
 const {
+  disallowConsoleErrors,
   disallowWarnings,
   expectWarningWillFire,
 } = require('relay-test-utils-internal');
 
 disallowWarnings();
+disallowConsoleErrors();
 
 describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])(
   'deleteFromStore',
@@ -633,12 +635,14 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])(
       let store;
       let AppendCommentMutation;
       let AppendCommentsMutation;
+      let AppendCommentsWithArgsMutation;
       let PrependCommentMutation;
       let PrependCommentsMutation;
       let DeleteCommentMutation;
       let DeleteCommentsMutation;
       let appendOperation;
       let appendMultipleOperation;
+      let appendMultipleWithArgsOperation;
       let prependOperation;
       let prependMultipleOperation;
       let deleteOperation;
@@ -702,6 +706,26 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])(
           }
         `;
 
+        AppendCommentsWithArgsMutation = graphql`
+          mutation RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestAppendCommentsWithArgsMutation(
+            $connections: [ID!]!
+            $input: CommentCreateInput
+            $name: String!
+          ) {
+            commentCreate(input: $input) {
+              comment {
+                commentsFrom(name: $name)
+                  @appendEdge(connections: $connections) {
+                  cursor
+                  node {
+                    id
+                  }
+                }
+              }
+            }
+          }
+        `;
+
         PrependCommentMutation = graphql`
           mutation RelayModernEnvironmentExecuteMutationWithDeclarativeMutationTestPrependCommentMutation(
             $connections: [ID!]!
@@ -770,6 +794,14 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])(
             input: {},
           },
         );
+        appendMultipleWithArgsOperation = createOperationDescriptor(
+          AppendCommentsWithArgsMutation,
+          {
+            connections: [clientID],
+            name: 'Zuck',
+            input: {},
+          },
+        );
         prependOperation = createOperationDescriptor(PrependCommentMutation, {
           connections: [clientID],
           input: {},
@@ -1218,6 +1250,172 @@ describe.each(['RelayModernEnvironment', 'MultiActorEnvironment'])(
           ]);
         });
 
+        it('commits the mutation and inserts multiple comment edges on a field with args into the connection', () => {
+          const snapshot = environment.lookup(operation.fragment);
+          const callback = jest.fn();
+          environment.subscribe(snapshot, callback);
+
+          environment
+            .executeMutation({
+              operation: appendMultipleWithArgsOperation,
+            })
+            .subscribe(callbacks);
+
+          callback.mockClear();
+          subject.next({
+            data: {
+              commentCreate: {
+                comment: {
+                  id: 'unused-comment',
+                  commentsFrom: [
+                    {
+                      __typename: 'CommentsEdge',
+                      cursor: 'node-append-1',
+                      node: {
+                        __typename: 'Comment',
+                        id: 'node-append-1',
+                      },
+                    },
+                    {
+                      __typename: 'CommentsEdge',
+                      cursor: 'node-append-2',
+                      node: {
+                        __typename: 'Comment',
+                        id: 'node-append-2',
+                      },
+                    },
+                  ],
+                },
+              },
+            },
+          });
+          subject.complete();
+
+          expect(error).not.toBeCalled();
+          expect(complete).toBeCalled();
+          expect(callback.mock.calls.length).toBe(1);
+          // $FlowExpectedError[incompatible-use]
+          expect(callback.mock.calls[0][0].data.node.comments.edges).toEqual([
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'cursor-1',
+              node: {
+                __typename: 'Comment',
+                id: 'node-1',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'cursor-2',
+              node: {
+                __typename: 'Comment',
+                id: 'node-2',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-append-1',
+              node: {
+                __typename: 'Comment',
+                id: 'node-append-1',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-append-2',
+              node: {
+                __typename: 'Comment',
+                id: 'node-append-2',
+              },
+            },
+          ]);
+
+          environment
+            .executeMutation({
+              operation: prependMultipleOperation,
+            })
+            .subscribe(callbacks);
+
+          callback.mockClear();
+          subject.next({
+            data: {
+              commentsCreate: {
+                feedbackCommentEdges: [
+                  {
+                    __typename: 'CommentsEdge',
+                    cursor: 'node-prepend-1',
+                    node: {
+                      __typename: 'Comment',
+                      id: 'node-prepend-1',
+                    },
+                  },
+                  {
+                    __typename: 'CommentsEdge',
+                    cursor: 'node-prepend-2',
+                    node: {
+                      __typename: 'Comment',
+                      id: 'node-prepend-2',
+                    },
+                  },
+                ],
+              },
+            },
+          });
+          subject.complete();
+          expect(callback.mock.calls.length).toBe(1);
+          // $FlowExpectedError[incompatible-use]
+          expect(callback.mock.calls[0][0].data.node.comments.edges).toEqual([
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-prepend-2',
+              node: {
+                __typename: 'Comment',
+                id: 'node-prepend-2',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-prepend-1',
+              node: {
+                __typename: 'Comment',
+                id: 'node-prepend-1',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'cursor-1',
+              node: {
+                __typename: 'Comment',
+                id: 'node-1',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'cursor-2',
+              node: {
+                __typename: 'Comment',
+                id: 'node-2',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-append-1',
+              node: {
+                __typename: 'Comment',
+                id: 'node-append-1',
+              },
+            },
+            {
+              __typename: 'CommentsEdge',
+              cursor: 'node-append-2',
+              node: {
+                __typename: 'Comment',
+                id: 'node-append-2',
+              },
+            },
+          ]);
+        });
+
         it('inserts an comment edge during optmistic update, and reverts and inserts new edge when server payload resolves', () => {
           const snapshot = environment.lookup(operation.fragment);
           const callback = jest.fn();