Skip to content

fix: make sure fields inherited from abstract base models are properly recognized as id #1130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/sdk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,14 @@ export function isIdField(field: DataModelField) {
return true;
}

// NOTE: we have to use name to match fields because the fields
// may be inherited from an abstract base and have cloned identities

const model = field.$container as DataModel;

// model-level @@id attribute with a list of fields
const modelLevelIds = getModelIdFields(model);
if (modelLevelIds.includes(field)) {
if (modelLevelIds.map((f) => f.name).includes(field.name)) {
return true;
}

Expand All @@ -234,12 +237,12 @@ export function isIdField(field: DataModelField) {
// then, the first field with @unique can be used as id
const firstUniqueField = model.fields.find((f) => hasAttribute(f, '@unique'));
if (firstUniqueField) {
return firstUniqueField === field;
return firstUniqueField.name === field.name;
}

// last, the first model level @@unique can be used as id
const modelLevelUnique = getModelUniqueFields(model);
if (modelLevelUnique.includes(field)) {
if (modelLevelUnique.map((f) => f.name).includes(field.name)) {
return true;
}

Expand Down
87 changes: 87 additions & 0 deletions tests/integration/tests/regression/issue-1129.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { createPostgresDb, dropPostgresDb, loadSchema } from '@zenstackhq/testtools';

describe('Regression for issue 1129', () => {
it('regression', async () => {
let prisma;
const dbUrl = await createPostgresDb('regression-issue-1129');

try {
const r = await loadSchema(
`
model Relation1 {
id String @id @default(cuid())
field1 String
concrete Concrete[]
@@allow('all', true)
}

model Relation2 {
id String @id @default(cuid())
field2 String
concrete Concrete[]
@@allow('all', true)
}

abstract model WithRelation1 {
relation1Id String
relation1 Relation1 @relation(fields: [relation1Id], references: [id])
}
abstract model WithRelation2 {
relation2Id String
relation2 Relation2 @relation(fields: [relation2Id], references: [id])
}

model Concrete extends WithRelation1, WithRelation2 {
concreteField String
@@id([relation1Id, relation2Id])
@@allow('all', true)
}
`,
{ provider: 'postgresql', dbUrl }
);

prisma = r.prisma;
const db = r.enhance();

await db.$transaction(async (tx: any) => {
await tx.relation2.createMany({
data: [
{
id: 'relation2Id1',
field2: 'field2Value1',
},
{
id: 'relation2Id2',
field2: 'field2Value2',
},
],
});

await tx.relation1.create({
data: {
field1: 'field1Value',
concrete: {
createMany: {
data: [
{
concreteField: 'concreteFieldValue1',
relation2Id: 'relation2Id1',
},
{
concreteField: 'concreteFieldValue2',
relation2Id: 'relation2Id2',
},
],
},
},
},
});
});
} finally {
if (prisma) {
await prisma.$disconnect();
}
await dropPostgresDb('regression-issue-1129');
}
});
});