From 6b3babb3dea3e4ac9ef998378ca0904010926132 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Sun, 24 May 2026 12:52:05 +0100 Subject: [PATCH] fix(bases): strip all property-id refs from view config on delete --- .../strip-property-from-view-config.spec.ts | 49 +++++++++++++++++++ .../base/services/base-property.service.ts | 18 ++++--- .../strip-property-from-view-config.ts | 24 +++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/apps/server/src/core/base/services/__tests__/strip-property-from-view-config.spec.ts b/apps/server/src/core/base/services/__tests__/strip-property-from-view-config.spec.ts index 87bf85b22..d9ab7a78f 100644 --- a/apps/server/src/core/base/services/__tests__/strip-property-from-view-config.spec.ts +++ b/apps/server/src/core/base/services/__tests__/strip-property-from-view-config.spec.ts @@ -74,4 +74,53 @@ describe('stripPropertyFromViewConfig', () => { sorts: [{ propertyId: 'p-keep', direction: 'asc' as const }], }); }); + + it('strips visiblePropertyIds entries pointing at the deleted property', () => { + const config = { + visiblePropertyIds: ['p-deleted', 'p-keep'], + }; + expect(stripPropertyFromViewConfig(config, 'p-deleted')).toEqual({ + visiblePropertyIds: ['p-keep'], + }); + }); + + it('strips hiddenPropertyIds entries pointing at the deleted property', () => { + const config = { + hiddenPropertyIds: ['p-deleted', 'p-keep'], + }; + expect(stripPropertyFromViewConfig(config, 'p-deleted')).toEqual({ + hiddenPropertyIds: ['p-keep'], + }); + }); + + it('strips propertyOrder entries pointing at the deleted property', () => { + const config = { + propertyOrder: ['p-other', 'p-deleted', 'p-keep'], + }; + expect(stripPropertyFromViewConfig(config, 'p-deleted')).toEqual({ + propertyOrder: ['p-other', 'p-keep'], + }); + }); + + it('removes propertyWidths entry for the deleted property', () => { + const config = { + propertyWidths: { 'p-deleted': 120, 'p-keep': 200 }, + }; + expect(stripPropertyFromViewConfig(config, 'p-deleted')).toEqual({ + propertyWidths: { 'p-keep': 200 }, + }); + }); + + it('removes the visiblePropertyIds/propertyWidths keys when they become empty', () => { + const config = { + visiblePropertyIds: ['p-deleted'], + hiddenPropertyIds: ['p-deleted'], + propertyOrder: ['p-deleted'], + propertyWidths: { 'p-deleted': 120 }, + sorts: [{ propertyId: 'p-keep', direction: 'asc' as const }], + }; + expect(stripPropertyFromViewConfig(config, 'p-deleted')).toEqual({ + sorts: [{ propertyId: 'p-keep', direction: 'asc' as const }], + }); + }); }); diff --git a/apps/server/src/core/base/services/base-property.service.ts b/apps/server/src/core/base/services/base-property.service.ts index 54aec0bb2..7997faf91 100644 --- a/apps/server/src/core/base/services/base-property.service.ts +++ b/apps/server/src/core/base/services/base-property.service.ts @@ -30,6 +30,7 @@ import { parseTypeOptions, validateTypeOptions, isSystemPropertyType, + ViewConfig, } from '../base.schemas'; import { generateJitteredKeyBetween } from 'fractional-indexing-jittered'; import { QueueJob, QueueName } from '../../../integrations/queue/constants'; @@ -534,21 +535,17 @@ export class BasePropertyService { trx, }); for (const view of views) { - const before = (view.config ?? {}) as Record; const next = stripPropertyFromViewConfig( - view.config as any, + view.config as ViewConfig, dto.propertyId, ); - const after = next as Record; - if ( - Object.keys(before).length === Object.keys(after).length && - Object.keys(before).every((k) => k in after) && - JSON.stringify(before) === JSON.stringify(after) - ) { + if (JSON.stringify(view.config ?? {}) === JSON.stringify(next)) { continue; } await this.baseViewRepo.updateView( view.id, + // `config` column is typed `Json` by Kysely; ViewConfig is a Zod + // inferred shape that isn't structurally assignable to `Json`. { config: next as any }, { workspaceId, trx }, ); @@ -582,6 +579,11 @@ export class BasePropertyService { `Enqueue of cell-gc failed for property ${dto.propertyId}; reverting soft-delete`, err as Error, ); + // Best-effort revert: restores `deletedAt: null` on the property. The + // view-config cleanup and schema-bump that ran inside the earlier + // `executeTx` are NOT reverted — restoring those would require capturing + // the original configs before the transaction. Rare path (queue down); + // acceptable today. try { await this.basePropertyRepo.updateProperty(dto.propertyId, { deletedAt: null, diff --git a/apps/server/src/core/base/services/strip-property-from-view-config.ts b/apps/server/src/core/base/services/strip-property-from-view-config.ts index 0fe304f60..3b397d069 100644 --- a/apps/server/src/core/base/services/strip-property-from-view-config.ts +++ b/apps/server/src/core/base/services/strip-property-from-view-config.ts @@ -53,5 +53,29 @@ export function stripPropertyFromViewConfig( delete next.choiceOrder; } + if (config.visiblePropertyIds) { + const kept = config.visiblePropertyIds.filter((id) => id !== propertyId); + if (kept.length > 0) next.visiblePropertyIds = kept; + else delete next.visiblePropertyIds; + } + + if (config.hiddenPropertyIds) { + const kept = config.hiddenPropertyIds.filter((id) => id !== propertyId); + if (kept.length > 0) next.hiddenPropertyIds = kept; + else delete next.hiddenPropertyIds; + } + + if (config.propertyOrder) { + const kept = config.propertyOrder.filter((id) => id !== propertyId); + if (kept.length > 0) next.propertyOrder = kept; + else delete next.propertyOrder; + } + + if (config.propertyWidths && propertyId in config.propertyWidths) { + const { [propertyId]: _removed, ...rest } = config.propertyWidths; + if (Object.keys(rest).length > 0) next.propertyWidths = rest; + else delete next.propertyWidths; + } + return next as ViewConfig; }