Skip to content

Commit

Permalink
Fix shared name bugs
Browse files Browse the repository at this point in the history
Rate limit · GitHub

Whoa there!

You have triggered an abuse detection mechanism.

Please wait a few minutes before you try again;
in some cases this may take up to an hour.

timleslie committed Feb 19, 2021
1 parent 33fe5ce commit 26a9ce9
Showing 5 changed files with 181 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-fishes-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/adapter-knex': patch
---

Fixed a query generation issue when two relationship fields shared the same name.
5 changes: 5 additions & 0 deletions .changeset/thirty-pandas-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/adapter-prisma': patch
---

Fixed a schema generation issue when two one-sided many-to-many relationships shared the same name.
2 changes: 1 addition & 1 deletion packages/adapter-knex/lib/adapter-knex.js
Original file line number Diff line number Diff line change
@@ -822,7 +822,7 @@ class QueryBuilder {
.from(`${tableName} as ${subBaseTableAlias}`);
// We need to filter out nulls before passing back to the top level query
// otherwise postgres will give very incorrect answers.
subQuery.whereNotNull(columnName);
subQuery.whereNotNull(`${subBaseTableAlias}.${columnName}`);
} else {
const { near, far } = listAdapter._getNearFar(fieldAdapter);
otherTableAlias = `${subBaseTableAlias}__${p}`;
4 changes: 2 additions & 2 deletions packages/adapter-prisma/lib/adapter-prisma.js
Original file line number Diff line number Diff line change
@@ -170,7 +170,7 @@ class PrismaAdapter extends BaseKeystoneAdapter {
.filter(({ left }) => left.refListKey === listAdapter.key)
.filter(({ cardinality }) => cardinality === 'N:N')
.map(({ left: { path, listKey }, tableName }) => [
`from_${path} ${listKey}[] @relation("${tableName}", references: [id])`,
`from_${listKey}_${path} ${listKey}[] @relation("${tableName}", references: [id])`,
])
),
...flatten(
@@ -403,7 +403,7 @@ class PrismaListAdapter extends BaseListAdapter {
? a.field === a.rel.right // Two-sided
? a.rel.left.path
: a.rel.right.path
: `from_${a.rel.left.path}`; // One-sided
: `from_${a.rel.left.listKey}_${a.rel.left.path}`; // One-sided
ret.where[path] = { some: { id: Number(from.fromId) } };
} else {
ret.where[a.rel.columnName] = { id: Number(from.fromId) };
168 changes: 168 additions & 0 deletions tests/api-tests/relationships/shared-names.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
const { Text, Relationship } = require('@keystonejs/fields');
const { multiAdapterRunners, setupServer } = require('@keystonejs/test-utils');
const { createItems, updateItems } = require('@keystonejs/server-side-graphql-client');

const createInitialData = async keystone => {
const roles = await createItems({
keystone,
listKey: 'Role',
items: [{ data: { name: 'RoleA' } }, { data: { name: 'RoleB' } }, { data: { name: 'RoleC' } }],
returnFields: 'id name',
});
const companies = await createItems({
keystone,
listKey: 'Company',
items: [
{ data: { name: 'CompanyA' } },
{ data: { name: 'CompanyB' } },
{ data: { name: 'CompanyC' } },
],
returnFields: 'id name',
});
const employees = await createItems({
keystone,
listKey: 'Employee',
items: [
{
data: {
name: 'EmployeeA',
company: { connect: { id: companies.find(({ name }) => name === 'CompanyA').id } },
role: { connect: { id: roles.find(({ name }) => name === 'RoleA').id } },
},
},
{
data: {
name: 'EmployeeB',
company: { connect: { id: companies.find(({ name }) => name === 'CompanyB').id } },
role: { connect: { id: roles.find(({ name }) => name === 'RoleB').id } },
},
},
{
data: {
name: 'EmployeeC',
company: { connect: { id: companies.find(({ name }) => name === 'CompanyC').id } },
role: { connect: { id: roles.find(({ name }) => name === 'RoleC').id } },
},
},
],
returnFields: 'id name',
});
await createItems({
keystone,
listKey: 'Location',
items: [
{
data: {
name: 'LocationA',
employees: {
connect: employees
.filter(e => ['EmployeeA', 'EmployeeB'].includes(e.name))
.map(e => ({ id: e.id })),
},
},
},
{
data: {
name: 'LocationB',
employees: {
connect: employees
.filter(e => ['EmployeeB', 'EmployeeC'].includes(e.name))
.map(e => ({ id: e.id })),
},
},
},
{
data: {
name: 'LocationC',
employees: {
connect: employees
.filter(e => ['EmployeeC', 'EmployeeA'].includes(e.name))
.map(e => ({ id: e.id })),
},
},
},
],
returnFields: 'id name',
});
await updateItems({
keystone,
listKey: 'Role',
items: [
{
id: roles.find(({ name }) => name === 'RoleA').id,
data: {
company: { connect: { id: companies.find(({ name }) => name === 'CompanyA').id } },
employees: { connect: [{ id: employees.find(({ name }) => name === 'EmployeeA').id }] },
},
},
{
id: roles.find(({ name }) => name === 'RoleB').id,
data: {
company: { connect: { id: companies.find(({ name }) => name === 'CompanyB').id } },
employees: { connect: [{ id: employees.find(({ name }) => name === 'EmployeeB').id }] },
},
},
{
id: roles.find(({ name }) => name === 'RoleC').id,
data: {
company: { connect: { id: companies.find(({ name }) => name === 'CompanyC').id } },
employees: { connect: [{ id: employees.find(({ name }) => name === 'EmployeeC').id }] },
},
},
],
});
};

const setupKeystone = adapterName =>
setupServer({
adapterName,
createLists: keystone => {
keystone.createList('Employee', {
fields: {
name: { type: Text },
company: { type: Relationship, ref: 'Company.employees', many: false },
role: { type: Relationship, ref: 'Role', many: false },
},
});
keystone.createList('Company', {
fields: {
name: { type: Text },
employees: { type: Relationship, ref: 'Employee.company', many: true },
},
});
keystone.createList('Role', {
fields: {
name: { type: Text },
company: { type: Relationship, ref: 'Company', many: false },
employees: { type: Relationship, ref: 'Employee', many: true },
},
});
keystone.createList('Location', {
fields: {
name: { type: Text },
employees: { type: Relationship, ref: 'Employee', many: true },
},
});
},
});

multiAdapterRunners().map(({ runner, adapterName }) =>
describe(`Adapter: ${adapterName}`, () => {
test(
'Query',
runner(setupKeystone, async ({ keystone }) => {
await createInitialData(keystone);
const { data, errors } = await keystone.executeGraphQL({
query: `{
allEmployees(where: {
company: { employees_some: { role: { name: "RoleA" } } }
}) { id name }
}`,
});
expect(errors).toBe(undefined);
expect(data.allEmployees).toHaveLength(1);
expect(data.allEmployees[0].name).toEqual('EmployeeA');
})
);
})
);

0 comments on commit 26a9ce9

Please sign in to comment.