fix(server): allow relabelling onto a field introduced in the same manifest sync (#22727)
## Context Closes twentyhq/core-team-issues#2655. `computeOrderedMigrationActions` runs `objectMetadata.update` **before** `fieldMetadata.create`. In a single manifest sync that both introduces a new field and relabels the object's `labelIdentifierFieldMetadataUniversalIdentifier` onto that field, the object update handler resolved the label identifier's universal identifier against the persisted `flatFieldMetadataMaps` only. Since the field's `fieldMetadata.create` runs later in the same migration, the field isn't in the maps yet and the sync failed with `ENTITY_NOT_FOUND`. The API metadata path is unaffected because create-field and update-object are separate requests (separate transactions), so the field is already persisted by the time the object update resolves. ## What this does `update-object-action-handler.service.ts` now resolves `labelIdentifierFieldMetadataId` and `imageIdentifierFieldMetadataId` against the deterministically preallocated field ids first, then falls back to the persisted flat maps for fields that already exist. The preallocated ids (`preallocatedIdByUniversalIdentifierByMetadataName`) are built from every create action before the migration loop starts (`buildPreallocatedIdByUniversalIdentifierFromActions`) and are the same ids `create-field-action-handler` persists the fields with. This is the same "preallocated-first, then flat maps" resolution that `resolveUniversalRelationIdentifiersToIds` already uses for modeled many-to-one relations, so no ordering change or new machinery is needed, and the single sync stays one atomic transaction. This does not touch the underlying action ordering or the hand-rolled label/image identifier handling flagged by the `#2172` TODO; generalizing those into the relation config remains the follow-up. ## Test Adds `relabel-onto-new-field-manifest-sync.integration-spec.ts` (used as the TDD reproduction, now green): - introducing a field and relabelling onto it in a single sync succeeds, and the object's `labelIdentifierFieldMetadataId` points at the new field; - the split path (introduce in one sync, relabel in the next) still succeeds and exposes the enriched `labelIdentifierFieldMetadataId` through the metadata API. Both cases pass against the fix. `oxlint`, `oxfmt`, and `typecheck` are clean. --- _Generated by [Claude Code](https://claude.ai/code/session_01KJewXMuWYUyrX3YJh2JBDE)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22727?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
Paul Rastoin
+50
-26
@@ -67,35 +67,21 @@ export class UpdateObjectActionHandlerService extends WorkspaceMigrationRunnerAc
|
||||
};
|
||||
|
||||
if (isDefined(labelIdentifierFieldMetadataUniversalIdentifier)) {
|
||||
const flatFieldMetadata = findFlatEntityByUniversalIdentifier({
|
||||
flatEntityMaps: allFlatEntityMaps.flatFieldMetadataMaps,
|
||||
universalIdentifier: labelIdentifierFieldMetadataUniversalIdentifier,
|
||||
});
|
||||
|
||||
if (!isDefined(flatFieldMetadata)) {
|
||||
throw new FlatEntityMapsException(
|
||||
`Could not resolve labelIdentifierFieldMetadataUniversalIdentifier to labelIdentifierFieldMetadataId: no fieldMetadata found for universal identifier ${labelIdentifierFieldMetadataUniversalIdentifier}`,
|
||||
FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
transpiledUpdate.labelIdentifierFieldMetadataId = flatFieldMetadata.id;
|
||||
transpiledUpdate.labelIdentifierFieldMetadataId =
|
||||
this.resolveFieldMetadataIdOrThrow({
|
||||
context,
|
||||
universalIdentifier: labelIdentifierFieldMetadataUniversalIdentifier,
|
||||
foreignKeyName: 'labelIdentifierFieldMetadataId',
|
||||
});
|
||||
}
|
||||
|
||||
if (isDefined(imageIdentifierFieldMetadataUniversalIdentifier)) {
|
||||
const flatFieldMetadata = findFlatEntityByUniversalIdentifier({
|
||||
flatEntityMaps: allFlatEntityMaps.flatFieldMetadataMaps,
|
||||
universalIdentifier: imageIdentifierFieldMetadataUniversalIdentifier,
|
||||
});
|
||||
|
||||
if (!isDefined(flatFieldMetadata)) {
|
||||
throw new FlatEntityMapsException(
|
||||
`Could not resolve imageIdentifierFieldMetadataUniversalIdentifier to imageIdentifierFieldMetadataId: no fieldMetadata found for universal identifier ${imageIdentifierFieldMetadataUniversalIdentifier}`,
|
||||
FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
transpiledUpdate.imageIdentifierFieldMetadataId = flatFieldMetadata.id;
|
||||
transpiledUpdate.imageIdentifierFieldMetadataId =
|
||||
this.resolveFieldMetadataIdOrThrow({
|
||||
context,
|
||||
universalIdentifier: imageIdentifierFieldMetadataUniversalIdentifier,
|
||||
foreignKeyName: 'imageIdentifierFieldMetadataId',
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
@@ -106,6 +92,44 @@ export class UpdateObjectActionHandlerService extends WorkspaceMigrationRunnerAc
|
||||
};
|
||||
}
|
||||
|
||||
private resolveFieldMetadataIdOrThrow({
|
||||
context,
|
||||
universalIdentifier,
|
||||
foreignKeyName,
|
||||
}: {
|
||||
context: WorkspaceMigrationActionRunnerArgs<UniversalUpdateObjectAction>;
|
||||
universalIdentifier: string;
|
||||
foreignKeyName: string;
|
||||
}): string {
|
||||
const {
|
||||
allFlatEntityMaps,
|
||||
preallocatedIdByUniversalIdentifierByMetadataName,
|
||||
} = context;
|
||||
|
||||
const preallocatedFieldMetadataId =
|
||||
preallocatedIdByUniversalIdentifierByMetadataName?.fieldMetadata?.[
|
||||
universalIdentifier
|
||||
];
|
||||
|
||||
if (isDefined(preallocatedFieldMetadataId)) {
|
||||
return preallocatedFieldMetadataId;
|
||||
}
|
||||
|
||||
const flatFieldMetadata = findFlatEntityByUniversalIdentifier({
|
||||
flatEntityMaps: allFlatEntityMaps.flatFieldMetadataMaps,
|
||||
universalIdentifier,
|
||||
});
|
||||
|
||||
if (!isDefined(flatFieldMetadata)) {
|
||||
throw new FlatEntityMapsException(
|
||||
`Could not resolve ${foreignKeyName}: no fieldMetadata found for universal identifier ${universalIdentifier}`,
|
||||
FlatEntityMapsExceptionCode.ENTITY_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
return flatFieldMetadata.id;
|
||||
}
|
||||
|
||||
async executeForMetadata(
|
||||
context: WorkspaceMigrationActionRunnerContext<FlatUpdateObjectAction>,
|
||||
): Promise<void> {
|
||||
|
||||
+184
@@ -0,0 +1,184 @@
|
||||
import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util';
|
||||
import { buildDefaultObjectManifest } from 'test/integration/metadata/suites/application/utils/build-default-object-manifest.util';
|
||||
import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
|
||||
import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
|
||||
import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util';
|
||||
import { findManyObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata.util';
|
||||
import { type ObjectManifest } from 'twenty-shared/application';
|
||||
import { FieldMetadataType } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
|
||||
const TEST_APP_ID = uuidv4();
|
||||
const TEST_ROLE_ID = uuidv4();
|
||||
const TEST_OBJECT_ID = uuidv4();
|
||||
const NAME_FIELD_ID = uuidv4();
|
||||
const CODE_FIELD_ID = uuidv4();
|
||||
|
||||
const buildTicketObject = ({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier,
|
||||
additionalFields,
|
||||
}: {
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: string;
|
||||
additionalFields: ObjectManifest['fields'];
|
||||
}) =>
|
||||
buildDefaultObjectManifest({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
universalIdentifier: TEST_OBJECT_ID,
|
||||
nameSingular: 'ticket',
|
||||
namePlural: 'tickets',
|
||||
labelSingular: 'Ticket',
|
||||
labelPlural: 'Tickets',
|
||||
description: 'A support ticket',
|
||||
icon: 'IconTicket',
|
||||
labelIdentifierFieldMetadataUniversalIdentifier,
|
||||
additionalFields,
|
||||
});
|
||||
|
||||
const NAME_FIELD: ObjectManifest['fields'][number] = {
|
||||
universalIdentifier: NAME_FIELD_ID,
|
||||
type: FieldMetadataType.TEXT,
|
||||
name: 'name',
|
||||
label: 'Name',
|
||||
};
|
||||
|
||||
const CODE_FIELD: ObjectManifest['fields'][number] = {
|
||||
universalIdentifier: CODE_FIELD_ID,
|
||||
type: FieldMetadataType.TEXT,
|
||||
name: 'code',
|
||||
label: 'Code',
|
||||
};
|
||||
|
||||
const buildManifest = (object: ObjectManifest) =>
|
||||
buildBaseManifest({
|
||||
appId: TEST_APP_ID,
|
||||
roleId: TEST_ROLE_ID,
|
||||
overrides: { objects: [object] },
|
||||
});
|
||||
|
||||
const findTicketObject = async () => {
|
||||
const { objects } = await findManyObjectMetadata({
|
||||
input: {
|
||||
filter: {},
|
||||
paging: { first: 100 },
|
||||
},
|
||||
gqlFields: `
|
||||
id
|
||||
universalIdentifier
|
||||
labelIdentifierFieldMetadataId
|
||||
fieldsList {
|
||||
id
|
||||
name
|
||||
universalIdentifier
|
||||
}
|
||||
`,
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
return objects.find(
|
||||
(object) => object.universalIdentifier === TEST_OBJECT_ID,
|
||||
);
|
||||
};
|
||||
|
||||
describe('Manifest sync - relabel label identifier onto a newly introduced field', () => {
|
||||
beforeEach(async () => {
|
||||
await setupApplicationForSync({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
name: 'Test Application',
|
||||
description: 'App for testing label identifier relabel on manifest sync',
|
||||
sourcePath: 'test-relabel-onto-new-field',
|
||||
});
|
||||
}, 60000);
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanupApplicationAndAppRegistration({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
});
|
||||
});
|
||||
|
||||
it('should introduce a new field and relabel onto it in a single sync', async () => {
|
||||
await syncApplication({
|
||||
manifest: buildManifest(
|
||||
buildTicketObject({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: NAME_FIELD_ID,
|
||||
additionalFields: [NAME_FIELD],
|
||||
}),
|
||||
),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
const ticketAfterFirstSync = await findTicketObject();
|
||||
|
||||
expect(ticketAfterFirstSync).toBeDefined();
|
||||
|
||||
const nameField = ticketAfterFirstSync?.fieldsList?.find(
|
||||
(field) => field.name === 'name',
|
||||
);
|
||||
|
||||
expect(nameField).toBeDefined();
|
||||
expect(ticketAfterFirstSync?.labelIdentifierFieldMetadataId).toBe(
|
||||
nameField?.id,
|
||||
);
|
||||
|
||||
await syncApplication({
|
||||
manifest: buildManifest(
|
||||
buildTicketObject({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: CODE_FIELD_ID,
|
||||
additionalFields: [NAME_FIELD, CODE_FIELD],
|
||||
}),
|
||||
),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
const ticketAfterSecondSync = await findTicketObject();
|
||||
const codeField = ticketAfterSecondSync?.fieldsList?.find(
|
||||
(field) => field.name === 'code',
|
||||
);
|
||||
|
||||
expect(codeField).toBeDefined();
|
||||
expect(ticketAfterSecondSync?.labelIdentifierFieldMetadataId).toBe(
|
||||
codeField?.id,
|
||||
);
|
||||
}, 60000);
|
||||
|
||||
it('should relabel onto a newly introduced field when introduction and relabel are split across two syncs, exposing the enriched labelIdentifierFieldMetadataId through the metadata API', async () => {
|
||||
await syncApplication({
|
||||
manifest: buildManifest(
|
||||
buildTicketObject({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: NAME_FIELD_ID,
|
||||
additionalFields: [NAME_FIELD],
|
||||
}),
|
||||
),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
await syncApplication({
|
||||
manifest: buildManifest(
|
||||
buildTicketObject({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: NAME_FIELD_ID,
|
||||
additionalFields: [NAME_FIELD, CODE_FIELD],
|
||||
}),
|
||||
),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
await syncApplication({
|
||||
manifest: buildManifest(
|
||||
buildTicketObject({
|
||||
labelIdentifierFieldMetadataUniversalIdentifier: CODE_FIELD_ID,
|
||||
additionalFields: [NAME_FIELD, CODE_FIELD],
|
||||
}),
|
||||
),
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
const ticket = await findTicketObject();
|
||||
const codeField = ticket?.fieldsList?.find(
|
||||
(field) => field.name === 'code',
|
||||
);
|
||||
|
||||
expect(codeField).toBeDefined();
|
||||
expect(isDefined(ticket?.labelIdentifierFieldMetadataId)).toBe(true);
|
||||
expect(ticket?.labelIdentifierFieldMetadataId).toBe(codeField?.id);
|
||||
}, 60000);
|
||||
});
|
||||
Reference in New Issue
Block a user