Only coerce items API input when using resolveFields: false (#5517)

keystonejs · Apr 22, 2021 · a6cdf3d · a6cdf3d · vercel · Apr 22, 2021
1 parent f76938a
commit a6cdf3d
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 44 deletions.
diff --git a/.changeset/hot-poets-buy.md b/.changeset/hot-poets-buy.md
@@ -0,0 +1,5 @@
+---
+'@keystone-next/server-side-graphql-client-legacy': patch
+---
+
+Updated types to be more correct.
diff --git a/.changeset/lucky-apes-impress.md b/.changeset/lucky-apes-impress.md
@@ -0,0 +1,5 @@
+---
+'@keystone-next/keystone': patch
+---
+
+Fixed a bug using the items API to create/update `image` field items.
diff --git a/packages-next/fields/src/tests/test-fixtures.ts b/packages-next/fields/src/tests/test-fixtures.ts
@@ -44,7 +44,7 @@ export const filterTests = (withKeystone: any) => {
         listKey: 'Test',
         where,
         returnFields: 'id name',
-        sortBy: 'name_ASC',
+        sortBy: ['name_ASC'],
       })
     ).toEqual(expected);
 

diff --git a/packages-next/fields/src/types/autoIncrement/tests/test-fixtures.ts b/packages-next/fields/src/types/autoIncrement/tests/test-fixtures.ts
@@ -79,7 +79,7 @@ export const filterTests = (withKeystone: (arg: any) => any, matrixValue: Matrix
         listKey: 'Test',
         where,
         returnFields: 'name orderNumber',
-        sortBy: 'name_ASC',
+        sortBy: ['name_ASC'],
       })
     ).toEqual(expected.map(i => _storedValues[i]));
 

diff --git a/packages-next/fields/src/types/image/tests/test-fixtures.ts b/packages-next/fields/src/types/image/tests/test-fixtures.ts
@@ -68,22 +68,20 @@ export const crudTests = (keystoneTestWrapper: any) => {
       keystoneTestWrapper(async ({ context }: { context: any }) => {
         const filenames = ['keystone.jpeg', 'keystone.jpg', 'keystone'];
         for (const filename of filenames) {
-          const data = await createItem({
-            context,
-            listKey: 'Test',
-            item: { avatar: prepareFile(filename) },
-            returnFields: `
-            avatar {
-              id
-              mode
-              filesize
-              width
-              height
-              extension
-              ref
-              src
-            }
-        `,
+          const data = await context.lists.Test.createOne({
+            data: { avatar: prepareFile(filename) },
+            query: `
+              avatar {
+                id
+                mode
+                filesize
+                width
+                height
+                extension
+                ref
+                src
+              }
+          `,
           });
           expect(data).not.toBe(null);
           expect(data.avatar).toEqual({

diff --git a/packages-next/fields/src/types/timestamp/tests/test-fixtures.ts b/packages-next/fields/src/types/timestamp/tests/test-fixtures.ts
@@ -48,7 +48,7 @@ export const filterTests = (withKeystone: (args: any) => any) => {
     context: KeystoneContext,
     where: Record<string, any> | undefined,
     expected: any,
-    sortBy = 'name_ASC'
+    sortBy = ['name_ASC']
   ) =>
     expect(
       await getItems({
@@ -85,7 +85,7 @@ export const filterTests = (withKeystone: (args: any) => any) => {
               { name: 'person6', lastOnline: null },
               { name: 'person7', lastOnline: null },
             ],
-        'lastOnline_ASC'
+        ['lastOnline_ASC']
       )
     )
   );
@@ -115,7 +115,7 @@ export const filterTests = (withKeystone: (args: any) => any) => {
               { name: 'person2', lastOnline: '1980-10-01T23:59:59.999Z' },
               { name: 'person1', lastOnline: '1979-04-12T00:08:00.000Z' },
             ],
-        'lastOnline_DESC'
+        ['lastOnline_DESC']
       )
     )
   );

diff --git a/packages-next/keystone/src/lib/itemAPI.ts b/packages-next/keystone/src/lib/itemAPI.ts
@@ -61,21 +61,23 @@ export function itemAPIForList(
   return {
     findOne({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.findOne) throw new Error('You do not have access to this resource');
-      const args = getArgs.findOne(rawArgs) as { where: { id: string } };
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const args = rawArgs;
         return getItem({ listKey, context, returnFields, itemId: args.where.id });
       } else {
+        const args = getArgs.findOne(rawArgs) as { where: { id: string } };
         return list.itemQuery(args, context);
       }
     },
     findMany({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.findMany) throw new Error('You do not have access to this resource');
-      const args = getArgs.findMany(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const args = rawArgs;
         return getItems({ listKey, context, returnFields, ...args });
       } else {
+        const args = getArgs.findMany(rawArgs);
         return list.listQuery(args, context);
       }
     },
@@ -86,61 +88,67 @@ export function itemAPIForList(
     },
     createOne({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.createOne) throw new Error('You do not have access to this resource');
-      const { data } = getArgs.createOne(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { data } = rawArgs;
         return createItem({ listKey, context, returnFields, item: data });
       } else {
+        const { data } = getArgs.createOne(rawArgs);
         return list.createMutation(data, context);
       }
     },
     createMany({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.createMany) throw new Error('You do not have access to this resource');
-      const { data } = getArgs.createMany(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { data } = rawArgs;
         return createItems({ listKey, context, returnFields, items: data });
       } else {
+        const { data } = getArgs.createMany(rawArgs);
         return list.createManyMutation(data, context);
       }
     },
     updateOne({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.updateOne) throw new Error('You do not have access to this resource');
-      const { id, data } = getArgs.updateOne(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { id, data } = rawArgs;
         return updateItem({ listKey, context, returnFields, item: { id, data } });
       } else {
+        const { id, data } = getArgs.updateOne(rawArgs);
         return list.updateMutation(id, data, context);
       }
     },
     updateMany({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.updateMany) throw new Error('You do not have access to this resource');
-      const { data } = getArgs.updateMany(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { data } = rawArgs;
         return updateItems({ listKey, context, returnFields, items: data });
       } else {
+        const { data } = getArgs.updateMany(rawArgs);
         return list.updateManyMutation(data, context);
       }
     },
     deleteOne({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.deleteOne) throw new Error('You do not have access to this resource');
-      const { id } = getArgs.deleteOne(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { id } = rawArgs;
         return deleteItem({ listKey, context, returnFields, itemId: id });
       } else {
+        const { id } = getArgs.deleteOne(rawArgs);
         return list.deleteMutation(id, context);
       }
     },
     deleteMany({ query, resolveFields, ...rawArgs }) {
       if (!getArgs.deleteMany) throw new Error('You do not have access to this resource');
-      const { ids } = getArgs.deleteMany(rawArgs);
       const returnFields = defaultQueryParam(query, resolveFields);
       if (returnFields) {
+        const { ids } = rawArgs;
         return deleteItems({ listKey, context, returnFields, items: ids });
       } else {
+        const { ids } = getArgs.deleteMany(rawArgs);
         return list.deleteManyMutation(ids, context);
       }
     },

diff --git a/packages/server-side-graphql-client/src/server-side-graphql-client.ts b/packages/server-side-graphql-client/src/server-side-graphql-client.ts
@@ -11,12 +11,12 @@ const _runChunkedMutation = async ({
   query: string | DocumentNode;
   gqlName: string;
   pageSize: number;
-  items: Record<string, any>[];
+  items: readonly (Record<string, any> | string)[];
   context: KeystoneContext;
 }): Promise<Record<string, any>[]> => {
   if (pageSize <= 0) pageSize = 1;
 
-  const chunks = items.reduce((accum: Record<string, any>[][], item, index) => {
+  const chunks = items.reduce((accum: (Record<string, any> | string)[][], item, index) => {
     const chunkIndex = Math.floor(index / pageSize);
 
     if (!accum[chunkIndex]) {
@@ -69,7 +69,7 @@ const createItems = async ({
   context,
 }: {
   listKey: string;
-  items: Record<string, any>[];
+  items: readonly { readonly data: Record<string, any> }[];
   pageSize?: number;
   returnFields?: string;
   context: KeystoneContext;
@@ -118,10 +118,10 @@ const getItems = async ({
   context,
 }: {
   listKey: string;
-  where?: Record<string, any>;
-  sortBy?: string;
-  first?: number;
-  skip?: number;
+  where?: Record<string, any> | null;
+  sortBy?: readonly string[] | null;
+  first?: number | null;
+  skip?: number | null;
   pageSize?: number;
   returnFields?: string;
   context: KeystoneContext;
@@ -156,7 +156,10 @@ const getItems = async ({
     allItems.push(...latestResult);
 
     _skip += pageSize;
-  } while (latestResult.length === _first && (first === undefined || allItems.length < first));
+  } while (
+    latestResult.length === _first &&
+    (first === undefined || first === null || allItems.length < first)
+  );
 
   return allItems;
 };
@@ -191,7 +194,7 @@ const updateItems = async ({
   context,
 }: {
   listKey: string;
-  items: Record<string, any>[];
+  items: readonly { readonly id: string; readonly data: Record<string, any> }[];
   pageSize?: number;
   returnFields?: string;
   context: KeystoneContext;
@@ -241,7 +244,7 @@ const deleteItems = async ({
   listKey: string;
   pageSize?: number;
   returnFields?: string;
-  items: Record<string, any>[];
+  items: readonly string[];
   context: KeystoneContext;
 }) => {
   const { deleteManyMutationName } = context.gqlNames(listKey);

diff --git a/tests/api-tests/fields/crud.test.ts b/tests/api-tests/fields/crud.test.ts
@@ -104,7 +104,7 @@ multiAdapterRunners().map(({ runner, provider }) =>
                     context,
                     listKey,
                     returnFields,
-                    sortBy: 'name_ASC',
+                    sortBy: ['name_ASC'],
                   });
                   return wrappedFn({ context, listKey, items });
                 };

diff --git a/tests/api-tests/fields/filter.test.ts b/tests/api-tests/fields/filter.test.ts
@@ -83,7 +83,7 @@ multiAdapterRunners().map(({ runner, provider }) =>
                 context: KeystoneContext,
                 where: Record<string, any> | undefined,
                 expected: any[],
-                sortBy = 'name_ASC'
+                sortBy = ['name_ASC']
               ) =>
                 expect(await getItems({ context, listKey, where, returnFields, sortBy })).toEqual(
                   expected.map(i => storedValues[i])

diff --git a/tests/api-tests/server-side-graphql-client.test.ts b/tests/api-tests/server-side-graphql-client.test.ts
@@ -186,7 +186,7 @@ multiAdapterRunners().map(({ runner, provider }) =>
           await seedDb({ context });
 
           const getItemsBySortOrder = (sortBy: string) =>
-            getItems({ context, listKey, returnFields, sortBy });
+            getItems({ context, listKey, returnFields, sortBy: [sortBy] });
 
           const allItemsAscAge = await getItemsBySortOrder('age_ASC');
           const allItemsDescAge = await getItemsBySortOrder('age_DESC');
@@ -206,7 +206,7 @@ multiAdapterRunners().map(({ runner, provider }) =>
               returnFields,
               pageSize,
               first,
-              sortBy: 'age_ASC',
+              sortBy: ['age_ASC'],
             });
           expect(await getFirstItems(9, 5)).toEqual(testData.slice(0, 9).map(d => d.data));
           expect(await getFirstItems(5, 9)).toEqual(testData.slice(0, 5).map(d => d.data));
@@ -236,7 +236,7 @@ multiAdapterRunners().map(({ runner, provider }) =>
           await seedDb({ context });
           const first = 4;
           const getSortItems = (sortBy: string) =>
-            getItems({ context, listKey, returnFields, skip: 3, first, sortBy });
+            getItems({ context, listKey, returnFields, skip: 3, first, sortBy: [sortBy] });
           const itemsDESC = await getSortItems('age_DESC');
           expect(itemsDESC.length).toEqual(first);
           expect(itemsDESC[0]).toEqual({ name: 'test46', age: 460 });