From 13a7e55a4aa6f6789b9a03006a49265c6af579d6 Mon Sep 17 00:00:00 2001 From: tea artist Date: Thu, 14 Nov 2024 23:11:05 +0800 Subject: [PATCH] fix: filter not clean when remove last visible item --- .../features/calculation/reference.service.ts | 78 ++++++++++++------- .../src/features/next/next.service.ts | 2 +- apps/nestjs-backend/src/ws/ws.gateway.dev.ts | 47 ++++++----- apps/nestjs-backend/test/lookup.e2e-spec.ts | 30 +++++++ apps/nestjs-backend/test/rollup.e2e-spec.ts | 40 ++++++++-- 5 files changed, 137 insertions(+), 60 deletions(-) diff --git a/apps/nestjs-backend/src/features/calculation/reference.service.ts b/apps/nestjs-backend/src/features/calculation/reference.service.ts index f74d685528..b7f174fb98 100644 --- a/apps/nestjs-backend/src/features/calculation/reference.service.ts +++ b/apps/nestjs-backend/src/features/calculation/reference.service.ts @@ -687,10 +687,6 @@ export class ReferenceService { }); if (field.type === FieldType.Rollup) { - // console.log('calculateRollup', field, lookupField, record, lookupValues); - if (lookupValues == null) { - return null; - } return field .evaluate( { values: virtualField }, @@ -1088,41 +1084,63 @@ export class ReferenceService { : toRecordIds; const { fkHostTableName, selfKeyName, foreignKeyName } = options; - const query = knex - .with('initial_to_ids', (qb) => { - if (fromRecordIds?.length) { - const fromQuery = knex - .select(selfKeyName) - .from(fkHostTableName) - .whereIn(foreignKeyName, fromRecordIds); - - qb.select(selfKeyName).from(fromQuery.as('t')); - } - if (unionToRecordIds?.length) { - const valueQueries = unionToRecordIds.map((id) => - knex.select(knex.raw('? as ??', [id, selfKeyName])) - ); - qb.union(valueQueries); - } - }) - .select({ - toId: knex.ref(`i.${selfKeyName}`), - fromId: knex.ref(`a.${foreignKeyName}`), - }) - .from('initial_to_ids as i') - .leftJoin(`${fkHostTableName} as a`, `a.${selfKeyName}`, `i.${selfKeyName}`); + // 1. Build the base query with initial_to_ids CTE + const query = knex.with('initial_to_ids', (qb) => { + if (fromRecordIds?.length) { + const fromQuery = knex + .select(selfKeyName) + .from(fkHostTableName) + .whereIn(foreignKeyName, fromRecordIds); + + qb.select(selfKeyName).from(fromQuery.as('t')); + } + if (unionToRecordIds?.length) { + const valueQueries = unionToRecordIds.map((id) => + knex.select(knex.raw('? as ??', [id, selfKeyName])) + ); + qb.union(valueQueries); + } + }); + + // 2. Add filter logic and build final query if (field.lookupOptions?.filter) { + // First get filtered records query .with('filtered_records', (qb) => { const dataTableName = tableId2DbTableName[options.foreignTableId]; qb.select('__id').from(dataTableName); dbProvider.filterQuery(qb, fieldMap, field.lookupOptions!.filter).appendQueryBuilder(); }) - .whereIn(`a.${foreignKeyName}`, function () { - this.select('__id').from('filtered_records'); - }); + // Get valid pairs (where fromId passes filter) + .with('valid_pairs', (qb) => { + qb.select([`i.${selfKeyName} as toId`, `a.${foreignKeyName} as fromId`]) + .from('initial_to_ids as i') + .leftJoin(`${fkHostTableName} as a`, `a.${selfKeyName}`, `i.${selfKeyName}`) + .whereIn(`a.${foreignKeyName}`, function () { + this.select('__id').from('filtered_records'); + }); + }) + // Union with toIds that have no valid pairs (with null fromId) + .select('*') + .from('valid_pairs') + .unionAll( + knex + .select([`initial_to_ids.${selfKeyName} as toId`, knex.raw('NULL as fromId')]) + .from('initial_to_ids') + .whereNotExists(function () { + this.select('*') + .from('valid_pairs') + .where('toId', knex.ref(`initial_to_ids.${selfKeyName}`)); + }) + ); + } else { + // No filter, just get all pairs + query + .select([`i.${selfKeyName} as toId`, `a.${foreignKeyName} as fromId`]) + .from('initial_to_ids as i') + .leftJoin(`${fkHostTableName} as a`, `a.${selfKeyName}`, `i.${selfKeyName}`); } const affectedRecordItemsQuerySql = query.toQuery(); diff --git a/apps/nestjs-backend/src/features/next/next.service.ts b/apps/nestjs-backend/src/features/next/next.service.ts index 1d1ca48a83..644ae6c93c 100644 --- a/apps/nestjs-backend/src/features/next/next.service.ts +++ b/apps/nestjs-backend/src/features/next/next.service.ts @@ -32,6 +32,6 @@ export class NextService implements OnModuleInit, OnModuleDestroy { } async onModuleDestroy() { - this.server.close(); + await this.server?.close(); } } diff --git a/apps/nestjs-backend/src/ws/ws.gateway.dev.ts b/apps/nestjs-backend/src/ws/ws.gateway.dev.ts index a533e7c5f7..0886b52338 100644 --- a/apps/nestjs-backend/src/ws/ws.gateway.dev.ts +++ b/apps/nestjs-backend/src/ws/ws.gateway.dev.ts @@ -53,34 +53,33 @@ export class DevWsGateway implements OnModuleInit, OnModuleDestroy { async onModuleDestroy() { try { this.logger.log('Starting graceful shutdown...'); - - await new Promise((resolve, reject) => { - this.shareDb.close((err) => { - if (err) { - this.logger.error('ShareDb close error', err?.stack); - reject(err); - } else { - this.logger.log('ShareDb closed successfully'); - resolve(null); - } - }); - }); - - this.server.clients.forEach((client) => { + this.server?.clients.forEach((client) => { client.terminate(); }); - await new Promise((resolve, reject) => { - this.server.close((err) => { - if (err) { - this.logger.error('DevWsGateway close error', err?.stack); - reject(err); - } else { - this.logger.log('WebSocket server closed successfully'); + await Promise.all([ + new Promise((resolve) => { + this.shareDb.close((err) => { + if (err) { + this.logger.error('ShareDb close error', err?.stack); + } else { + this.logger.log('ShareDb closed successfully'); + } resolve(null); - } - }); - }); + }); + }), + + new Promise((resolve) => { + this.server.close((err) => { + if (err) { + this.logger.error('DevWsGateway close error', err?.stack); + } else { + this.logger.log('WebSocket server closed successfully'); + } + resolve(null); + }); + }), + ]); this.logger.log('Graceful had shutdown completed'); process.exit(0); diff --git a/apps/nestjs-backend/test/lookup.e2e-spec.ts b/apps/nestjs-backend/test/lookup.e2e-spec.ts index 0604f82f30..76ee905a70 100644 --- a/apps/nestjs-backend/test/lookup.e2e-spec.ts +++ b/apps/nestjs-backend/test/lookup.e2e-spec.ts @@ -924,6 +924,36 @@ describe('OpenAPI Lookup field (e2e)', () => { const table1Records3 = (await getRecords(table1.id, { fieldKeyType: FieldKeyType.Id })).data; expect(table1Records3.records[0].fields[lookupField.id]).toEqual(['B2']); + + // set it to filtered null + await updateRecords(table1.id, { + fieldKeyType: FieldKeyType.Id, + typecast: true, + records: [ + { + id: table1.records[0].id, + fields: { [linkField.id]: [{ id: table2.records[0].id }] }, + }, + ], + }); + + const table1Records4 = (await getRecords(table1.id, { fieldKeyType: FieldKeyType.Id })).data; + expect(table1Records4.records[0].fields[lookupField.id]).toBeUndefined(); + + // set it to null + await updateRecords(table1.id, { + fieldKeyType: FieldKeyType.Id, + typecast: true, + records: [ + { + id: table1.records[0].id, + fields: { [linkField.id]: null }, + }, + ], + }); + + const table1Records5 = (await getRecords(table1.id, { fieldKeyType: FieldKeyType.Id })).data; + expect(table1Records5.records[0].fields[lookupField.id]).toBeUndefined(); }); }); }); diff --git a/apps/nestjs-backend/test/rollup.e2e-spec.ts b/apps/nestjs-backend/test/rollup.e2e-spec.ts index 921611535b..52fde5668d 100644 --- a/apps/nestjs-backend/test/rollup.e2e-spec.ts +++ b/apps/nestjs-backend/test/rollup.e2e-spec.ts @@ -267,7 +267,7 @@ describe('OpenAPI Rollup field (e2e)', () => { ); const recordAfter2 = await getRecord(table1.id, table1.records[1].id); - expect(recordAfter2.fields[rollupFieldVo.id]).toBeUndefined(); + expect(recordAfter2.fields[rollupFieldVo.id]).toEqual(0); // add a link record from many - one field await updateRecordField( @@ -312,7 +312,7 @@ describe('OpenAPI Rollup field (e2e)', () => { const record3 = await getRecord(table2.id, table2.records[1].id); expect(record3.fields[rollupFieldVo.id]).toEqual(123); const record4 = await getRecord(table2.id, table2.records[2].id); - expect(record4.fields[rollupFieldVo.id]).toBeUndefined(); + expect(record4.fields[rollupFieldVo.id]).toEqual(0); // remove all link record await updateRecordField( @@ -323,7 +323,7 @@ describe('OpenAPI Rollup field (e2e)', () => { ); const record5 = await getRecord(table2.id, table2.records[1].id); - expect(record5.fields[rollupFieldVo.id]).toBeUndefined(); + expect(record5.fields[rollupFieldVo.id]).toEqual(0); // add a link record from many - one field await updateRecordField( @@ -377,7 +377,7 @@ describe('OpenAPI Rollup field (e2e)', () => { ); const record1 = await getRecord(table1.id, table1.records[1].id); - expect(record1.fields[rollupFieldVo.id]).toBeUndefined(); + expect(record1.fields[rollupFieldVo.id]).toEqual(0); const record2 = await getRecord(table1.id, table1.records[2].id); expect(record2.fields[rollupFieldVo.id]).toEqual(1); }); @@ -480,7 +480,7 @@ describe('OpenAPI Rollup field (e2e)', () => { const rollupFieldVo = await rollupFrom(table2, lookedUpToField.id); const record0 = await getRecord(table2.id, table2.records[0].id); - expect(record0.fields[rollupFieldVo.id]).toEqual(undefined); + expect(record0.fields[rollupFieldVo.id]).toEqual(0); const record1 = await getRecord(table2.id, table2.records[1].id); expect(record1.fields[rollupFieldVo.id]).toEqual(1); const record2 = await getRecord(table2.id, table2.records[2].id); @@ -579,5 +579,35 @@ describe('OpenAPI Rollup field (e2e)', () => { const record2 = await getRecord(table2.id, table2.records[0].id); expect([record2.fields[rollup1.id], record2.fields[rollup2.id]]).toEqual([4, 4]); }); + + it('should calculate rollup event has no link record', async () => { + const numberField = await createField(table1.id, { + type: FieldType.Number, + }); + + const linkField = await createField(table2.id, { + type: FieldType.Link, + options: { + relationship: Relationship.OneMany, + foreignTableId: table1.id, + }, + }); + + const rollup1 = await createField(table2.id, { + name: `rollup 1`, + type: FieldType.Rollup, + options: { + expression: `sum({values})`, + }, + lookupOptions: { + foreignTableId: table1.id, + linkFieldId: linkField.id, + lookupFieldId: numberField.id, + } as ILookupOptionsRo, + }); + + const record1 = await getRecord(table2.id, table2.records[0].id); + expect(record1.fields[rollup1.id]).toEqual(0); + }); }); });