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

Delete by nodeId doesn't work #71

Open
K-Markopoulos opened this issue Aug 11, 2021 · 6 comments
Open

Delete by nodeId doesn't work #71

K-Markopoulos opened this issue Aug 11, 2021 · 6 comments
Assignees
Labels

Comments

@K-Markopoulos
Copy link

I have a relation table that doesn't have an id column and I'm trying to delete a record by nodeId with custom mutation. I get the following error:

Variable "$input" of type "Delete[RELATION]Input!" used in position expecting type "Delete[RELATION]ByNodeIdInput!"

Seems like that Delete${resourceTypename}Input doesn't match with the input of ${deleteResourceName} as the latter includes the ByNodeId. Also, the variables object for this mutation (input: { id }) is incompatible with DeleteByNodeId which expects input { nodeId }.

Code of interest:

case DELETE: {
return {
variables: {
input: {
id: mapType(primaryKeyType, (params as DeleteParams).id),
},
},
query: gql`
mutation ${deleteResourceName}($input: Delete${resourceTypename}Input!) {
${deleteResourceName}(input: $input) {

Is this a bug or is there another way to delete a record when the table doesn't have id column? I tried some changes. but I wanted to make sure I'm not doing something wrong before I open a PR.

The following changes resolve the issue, but I'm not sure how to test them for other cases.

diff --git a/src/buildQuery.ts b/src/buildQuery.ts
index b21081d..9eddc24 100644
--- a/src/buildQuery.ts
+++ b/src/buildQuery.ts
@@ -243,12 +243,15 @@ export const buildQuery = (introspectionResults: IntrospectionResult, factory: F
     case DELETE: {
       return {
         variables: {
-          input: {
-            id: mapType(primaryKeyType, (params as DeleteParams).id),
-          },
+          input: mapInputToVariables(
+            params,
+            typeMap[`${capitalize(deleteResourceName)}Input`],
+            type,
+            typeMapConfiguration
+          ),
         },
         query: gql`
-          mutation ${deleteResourceName}($input: Delete${resourceTypename}Input!) {
+          mutation ${deleteResourceName}($input: ${capitalize(deleteResourceName)}Input!) {
             ${deleteResourceName}(input: $input) {
             ${singleLowerResourceName} {
             ${createQueryFromType(
@BowlingX
Copy link
Owner

Hi :) sorry for the late reply. This is indeed a bug; It actually affects more queries. I will Push a fix and add tests for this case. Thank you!

BowlingX added a commit that referenced this issue Sep 12, 2021
Reworked `non-id` primary keys to just alias the named primary key of the table instead of using `nodeId` - which would be inconsistent and not work for all cases e.g. `GET_MANY`. Fixes #71

BREAKING CHANGE: This might be a breaking change for some people who relied on `nodeId` semantics.
BowlingX pushed a commit that referenced this issue Sep 12, 2021
# [5.0.0](v4.5.0...v5.0.0) (2021-09-12)

### Bug Fixes

* non-id primary key handling. ([70c5bbc](70c5bbc)), closes [#71](#71)

### BREAKING CHANGES

* This might be a breaking change for some people who relied on `nodeId` semantics.
@BowlingX
Copy link
Owner

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@BowlingX
Copy link
Owner

I reworked the non-id primary key logic and removed nodeId from the current implementation; Instead I alias the pimary key name to create a consistent behaviour for all queries. I released it under the next tag in npm; Please try it out if it works for your usecase.

@BowlingX BowlingX self-assigned this Sep 12, 2021
@BowlingX BowlingX added the bug Something isn't working label Sep 12, 2021
@K-Markopoulos
Copy link
Author

Thanks for the fix! It works fine for non-id primary keys, but unfortunately it doesn't work when primary key consists of 2 columns, like relationship tables.
The queries are now using only the first key, but all keys are required.

@BowlingX
Copy link
Owner

Yes, indeed that might be a problem; I will check if I can implement something for this case; Not all features are supported then, but at least fetch one update etc should work.

@BowlingX BowlingX reopened this Sep 16, 2021
@BowlingX
Copy link
Owner

BowlingX commented Oct 1, 2021

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants