Skip to content

Commit 07654b6

Browse files
guguclaude
andcommitted
fix: preserve original FK value when identity lookup misses
GetTableRowsUseCase was overwriting null and orphaned foreign key values with `{}` because the merge step's else branch assigned an empty object whenever the identity lookup map missed. Null FKs (filtered out before the identity query) and orphaned FKs (referenced row deleted) both trigger this path, silently destroying the original cell value. Fall back to the original cell value instead, so null stays null and orphaned ids are preserved as primitives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eeb1622 commit 07654b6

2 files changed

Lines changed: 83 additions & 2 deletions

File tree

backend/src/entities/table/use-cases/get-table-rows.use.case.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,11 +320,12 @@ export class GetTableRowsUseCase extends AbstractUseCase<GetTableRowsDs, FoundTa
320320

321321
if (!identityColumnsMap) continue;
322322

323-
const identityForCurrentValue = identityColumnsMap.get(row[element.currentFKeyName]);
323+
const originalValue = row[element.currentFKeyName];
324+
const identityForCurrentValue = identityColumnsMap.get(originalValue);
324325
row[element.currentFKeyName] =
325326
typeof identityForCurrentValue === 'object' && identityForCurrentValue !== null
326327
? identityForCurrentValue
327-
: {};
328+
: originalValue;
328329
}
329330
}
330331

backend/test/ava-tests/complex-table-tests/complex-postgres-table-e2e.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { DatabaseModule } from '../../../src/shared/database/database.module.js'
1515
import { DatabaseService } from '../../../src/shared/database/database.service.js';
1616
import { MockFactory } from '../../mock.factory.js';
1717
import { getTestData } from '../../utils/get-test-data.js';
18+
import { getTestKnex } from '../../utils/get-test-knex.js';
1819
import { registerUserAndReturnUserInfo } from '../../utils/register-user-and-return-user-info.js';
1920
import { TestUtils } from '../../utils/test.utils.js';
2021
import {
@@ -251,6 +252,85 @@ test.serial(
251252
},
252253
);
253254

255+
test.serial(
256+
`GET /table/rows/:slug - Should preserve null FK as null and orphaned FK as raw value (not empty object)`,
257+
async (t) => {
258+
const parentTableName = 'FKBugFix_Parent';
259+
const childTableName = 'FKBugFix_Child';
260+
const knex = getTestKnex(connectionToTestDB);
261+
262+
await knex.schema.dropTableIfExists(childTableName);
263+
await knex.schema.dropTableIfExists(parentTableName);
264+
265+
await knex.schema.createTable(parentTableName, (table) => {
266+
table.increments('id').primary();
267+
table.string('label', 100);
268+
});
269+
await knex.schema.createTable(childTableName, (table) => {
270+
table.increments('id').primary();
271+
table.integer('parent_id');
272+
table.string('description', 100);
273+
});
274+
275+
const insertedParentIds = await knex(parentTableName)
276+
.insert([{ label: 'real-parent-1' }, { label: 'real-parent-to-orphan' }])
277+
.returning('id');
278+
const realParentId = (insertedParentIds[0] as any).id ?? insertedParentIds[0];
279+
const orphanedParentId = (insertedParentIds[1] as any).id ?? insertedParentIds[1];
280+
281+
await knex(childTableName).insert([
282+
{ parent_id: realParentId, description: 'valid-fk' },
283+
{ parent_id: null, description: 'null-fk' },
284+
{ parent_id: orphanedParentId, description: 'orphan-fk' },
285+
]);
286+
287+
// Now make the orphan-fk row truly orphaned by deleting its parent,
288+
// then register the FK with NOT VALID so Postgres reports it in metadata
289+
// without rejecting the existing orphan row.
290+
await knex(parentTableName).where({ id: orphanedParentId }).del();
291+
await knex.raw(
292+
`ALTER TABLE "${childTableName}" ADD CONSTRAINT fk_bugfix_child_parent FOREIGN KEY (parent_id) REFERENCES "${parentTableName}" (id) NOT VALID`,
293+
);
294+
295+
const userToken = (await registerUserAndReturnUserInfo(app)).token;
296+
297+
const createConnectionResponse = await request(app.getHttpServer())
298+
.post('/connection')
299+
.send(connectionToTestDB)
300+
.set('Cookie', userToken)
301+
.set('Content-Type', 'application/json')
302+
.set('Accept', 'application/json');
303+
t.is(createConnectionResponse.status, 201);
304+
const createConnectionRO = JSON.parse(createConnectionResponse.text);
305+
306+
const getRowsResponse = await request(app.getHttpServer())
307+
.get(`/table/rows/${createConnectionRO.id}?tableName=${childTableName}`)
308+
.set('Cookie', userToken)
309+
.set('Content-Type', 'application/json')
310+
.set('Accept', 'application/json');
311+
t.is(getRowsResponse.status, 200);
312+
const rowsRO = JSON.parse(getRowsResponse.text);
313+
314+
const validRow = rowsRO.rows.find((r: any) => r.description === 'valid-fk');
315+
const nullRow = rowsRO.rows.find((r: any) => r.description === 'null-fk');
316+
const orphanRow = rowsRO.rows.find((r: any) => r.description === 'orphan-fk');
317+
318+
t.truthy(validRow, 'valid-fk row should be present');
319+
t.truthy(nullRow, 'null-fk row should be present');
320+
t.truthy(orphanRow, 'orphan-fk row should be present');
321+
322+
// Sanity: FK was discovered (otherwise the response would not transform parent_id at all
323+
// and all branches below would trivially pass).
324+
t.is(typeof validRow.parent_id, 'object');
325+
t.truthy(validRow.parent_id);
326+
t.is(validRow.parent_id.id, realParentId);
327+
328+
// Bug: previously these were assigned `{}`. Both should preserve the original value.
329+
t.is(nullRow.parent_id, null, 'null FK must remain null, not be converted to {}');
330+
t.is(orphanRow.parent_id, orphanedParentId, 'orphaned FK must keep its raw value, not become {}');
331+
},
332+
);
333+
254334
// GET /table/structure/:connectionId
255335

256336
test.serial(

0 commit comments

Comments
 (0)