From 398c085295d992658a9e7e22aae037f55528c258 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Thu, 9 Sep 2021 09:57:36 +1000 Subject: [PATCH] Add more tests for error cases when ordering/filtering (#6505) --- .changeset/twenty-buttons-teach.md | 6 + .../src/lib/core/queries/resolvers.ts | 2 + .../keystone/src/lib/core/where-inputs.ts | 6 +- tests/api-tests/queries/filters.test.ts | 124 +++++++++++++++++- tests/api-tests/queries/orderBy.test.ts | 32 ++++- 5 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 .changeset/twenty-buttons-teach.md diff --git a/.changeset/twenty-buttons-teach.md b/.changeset/twenty-buttons-teach.md new file mode 100644 index 00000000000..808e255e93c --- /dev/null +++ b/.changeset/twenty-buttons-teach.md @@ -0,0 +1,6 @@ +--- +'@keystone-next/keystone': patch +'@keystone-next/api-tests-legacy': patch +--- + +Improved error message for bad relationship filter inputs. diff --git a/packages/keystone/src/lib/core/queries/resolvers.ts b/packages/keystone/src/lib/core/queries/resolvers.ts index 2f9876761a2..9b23220a3de 100644 --- a/packages/keystone/src/lib/core/queries/resolvers.ts +++ b/packages/keystone/src/lib/core/queries/resolvers.ts @@ -147,6 +147,8 @@ async function resolveOrderBy( const resolve = field.input!.orderBy!.resolve; const resolvedValue = resolve ? await resolve(value, context) : value; if (field.dbField.kind === 'multi') { + // Note: no built-in field types support multi valued database fields *and* orderBy. + // This code path is only relevent to custom fields which fit that criteria. const keys = Object.keys(resolvedValue); if (keys.length !== 1) { throw new Error( diff --git a/packages/keystone/src/lib/core/where-inputs.ts b/packages/keystone/src/lib/core/where-inputs.ts index 5f36b42901c..77436b9bf10 100644 --- a/packages/keystone/src/lib/core/where-inputs.ts +++ b/packages/keystone/src/lib/core/where-inputs.ts @@ -86,10 +86,10 @@ export async function resolveWhereInput( Object.entries(value).map(async ([key, val]) => { if (val === null) { throw new Error( - `The key ${key} in a many relation filter cannot be set to null` + `The key "${key}" in a many relation filter cannot be set to null` ); } - return [key, await whereResolver(val as any)]; + return [key, await whereResolver(val)]; }) ) ); @@ -106,6 +106,8 @@ export async function resolveWhereInput( : value; if (ret === null) { if (field.dbField.kind === 'multi') { + // Note: no built-in field types support multi valued database fields *and* filtering. + // This code path is only relevent to custom fields which fit that criteria. throw new Error('multi db fields cannot return null from where input resolvers'); } return { [fieldKey]: null }; diff --git a/tests/api-tests/queries/filters.test.ts b/tests/api-tests/queries/filters.test.ts index 82023a1dee4..6efb773192f 100644 --- a/tests/api-tests/queries/filters.test.ts +++ b/tests/api-tests/queries/filters.test.ts @@ -1,22 +1,24 @@ import { text, relationship } from '@keystone-next/keystone/fields'; import { createSchema, list } from '@keystone-next/keystone'; import { setupTestRunner } from '@keystone-next/keystone/testing'; -import { apiTestConfig } from '../utils'; +import { apiTestConfig, expectInternalServerError } from '../utils'; const runner = setupTestRunner({ config: apiTestConfig({ lists: createSchema({ User: list({ fields: { - noDash: text({ isFilterable: true }), + noDash: text({ isFilterable: true, isOrderable: true }), single_dash: text({ isFilterable: true }), many_many_many_dashes: text({ isFilterable: true }), multi____dash: text({ isFilterable: true }), + email: text({ isIndexed: 'unique', isFilterable: true }), }, }), SecondaryList: list({ fields: { someUser: relationship({ ref: 'User', isFilterable: true }), + otherUsers: relationship({ ref: 'User', isFilterable: true, many: true }), }, }), }), @@ -68,3 +70,121 @@ describe('filtering on field name', () => { }) ); }); + +describe('filtering on relationships', () => { + test( + 'findMany throws error with null relationship query', + runner(async ({ graphQLRequest }) => { + const { body } = await graphQLRequest({ + query: '{ secondaryLists(where: { otherUsers: null }) { id } }', + }); + // Returns null and throws an error + expect(body.data).toEqual({ secondaryLists: null }); + expectInternalServerError(body.errors, false, [ + { + message: 'A many relation filter cannot be set to null', + path: ['secondaryLists'], + }, + ]); + }) + ); + + test( + 'findMany throws error with null relationship query value', + runner(async ({ graphQLRequest }) => { + const { body } = await graphQLRequest({ + query: '{ secondaryLists(where: { otherUsers: { some: null } }) { id } }', + }); + // Returns null and throws an error + expect(body.data).toEqual({ secondaryLists: null }); + expectInternalServerError(body.errors, false, [ + { + message: 'The key "some" in a many relation filter cannot be set to null', + path: ['secondaryLists'], + }, + ]); + }) + ); + + test( + 'findMany returns all items with empty relationship query value', + runner(async ({ context, graphQLRequest }) => { + await context.lists.SecondaryList.createOne({ + data: { otherUsers: { create: [{ noDash: 'a' }, { noDash: 'b' }] } }, + }); + const { body } = await graphQLRequest({ + query: + '{ secondaryLists(where: { otherUsers: {} }) { otherUsers(orderBy: { noDash: asc }) { noDash } } }', + }); + // Returns all the data + expect(body.errors).toBe(undefined); + expect(body.data).toEqual({ + secondaryLists: [{ otherUsers: [{ noDash: 'a' }, { noDash: 'b' }] }], + }); + }) + ); +}); + +describe('searching by unique fields', () => { + test( + 'findOne works on a unique text field', + runner(async ({ context }) => { + const item = await context.lists.User.createOne({ data: { email: 'test@example.com' } }); + const { data, errors } = await context.graphql.raw({ + query: '{ user(where: { email: "test@example.com" }) { id email } }', + }); + expect(errors).toBe(undefined); + expect(data).toEqual({ user: { id: item.id, email: 'test@example.com' } }); + }) + ); + + test( + 'findOne throws error with zero where values', + runner(async ({ graphQLRequest }) => { + const { body } = await graphQLRequest({ query: '{ user(where: {}) { id email } }' }); + // Returns null and throws an error + expect(body.data).toEqual({ user: null }); + expectInternalServerError(body.errors, false, [ + { + message: 'Exactly one key must be passed in a unique where input but 0 keys were passed', + path: ['user'], + }, + ]); + }) + ); + + test( + 'findOne throws error with more than one where values', + runner(async ({ context, graphQLRequest }) => { + const item = await context.lists.User.createOne({ data: { email: 'test@example.com' } }); + const { body } = await graphQLRequest({ + query: `{ user(where: { id: "${item.id}" email: "test@example.com" }) { id email } }`, + }); + // Returns null and throws an error + expect(body.data).toEqual({ user: null }); + expectInternalServerError(body.errors, false, [ + { + message: 'Exactly one key must be passed in a unique where input but 2 keys were passed', + path: ['user'], + }, + ]); + }) + ); + + test( + 'findOne throws error with null where values', + runner(async ({ graphQLRequest }) => { + const { body } = await graphQLRequest({ + query: '{ user(where: { email: null }) { id email } }', + }); + // Returns null and throws an error + expect(body.data).toEqual({ user: null }); + expectInternalServerError(body.errors, false, [ + { + message: 'The unique value provided in a unique where input must not be null', + path: ['user'], + }, + ]); + }) + ); +}); diff --git a/tests/api-tests/queries/orderBy.test.ts b/tests/api-tests/queries/orderBy.test.ts index a2fbdd89626..972a71c5bcf 100644 --- a/tests/api-tests/queries/orderBy.test.ts +++ b/tests/api-tests/queries/orderBy.test.ts @@ -250,7 +250,7 @@ describe('Ordering by a single field', () => { ); test( - 'Multi filter, bad format throws error ', + 'Multi filter, multiple keys throws error ', runner(async ({ context, graphQLRequest }) => { await initialiseData({ context }); @@ -263,4 +263,34 @@ describe('Ordering by a single field', () => { ]); }) ); + + test( + 'Multi filter, zero keys throws error ', + runner(async ({ context, graphQLRequest }) => { + await initialiseData({ context }); + + const { body } = await graphQLRequest({ + query: 'query { users(orderBy: [{}]) { id } }', + }); + expect(body.data).toEqual({ users: null }); + expectBadUserInput(body.errors, [ + { path: ['users'], message: 'Only a single key must be passed to UserOrderByInput' }, + ]); + }) + ); + + test( + 'Multi filter, null values throws error ', + runner(async ({ context, graphQLRequest }) => { + await initialiseData({ context }); + + const { body } = await graphQLRequest({ + query: 'query { users(orderBy: [{ a: null }]) { id } }', + }); + expect(body.data).toEqual({ users: null }); + expectBadUserInput(body.errors, [ + { path: ['users'], message: 'null cannot be passed as an order direction' }, + ]); + }) + ); });