Skip to content

Commit

Permalink
Add more tests for error cases when ordering/filtering (#6505)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Sep 8, 2021
1 parent 10c61bd commit 398c085
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .changeset/twenty-buttons-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@keystone-next/keystone': patch
'@keystone-next/api-tests-legacy': patch
---

Improved error message for bad relationship filter inputs.
2 changes: 2 additions & 0 deletions packages/keystone/src/lib/core/queries/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions packages/keystone/src/lib/core/where-inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)];
})
)
);
Expand All @@ -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 };
Expand Down
124 changes: 122 additions & 2 deletions tests/api-tests/queries/filters.test.ts
Original file line number Diff line number Diff line change
@@ -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 }),
},
}),
}),
Expand Down Expand Up @@ -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: '[email protected]' } });
const { data, errors } = await context.graphql.raw({
query: '{ user(where: { email: "[email protected]" }) { id email } }',
});
expect(errors).toBe(undefined);
expect(data).toEqual({ user: { id: item.id, email: '[email protected]' } });
})
);

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: '[email protected]' } });
const { body } = await graphQLRequest({
query: `{ user(where: { id: "${item.id}" email: "[email protected]" }) { 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'],
},
]);
})
);
});
32 changes: 31 additions & 1 deletion tests/api-tests/queries/orderBy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand All @@ -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' },
]);
})
);
});

0 comments on commit 398c085

Please sign in to comment.