From 455724dd07ad715a7792967541a8e7efb8ec95a1 Mon Sep 17 00:00:00 2001
From: Daniel Chambers <daniel@hasura.io>
Date: Wed, 17 Jul 2024 18:36:30 +1000
Subject: [PATCH] Fixed command targeted relationships not using data connector
 argument names (#841)

### What
This PR fixes an issue where relationships that target commands do not
correctly use the data connector's argument name when making the ndc
request. Instead, they use the OpenDD argument name, which is incorrect.

For metadata where the OpenDD argument name is the same as the data
connector's argument name, the code works but only coincidentally.

### How
I've updated an existing test to change the name of the command argument
to be different from the data connector's argument name. This test
failed but is now fixed by this PR, which simply looks up the name of
the data connector argument name and uses that instead.

V3_GIT_ORIGIN_REV_ID: 71f1e812174c7bb9922792523129e4bcdce911ed
---
 v3/changelog.md                                 |  6 ++++++
 .../model_to_command/metadata.json              | 12 ++++++------
 v3/crates/execute/src/plan/error.rs             | 16 +++++++++++++++-
 v3/crates/execute/src/plan/relationships.rs     | 17 ++++++++++++++++-
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/v3/changelog.md b/v3/changelog.md
index c8326e0da43..1054d00e6a6 100644
--- a/v3/changelog.md
+++ b/v3/changelog.md
@@ -2,6 +2,12 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed a bug where command targeted relationships were not using the Open DD
+  argument name instead of the data connector's argument name when querying the
+  data connector
+
 ## [v2024.07.10]
 
 ### Fixed
diff --git a/v3/crates/engine/tests/execute/relationships/model_to_command/metadata.json b/v3/crates/engine/tests/execute/relationships/model_to_command/metadata.json
index 788ca8267d9..cecf3320c58 100644
--- a/v3/crates/engine/tests/execute/relationships/model_to_command/metadata.json
+++ b/v3/crates/engine/tests/execute/relationships/model_to_command/metadata.json
@@ -220,7 +220,7 @@
             "name": "get_movie_by_id",
             "arguments": [
               {
-                "name": "movie_id",
+                "name": "id",
                 "type": "Int!"
               }
             ],
@@ -231,7 +231,7 @@
                 "function": "get_movie_by_id"
               },
               "argumentMapping": {
-                "movie_id": "movie_id"
+                "id": "movie_id"
               }
             },
             "graphql": {
@@ -241,6 +241,8 @@
           }
         },
         {
+          "kind": "Relationship",
+          "version": "v1",
           "definition": {
             "sourceType": "actor",
             "name": "MovieFromCommand",
@@ -260,14 +262,12 @@
                 },
                 "target": {
                   "argument": {
-                    "argumentName": "movie_id"
+                    "argumentName": "id"
                   }
                 }
               }
             ]
-          },
-          "version": "v1",
-          "kind": "Relationship"
+          }
         }
       ]
     }
diff --git a/v3/crates/execute/src/plan/error.rs b/v3/crates/execute/src/plan/error.rs
index 85897db8a88..1325641f71c 100644
--- a/v3/crates/execute/src/plan/error.rs
+++ b/v3/crates/execute/src/plan/error.rs
@@ -1,4 +1,10 @@
-use open_dds::{relationships::RelationshipName, types::FieldName};
+use metadata_resolve::Qualified;
+use open_dds::{
+    arguments::ArgumentName,
+    commands::CommandName,
+    relationships::RelationshipName,
+    types::{CustomTypeName, FieldName},
+};
 use tracing_util::TraceableError;
 
 use crate::ndc;
@@ -25,6 +31,14 @@ pub enum InternalError {
         relationship_name: RelationshipName,
     },
 
+    #[error("Missing argument mapping to command {command_name} data connector source for argument {argument_name} used in relationship {relationship_name} on type {source_type}")]
+    MissingArgumentMappingInCommandRelationship {
+        source_type: Qualified<CustomTypeName>,
+        relationship_name: RelationshipName,
+        command_name: Qualified<CommandName>,
+        argument_name: ArgumentName,
+    },
+
     #[error("remote relationships should have been handled separately")]
     RemoteRelationshipsAreNotSupported,
 
diff --git a/v3/crates/execute/src/plan/relationships.rs b/v3/crates/execute/src/plan/relationships.rs
index 2dc19ff72f2..0cd8b96ebd7 100644
--- a/v3/crates/execute/src/plan/relationships.rs
+++ b/v3/crates/execute/src/plan/relationships.rs
@@ -190,9 +190,24 @@ pub(crate) fn process_command_relationship_definition(
                 name: ndc_models::FieldName::from(source_column.column.as_str()),
             };
 
+            let connector_argument_name = target_source
+                .details
+                .argument_mappings
+                .get(target_argument)
+                .ok_or_else(|| {
+                    error::Error::Internal(
+                        error::InternalError::MissingArgumentMappingInCommandRelationship {
+                            source_type: annotation.source_type.clone(),
+                            relationship_name: annotation.relationship_name.clone(),
+                            command_name: annotation.command_name.clone(),
+                            argument_name: target_argument.clone(),
+                        },
+                    )
+                })?;
+
             if arguments
                 .insert(
-                    ndc_models::ArgumentName::from(target_argument.as_str()),
+                    ndc_models::ArgumentName::from(connector_argument_name.as_str()),
                     relationship_argument,
                 )
                 .is_some()