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.

Sign up for GitHub

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: create table with link cellValue issues #210

Merged
merged 4 commits into from
Oct 23, 2023
Merged

fix: create table with link cellValue issues #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2541240
fix: refuse duplicate one-many link record id in one request
tea-artist Oct 23, 2023
7d39a6a
fix: update table error
tea-artist Oct 23, 2023
a6216ff
fix: create record with link cellValue missing title
tea-artist Oct 23, 2023
50ef4d0
fix: ignore duplicated link record id when convert to oneMany link
tea-artist Oct 23, 2023
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
12 changes: 6 additions & 6 deletions apps/nestjs-backend/src/features/calculation/batch.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,9 @@ export class BatchService {
tableId2DbTableName: { [tableId: string]: string }
) {
const tableIds = Object.keys(opsMap);
const missingTableIds = tableIds.filter((id) => !tableId2DbTableName[id]);
if (!missingTableIds.length) {
return { tableId2DbTableName, fieldMap };
}

const missingFieldIds = Array.from(
missingTableIds.reduce<Set<string>>((pre, id) => {
tableIds.reduce<Set<string>>((pre, id) => {
Object.values(opsMap[id]).forEach((ops) =>
ops.forEach((op) => {
const fieldId = RecordOpBuilder.editor.setRecord.detect(op)?.fieldId;
Expand All @@ -59,8 +55,12 @@ export class BatchService {
}, new Set())
);

if (!missingFieldIds.length) {
return { fieldMap, tableId2DbTableName };
}

const tableRaw = await this.prismaService.txClient().tableMeta.findMany({
where: { id: { in: missingTableIds }, deletedTime: null },
where: { id: { in: tableIds }, deletedTime: null },
select: { id: true, dbTableName: true },
});

Expand Down
79 changes: 4 additions & 75 deletions apps/nestjs-backend/src/features/calculation/link.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ describe('LinkService', () => {
]);
});

it('should create correct ForeignKeyParams even when illegal value for oneMany field', () => {
it('should throw error when when illegal value for oneMany field', () => {
const ctx1: ILinkCellContext[] = [
{
recordId: 'B1',
Expand All @@ -319,80 +319,9 @@ describe('LinkService', () => {
},
];

const result1 = service['getRecordMapStructAndForeignKeyParams'](
'tableB',
fieldMapByTableId,
ctx1
);

expect(result1.recordMapByTableId).toEqual({
tableA: {
A1: { fieldA: undefined, 'ManyOne-LinkB': undefined, '__fk_ManyOne-LinkB': undefined },
A2: { fieldA: undefined, 'ManyOne-LinkB': undefined, '__fk_ManyOne-LinkB': undefined },
},
tableB: {
B1: { fieldB: undefined, 'OneMany-LinkA': undefined },
B2: { fieldB: undefined, 'OneMany-LinkA': undefined },
},
});

expect(result1.updateForeignKeyParams).toEqual([
{
tableId: 'tableA',
foreignTableId: 'tableB',
mainLinkFieldId: 'ManyOne-LinkB',
mainTableLookupFieldId: 'fieldA',
foreignLinkFieldId: 'OneMany-LinkA',
foreignTableLookupFieldId: 'fieldB',
dbForeignKeyName: '__fk_ManyOne-LinkB',
recordId: 'A1',
fRecordId: null,
},
{
tableId: 'tableA',
foreignTableId: 'tableB',
mainLinkFieldId: 'ManyOne-LinkB',
mainTableLookupFieldId: 'fieldA',
foreignLinkFieldId: 'OneMany-LinkA',
foreignTableLookupFieldId: 'fieldB',
dbForeignKeyName: '__fk_ManyOne-LinkB',
recordId: 'A1',
fRecordId: 'B1',
},
{
tableId: 'tableA',
foreignTableId: 'tableB',
mainLinkFieldId: 'ManyOne-LinkB',
mainTableLookupFieldId: 'fieldA',
foreignLinkFieldId: 'OneMany-LinkA',
foreignTableLookupFieldId: 'fieldB',
dbForeignKeyName: '__fk_ManyOne-LinkB',
recordId: 'A2',
fRecordId: 'B1',
},
{
tableId: 'tableA',
foreignTableId: 'tableB',
mainLinkFieldId: 'ManyOne-LinkB',
mainTableLookupFieldId: 'fieldA',
foreignLinkFieldId: 'OneMany-LinkA',
foreignTableLookupFieldId: 'fieldB',
dbForeignKeyName: '__fk_ManyOne-LinkB',
recordId: 'A1',
fRecordId: 'B2',
},
{
tableId: 'tableA',
foreignTableId: 'tableB',
mainLinkFieldId: 'ManyOne-LinkB',
mainTableLookupFieldId: 'fieldA',
foreignLinkFieldId: 'OneMany-LinkA',
foreignTableLookupFieldId: 'fieldB',
dbForeignKeyName: '__fk_ManyOne-LinkB',
recordId: 'A2',
fRecordId: 'B2',
},
]);
expect(() =>
service['getRecordMapStructAndForeignKeyParams']('tableB', fieldMapByTableId, ctx1)
).toThrow();
});

it('should update foreign key in memory correctly when add value', () => {
Expand Down
45 changes: 35 additions & 10 deletions apps/nestjs-backend/src/features/calculation/link.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ILinkFieldOptions } from '@teable-group/core';
import { FieldType, Relationship } from '@teable-group/core';
import { PrismaService } from '@teable-group/db-main-prisma';
import { Knex } from 'knex';
import { cloneDeep, isEqual, set } from 'lodash';
import { cloneDeep, isEqual, keyBy, set } from 'lodash';
import { InjectModel } from 'nest-knexjs';
import type { IFkOpMap } from './reference.service';
import type { ICellChange } from './utils/changes';
Expand Down Expand Up @@ -150,7 +150,9 @@ export class LinkService {
if (oldFRecordId) {
const fRecord = foreignTable[oldFRecordId];
if (!fRecord) {
throw new BadRequestException('Can not set duplicate link record in this field');
throw new BadRequestException(
'Consistency error, Can not set duplicate link record in this field'
);
}
const oldFRecordFLink = fRecord[foreignLinkFieldId] as
| { id: string; title?: string }[]
Expand Down Expand Up @@ -203,7 +205,20 @@ export class LinkService {
cellContexts: ILinkCellContext[]
) {
const recordMapByTableId: IRecordMapByTableId = {};
// include main table update message, and foreign table update will reflect to main table
const updateForeignKeyParams: IUpdateForeignKeyParam[] = [];
const checkSet = new Set<string>();
function pushForeignKeyParam(param: IUpdateForeignKeyParam) {
if (param.fRecordId) {
if (checkSet.has(param.recordId)) {
throw new BadRequestException(
`Consistency error, link field {${param.foreignLinkFieldId}} unable to link a record (${param.recordId}) more than once`
);
}
checkSet.add(param.recordId);
}
return updateForeignKeyParams.push(param);
}
for (const cellContext of cellContexts) {
const { recordId, fieldId, newValue, oldValue } = cellContext;
const linkRecordIds = [oldValue, newValue]
Expand Down Expand Up @@ -232,7 +247,7 @@ export class LinkService {
}
// add dbForeignKeyName to the record
set(recordMapByTableId, [tableId, recordId, dbForeignKeyName], undefined);
updateForeignKeyParams.push({
pushForeignKeyParam({
tableId,
foreignTableId,
mainLinkFieldId: fieldId,
Expand All @@ -241,20 +256,20 @@ export class LinkService {
foreignTableLookupFieldId: foreignLookupFieldId,
dbForeignKeyName: field.options.dbForeignKeyName,
recordId,
fRecordId: newValue?.id || null,
fRecordId: newValue?.id || null, // to set main table link cellValue to null;
});
}
if (field.options.relationship === Relationship.OneMany) {
if (newValue && !Array.isArray(newValue)) {
throw new BadRequestException(
`OneMany relationship newValue should have multiple records, received: ${JSON.stringify(
`Consistency error, OneMany relationship newValue should have multiple records, received: ${JSON.stringify(
newValue
)}`
);
}
if (oldValue && !Array.isArray(oldValue)) {
throw new BadRequestException(
`OneMany relationship oldValue should have multiple records, received: ${JSON.stringify(
`Consistency error, OneMany relationship oldValue should have multiple records, received: ${JSON.stringify(
oldValue
)}`
);
Expand All @@ -273,7 +288,7 @@ export class LinkService {
oldValue.forEach((oldValueItem) => {
// add dbForeignKeyName to the record
set(recordMapByTableId, [foreignTableId, oldValueItem.id, dbForeignKeyName], undefined);
updateForeignKeyParams.push({
pushForeignKeyParam({
...paramCommon,
recordId: oldValueItem.id,
fRecordId: null,
Expand All @@ -283,7 +298,7 @@ export class LinkService {
newValue.forEach((newValueItem) => {
// add dbForeignKeyName to the record
set(recordMapByTableId, [foreignTableId, newValueItem.id, dbForeignKeyName], undefined);
updateForeignKeyParams.push({
pushForeignKeyParam({
...paramCommon,
recordId: newValueItem.id,
fRecordId: recordId,
Expand All @@ -302,8 +317,10 @@ export class LinkService {
tableId2DbTableName: { [tableId: string]: string },
fieldMapByTableId: ITinyFieldMapByTableId,
recordMapByTableId: IRecordMapByTableId,
cellContexts: ICellContext[],
fromReset?: boolean
): Promise<IRecordMapByTableId> {
const cellContextGroup = keyBy(cellContexts, (ctx) => `${ctx.recordId}-${ctx.fieldId}`);
for (const tableId in recordMapByTableId) {
const recordLookupFieldsMap = recordMapByTableId[tableId];
const recordIds = Object.keys(recordLookupFieldsMap);
Expand Down Expand Up @@ -357,6 +374,13 @@ export class LinkService {
continue;
}

// Overlay with new data, especially cellValue in primary field
const inputData = cellContextGroup[`${recordId}-${fieldId}`];
if (field.type !== FieldType.Link && inputData !== undefined) {
recordLookupFieldsMap[recordId][fieldId] = inputData.newValue ?? undefined;
continue;
}

if (field.type === FieldType.Link && cellValue != null) {
cellValue = JSON.parse(cellValue as string);
}
Expand Down Expand Up @@ -508,17 +532,17 @@ export class LinkService {
tableId2DbTableName: { [tableId: string]: string },
fieldMapByTableId: ITinyFieldMapByTableId,
linkContexts: ILinkCellContext[],
cellContexts: ICellContext[],
fromReset?: boolean
): Promise<{ cellChanges: ICellChange[]; fkRecordMap: IFkOpMap }> {
const { recordMapByTableId, updateForeignKeyParams } =
this.getRecordMapStructAndForeignKeyParams(tableId, fieldMapByTableId, linkContexts);

// console.log('recordMapByTableId:', recordMapByTableId);
// console.log('updateForeignKeyParams:', updateForeignKeyParams);
const originRecordMapByTableId = await this.fillRecordMap(
tableId2DbTableName,
fieldMapByTableId,
recordMapByTableId,
cellContexts,
fromReset
);
// console.log('originRecordMapByTableId:', JSON.stringify(originRecordMapByTableId, null, 2));
Expand Down Expand Up @@ -583,6 +607,7 @@ export class LinkService {
tableId2DbTableName,
fieldMapByTableId,
linkContexts,
cellContexts,
fromReset
);
}
Expand Down
16 changes: 13 additions & 3 deletions apps/nestjs-backend/src/features/field/field-calculate/field-converting-link.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export class FieldConvertingLinkService {
return records;
}

// TODO: how to handle consistency of one-many ? Same value should not be select in a multiple cell value field.
/**
* convert oldCellValue to new link field cellValue
* if oldCellValue is not in foreignTable, create new record in foreignTable
Expand All @@ -183,6 +182,7 @@ export class FieldConvertingLinkService {
const lookupField = createFieldInstanceByRaw(lookupFieldRaw);

const records = await this.getRecords(tableId, oldField);
// TODO: should not get all records in foreignTable, only get records witch title is not exist in candidate records link cell value title
const foreignRecords = await this.getRecords(foreignTableId, lookupField);

const primaryNameToIdMap = foreignRecords.reduce<{ [name: string]: string }>((pre, record) => {
Expand All @@ -193,6 +193,7 @@ export class FieldConvertingLinkService {

const recordOpsMap: IOpsMap = { [tableId]: {}, [foreignTableId]: {} };
const recordsForCreate: { [title: string]: ITinyRecord } = {};
const checkSet = new Set<string>();
// eslint-disable-next-line sonarjs/cognitive-complexity
records.forEach((record) => {
const oldCellValue = record.fields[fieldId];
Expand All @@ -211,10 +212,19 @@ export class FieldConvertingLinkService {
}

const newCellValue: ILinkCellValue[] = [];
function pushNewCellValue(linkCell: ILinkCellValue) {
if (newField.options.relationship !== Relationship.OneMany) {
return newCellValue.push(linkCell);
}
// OneMany relationship only allow link to one same recordId
if (checkSet.has(linkCell.id)) return;
checkSet.add(linkCell.id);
return newCellValue.push(linkCell);
}

newCellValueTitle.forEach((title) => {
if (primaryNameToIdMap[title]) {
newCellValue.push({ id: primaryNameToIdMap[title], title });
pushNewCellValue({ id: primaryNameToIdMap[title], title });
return;
}

Expand Down Expand Up @@ -243,7 +253,7 @@ export class FieldConvertingLinkService {
},
};
}
newCellValue.push({ id: recordsForCreate[title].id, title });
pushNewCellValue({ id: recordsForCreate[title].id, title });
});

if (!recordOpsMap[tableId][record.id]) {
Expand Down
26 changes: 13 additions & 13 deletions apps/nestjs-backend/src/features/record/translator/filter-query-translator.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,19 +1004,19 @@ describe('FilterQueryTranslator', () => {
relationship: Relationship.ManyOne,
},
},
{
id: 'fld8',
name: 'link',
type: FieldType.Link,
dbFieldName: 'link_fld8',
dbFieldType: DbFieldType.Json,
cellValueType: CellValueType.String,
isMultipleCellValue: true,
columnMeta: {},
options: {
relationship: Relationship.ManyMany,
},
},
// {
// id: 'fld8',
// name: 'link',
// type: FieldType.Link,
// dbFieldName: 'link_fld8',
// dbFieldType: DbFieldType.Json,
// cellValueType: CellValueType.String,
// isMultipleCellValue: true,
// columnMeta: {},
// options: {
// relationship: Relationship.ManyMany,
// },
// },
{
id: 'fld9',
name: 'link',
Expand Down
Loading