Merge pull request #68 from pjweisberg/remove-string-ids

Better handling of string IDs

Author: alexisjanvier

src/resolver/Mutation/remove.js

@@ -1,10 +1,14 @@
 export default (entityData = []) => (_, { id }) => {
-    const parsedId = parseInt(id, 10); // FIXME fails for non-integer ids
-    const indexOfEntity = entityData.findIndex(e => e.id === parsedId);
     let removedEntity = undefined;
+    if (id != null) {
+        const stringId = id.toString();
+        const indexOfEntity = entityData.findIndex(
+            e => e.id != null && e.id.toString() === stringId
+        );
 
-    if (indexOfEntity !== -1) {
-        removedEntity = entityData.splice(indexOfEntity, 1)[0];
+        if (indexOfEntity !== -1) {
+            removedEntity = entityData.splice(indexOfEntity, 1)[0];
+        }
     }
     return removedEntity;
 };

src/resolver/Mutation/remove.spec.js

@@ -27,3 +27,21 @@ test('removes record when found', () => {
     remove(data)(null, { id: 1 });
     expect(data).toEqual([{ id: 2, value: 'bar' }]);
 });
+
+test('removes with string data id', () => {
+    const data = [{ id: '1', value: 'foo' }, { id: '2', value: 'bar' }];
+    remove(data)(null, { id: 1 });
+    expect(data).toEqual([{ id: '2', value: 'bar' }]);
+});
+
+test('removes with string input and data ids', () => {
+    const data = [{ id: 'abc', value: 'foo' }, { id: 'def', value: 'bar' }];
+    remove(data)(null, { id: 'abc' });
+    expect(data).toEqual([{ id: 'def', value: 'bar' }]);
+});
+
+test("doesn't confuse undefined id with the id 'undefined'", () => {
+    const data = [{ value: 'foo' }, { id: 'def', value: 'bar' }];
+    expect(remove(data)(null, { id: 'undefined' })).toBeUndefined();
+    expect(data).toEqual([{ value: 'foo' }, { id: 'def', value: 'bar' }]);
+});

src/resolver/Mutation/update.js

@@ -1,14 +1,18 @@
 export default (entityData = []) => (_, params) => {
-    const parsedId = parseInt(params.id, 10); // FIXME fails for non-integer ids
-    const indexOfEntity = entityData.findIndex(
-        e => parseInt(e.id, 10) === parsedId
-    );
-    if (indexOfEntity !== -1) {
-        entityData[indexOfEntity] = Object.assign(
-            {},
-            entityData[indexOfEntity],
-            params
+    let updatedEntity = undefined;
+    if (params.id != null) {
+        const stringId = params.id.toString();
+        const indexOfEntity = entityData.findIndex(
+            e => e.id != null && e.id.toString() === stringId
         );
-        return entityData[indexOfEntity];
+        if (indexOfEntity !== -1) {
+            entityData[indexOfEntity] = Object.assign(
+                {},
+                entityData[indexOfEntity],
+                params
+            );
+            updatedEntity = entityData[indexOfEntity];
+        }
     }
+    return updatedEntity;
 };

src/resolver/Mutation/update.spec.js

@@ -24,8 +24,22 @@ test('updates record when found', () => {
     expect(data).toEqual([{ id: 1, value: 'bar', bar: 'baz' }]);
 });
 
+test('updates record with string id', () => {
+    const data = [{ id: 'abc', value: 'foo' }];
+    update(data)(null, { id: 'abc', value: 'bar', bar: 'baz' });
+    expect(data).toEqual([{ id: 'abc', value: 'bar', bar: 'baz' }]);
+});
+
 test('removes property when setting the value to undefined', () => {
     const data = [{ id: 1, value: 'foo' }];
     update(data)(null, { id: 1, value: undefined });
     expect(data).toEqual([{ id: 1 }]);
 });
+
+test("doesn't confuse undefined id with the id 'undefined'", () => {
+    const data = [{ value: 'foo' }];
+    expect(
+        update(data)(null, { id: 'undefined', value: 'bar', bar: 'baz' })
+    ).toBeUndefined();
+    expect(data).toEqual([{ value: 'foo' }]);
+});