Skip to content

Commit

Permalink
fix: lookup crash when many-many self link (#1097)
Browse files Browse the repository at this point in the history
  • Loading branch information
tea-artist authored Nov 19, 2024
1 parent 25901f9 commit 61c948b
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
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']);
});
});
});

0 comments on commit 61c948b

Please sign in to comment.