Skip to content
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

fix: lookup crash when many-many self link #1097

Merged
merged 1 commit into from
Nov 19, 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
24 changes: 14 additions & 10 deletions apps/nestjs-backend/src/features/calculation/reference.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { PrismaService } from '@teable/db-main-prisma';
import type { IUserInfoVo } from '@teable/openapi';
import { instanceToPlain } from 'class-transformer';
import { Knex } from 'knex';
import { difference, groupBy, isEmpty, keyBy, uniq } from 'lodash';
import { difference, groupBy, isEmpty, isEqual, keyBy, uniq } from 'lodash';
import { InjectModel } from 'nest-knexjs';
import { InjectDbProvider } from '../../db-provider/db.provider';
import { IDbProvider } from '../../db-provider/db.provider.interface';
Expand Down Expand Up @@ -197,15 +197,15 @@ export class ReferenceService {

const tableId = fieldId2TableId[field.id];

const changes = Object.values(recordMap).reduce<ICellChange[]>((pre, record) => {
const changes = recordIds.reduce<ICellChange[]>((pre, recordId) => {
let dependencies: IRecord[] | undefined;
const dependentRecordIds = dependentRecordIdsIndexed[record.id];
const recordItems = dependentRecordIdsIndexed[recordId];
const dependentRecordIds = recordItems.map((item) => item.fromId).filter(Boolean) as string[];
const record = recordMap[recordId];

if (dependentRecordIds) {
try {
dependencies = dependentRecordIds
.map((item) => item.fromId && foreignRecordMap[item.fromId])
.filter(Boolean) as IRecord[];
dependencies = dependentRecordIds.map((id) => foreignRecordMap[id]);
} catch (e) {
console.log('changes:field', field);
console.log('relatedRecordItems', relatedRecordItems);
Expand Down Expand Up @@ -515,7 +515,7 @@ export class ReferenceService {
: originLookupValues;

// console.log('calculateLookup:dependencies', recordItem.dependencies);
// console.log('calculateLookup:lookupValues', lookupValues, recordItem);
// console.log('calculateLookup:lookupValues', field.id, lookupValues, recordItem);

if (field.isLookup) {
return this.filterArrayNull(lookupValues);
Expand Down Expand Up @@ -806,7 +806,7 @@ export class ReferenceService {
const value = this.calculateComputeField(field, fieldMap, recordItem, userMap);

const oldValue = record.fields[field.id];
if (oldValue === value) {
if (isEqual(oldValue, value)) {
return;
}

Expand Down Expand Up @@ -907,7 +907,9 @@ export class ReferenceService {
[dbTableName]: new Set(recordIds),
};
if (foreignDbTableName && foreignRecordIds) {
recordIdsByTableName[foreignDbTableName] = new Set(foreignRecordIds);
recordIdsByTableName[foreignDbTableName] = recordIdsByTableName[foreignDbTableName]
? new Set([...recordIdsByTableName[foreignDbTableName], ...foreignRecordIds])
: new Set(foreignRecordIds);
}

return await this.getRecordMap(recordIdsByTableName, dbTableName2fields);
Expand Down Expand Up @@ -1145,9 +1147,11 @@ export class ReferenceService {

const affectedRecordItemsQuerySql = query.toQuery();

return await this.prismaService
const result = await this.prismaService
.txClient()
.$queryRawUnsafe<IRelatedRecordItem[]>(affectedRecordItemsQuerySql);

return result.filter((item) => item.fromId || item.toId);
}

flatGraph(graph: { toFieldId: string; fromFieldId: string }[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,17 @@ export class TableOpenApiService {
// handle the link field in this table
const linkFields = await this.prismaService.txClient().field.findMany({
where: { tableId, type: FieldType.Link, isLookup: null, deletedTime: null },
select: { id: true },
select: { id: true, options: true },
});

for (const field of linkFields) {
if (field.options) {
const options = JSON.parse(field.options as string) as ILinkFieldOptions;
// if the link field is a self-link field, skip it
if (options.foreignTableId === tableId) {
continue;
}
}
await this.fieldOpenApiService.convertField(tableId, field.id, {
type: FieldType.SingleLineText,
});
Expand All @@ -228,6 +235,9 @@ export class TableOpenApiService {
const relatedLinkFieldRaws = await this.linkService.getRelatedLinkFieldRaws(tableId);

for (const field of relatedLinkFieldRaws) {
if (field.tableId === tableId) {
continue;
}
await this.fieldOpenApiService.convertField(field.tableId, field.id, {
type: FieldType.SingleLineText,
});
Expand Down
51 changes: 51 additions & 0 deletions apps/nestjs-backend/test/lookup.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -955,5 +955,56 @@ describe('OpenAPI Lookup field (e2e)', () => {
const table1Records5 = (await getRecords(table1.id, { fieldKeyType: FieldKeyType.Id })).data;
expect(table1Records5.records[0].fields[lookupField.id]).toBeUndefined();
});

it('should update a many-many self-link lookup field', async () => {
const linkField = await createField(table1.id, {
type: FieldType.Link,
options: {
relationship: Relationship.ManyMany,
foreignTableId: table1.id,
},
});
const symLinkFieldId = (linkField.options as ILinkFieldOptions).symmetricFieldId as string;

const lookupField = await createField(table1.id, {
type: FieldType.SingleLineText,
isLookup: true,
lookupOptions: {
foreignTableId: table1.id,
linkFieldId: linkField.id,
lookupFieldId: table1.fields[0].id,
},
});

await updateRecords(table1.id, {
fieldKeyType: FieldKeyType.Id,
typecast: true,
records: [
{
id: table1.records[0].id,
fields: {
[table1.fields[0].id]: 'B1',
[symLinkFieldId]: [table1.records[0].id],
},
},
],
});
await updateRecords(table1.id, {
fieldKeyType: FieldKeyType.Id,
typecast: true,
records: [
{
id: table1.records[1].id,
fields: {
[table1.fields[0].id]: 'B2',
[symLinkFieldId]: [table1.records[0].id],
},
},
],
});

const table1Records = (await getRecords(table1.id, { fieldKeyType: FieldKeyType.Id })).data;
expect(table1Records.records[0].fields[lookupField.id]).toEqual(['B1', 'B2']);
});
});
});
Loading