Skip to content

Commit

Permalink
Fix issues with relationship names shared across lists (#4890)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Feb 19, 2021
1 parent 891cd49 commit 00f19da
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/sixty-terms-exercise.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.
5 changes: 5 additions & 0 deletions .changeset/thick-roses-argue.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.
2 changes: 1 addition & 1 deletion packages/adapter-knex/lib/adapter-knex.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down
4 changes: 2 additions & 2 deletions packages/adapter-prisma/lib/adapter-prisma.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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) };
Expand Down
171 changes: 171 additions & 0 deletions tests/api-tests/relationships/shared-names.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
const { text, relationship } = require('@keystone-next/fields');
const { createSchema, list } = require('@keystone-next/keystone/schema');
const { multiAdapterRunners, setupFromConfig } = require('@keystonejs/test-utils');
const { createItems, updateItems } = require('@keystonejs/server-side-graphql-client');

const createInitialData = async context => {
const roles = await createItems({
context,
listKey: 'Role',
items: [{ data: { name: 'RoleA' } }, { data: { name: 'RoleB' } }, { data: { name: 'RoleC' } }],
returnFields: 'id name',
});
const companies = await createItems({
context,
listKey: 'Company',
items: [
{ data: { name: 'CompanyA' } },
{ data: { name: 'CompanyB' } },
{ data: { name: 'CompanyC' } },
],
returnFields: 'id name',
});
const employees = await createItems({
context,
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({
context,
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({
context,
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 =>
setupFromConfig({
adapterName,
config: createSchema({
lists: {
Employee: list({
fields: {
name: text(),
company: relationship({ ref: 'Company.employees', many: false }),
role: relationship({ ref: 'Role', many: false }),
},
}),
Company: list({
fields: {
name: text(),
employees: relationship({ ref: 'Employee.company', many: true }),
},
}),
Role: list({
fields: {
name: text(),
company: relationship({ ref: 'Company', many: false }),
employees: relationship({ ref: 'Employee', many: true }),
},
}),
Location: list({
fields: {
name: text(),
employees: relationship({ ref: 'Employee', many: true }),
},
}),
},
}),
});

multiAdapterRunners().map(({ runner, adapterName }) =>
describe(`Adapter: ${adapterName}`, () => {
test(
'Query',
runner(setupKeystone, async ({ context }) => {
await createInitialData(context);
const { data, errors } = await context.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');
})
);
})
);

1 comment on commit 00f19da

@vercel
Copy link

@vercel vercel bot commented on 00f19da Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.