removing by nonexisting ID deletes the last "record"

[robertbbb]

Hi,

It seems that the remove Mutation, if it receives a nonexisting ID, will still remove something (at random?)

Otherwise thanks for a great tool.

[pjweisberg]

From what I'm seeing, the remove mutation deletes the last record regardless of whether the ID exists or not.

[pjweisberg]

I'm using version 2.1.3 from npm. If I start it with this data file:

module.exports = {
  Orders: [
    {
      id: '12345',
      status: 'DONE',
    },
    {
      id: '12346',
      status: 'BLOCKED',
    },
    {
      id: '12347',
      status: 'DONE',
    },
  ],
};

then I run this mutation in GraphiQL:

mutation {
  removeOrder(id: "12345")
}

I get this response:

{
  "data": {
    "removeOrder": null
  }
}

Then I query for all orders again, and 12347 (the last one) is missing. 12345 (the one I told it to delete) is still there.

[svey]

The id attributes in your data files must be integers @pjweisberg @robertbbb —

Unfortunately, if you create something using mutation (not initial data) and then attempt to remove it your response will be:

{
  "data": {
    "something": null
  }
}

Interesting that only remove is broken and not update

— it's a bug 😞

[pjweisberg]

Interesting. Still a bug, but I guess that reduces the "how did anyone ever miss this?" factor. IDs still come out as strings in the query responses, which I why I didn't think of changing to integers.

In the real world IDs will frequently be UUIDs (or at least alphanumeric), but in my current project I can get away with assuming they're integers.

[svey]

I completely agree. And so does Graphql:

The ID scalar type represents a unique identifier, often used to refetch an object or as key for a cache. The ID type appears in a JSON response as a String; however, it is not intended to be human-readable. When expected as an input type, any string (such as "4") or integer (such as 4) input value will be accepted as an ID.

I was going through the code and found this— see comment in the remove resolver: https://github.com/marmelab/json-graphql-server/blob/master/src/resolver/Mutation/remove.js

So it seems the author is aware already. And it actually seems that the issue maybe on create.

[pjweisberg]

And there's the "remove last element" but, too. Not found, so indexOfEntity is -1, and that's what happens when you pass -1 into Array.splice().

That, at least, is easy to fix.

[a-rcrawford]

PJ Weisberg's recent merge in fixed the problem of arbitrarily deleting the last item when no such item ID was found. There is still a problem with not being able to delete IDs specified as strings, but we found that a small change to src/resolver/Mutation/remove.js fixed that bug:

export default (entityData = []) => (_, { id }) => {
const parsedId = ${id}; // surrounded by back-ticks that don't display on issues/66 properly.
const indexOfEntity = entityData.findIndex(e => ${e.id} === parsedId);
let removedEntity = undefined;

if (indexOfEntity !== -1) {
    removedEntity = entityData.splice(indexOfEntity, 1)[0];
}
return removedEntity;

};

[DarkEye123]

I still have this problem :( Removing existing or non-existing elements causes mentioned behavior and it is very problematic for me