fix(address): coerce addressLat/addressLng to numbers in ORM result formatting (#21542)
## Fixes #21390 Saved addresses render as the **"Empty"** placeholder in the record detail / side panel when `addressLat`/`addressLng` are populated (e.g. after picking a Google autocomplete suggestion). The list view shows the address correctly. ## Root cause `addressLat`/`addressLng` are `NUMERIC` composite subfields, stored as Postgres `numeric` columns — which the `pg` driver returns as **strings** to preserve precision. The ORM result formatter already normalizes this for Currency, but not for Address. In `packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts`, `formatCompositeFieldValue` had a case for `CURRENCY.amountMicros` (`parseInt`) but **no case for `ADDRESS`**, so coordinates were passed through as raw strings. This only breaks the **record detail**, not the list view, because: - The **standard GraphQL (Yoga) path** masks it — the `BigFloat` scalar's `serialize()` runs `parseFloat()` and quietly turns the string into a number on the wire. - The **direct-execution path** formats results itself and bypasses scalar serialization, so the string reaches the frontend. There, `addressFieldValueSchema` validates lat/lng with `z.number()` → `isFieldAddressValue` returns `false` → `isFieldValueEmpty` returns `true` → `RecordInlineCellDisplayMode` renders the placeholder. The table cell renders the value directly with no empty-check, so the list view is unaffected. ## Why it surfaced now The `z.number()` constraint on lat/lng is old ("latent since the address guard was introduced"). The trigger was **#19254 (2026-04-03) "Remove direct execution feature flag"**, which made direct execution always-on for workspace queries — the same PR added string→number coercion for aggregates but not for composite subfields. **#21033** (the PR the issue blames) only made `addressStreet1` nullable; it didn't touch lat/lng, but by fixing the overlapping null-street1 case it isolated and exposed this one. ## Fix Add the `ADDRESS` case to `formatCompositeFieldValue`, mirroring Currency. Coordinates are fractional, so `parseFloat` is used (Currency uses `parseInt` because micros are integers). This is the exact operation the `BigFloat` scalar already performs, so there is no behavior change on the standard path — it just makes direct execution consistent, and lat/lng are now numbers everywhere (matching `FieldAddressValue`). No frontend change is needed. ## Scope check — similar bugs in other field types/composites This bug class = a transforming scalar `serialize` that direct execution doesn't replicate. The only scalar that changes a pg-returned type for a real field is `BigFloat` (`NUMERIC` → number). The only `NUMERIC` fields are the two composite subfields: - `CURRENCY.amountMicros` — already handled ✅ - `ADDRESS.addressLat` / `addressLng` — fixed here ✅ Standalone `NUMERIC` is not user-creatable (it's in `SettingsExcludedFieldType`). Other scalars were checked and don't diverge: `Date.serialize` is identity; `NUMBER`/`POSITION` are stored as `float8` and returned as numbers (and `NUMBER` is already coerced in direct execution); `DATE_TIME` resolves to the same ISO string via both paths. So `ADDRESS` was the last gap. ## Tests - New `format-result.util.spec.ts`: `addressLat`/`addressLng` strings parse to numbers, already-number coordinates pass through, numeric-looking text subfields (e.g. `addressPostcode: "10001"`) are **not** coerced, and the existing Currency `amountMicros` coercion still holds. - `npx jest format-result.util.spec` → 4 passed - `npx nx lint:diff-with-main twenty-server` → 0 warnings, 0 errors - `npx nx typecheck twenty-server` → pass ## Follow-ups (not in this PR) - The cross-path parity integration test (#18972) doesn't cover a record with address coordinates — worth adding so this class can't regress. - `formatAddressDisplay` falls back to `ALLOWED_ADDRESS_SUBFIELDS` (which includes lat/lng) when a field has no `subFields` configured, unlike `getEnabledAddressSubFields` (which falls back to the text-only `DEFAULT_VISIBLE_ADDRESS_SUBFIELDS`). Harmless now that coordinates are numbers (filtered by `isNonEmptyString`), but a latent inconsistency. https://claude.ai/code/session_011pf9KQn4UDZGr4V4k8rRHh --- _Generated by [Claude Code](https://claude.ai/code/session_011pf9KQn4UDZGr4V4k8rRHh)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21542?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. --> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
+61
@@ -0,0 +1,61 @@
|
||||
import { FieldMetadataType } from 'twenty-shared/types';
|
||||
|
||||
import { getFlatFieldMetadataMock } from 'src/engine/metadata-modules/flat-field-metadata/__mocks__/get-flat-field-metadata.mock';
|
||||
import { formatCompositeFieldValue } from 'src/engine/twenty-orm/utils/format-composite-field-value.util';
|
||||
|
||||
describe('formatCompositeFieldValue', () => {
|
||||
const addressFieldMetadata = getFlatFieldMetadataMock({
|
||||
universalIdentifier: 'address',
|
||||
objectMetadataId: 'object-metadata-id',
|
||||
type: FieldMetadataType.ADDRESS,
|
||||
});
|
||||
|
||||
const currencyFieldMetadata = getFlatFieldMetadataMock({
|
||||
universalIdentifier: 'amount',
|
||||
objectMetadataId: 'object-metadata-id',
|
||||
type: FieldMetadataType.CURRENCY,
|
||||
});
|
||||
|
||||
it('should parse addressLat/addressLng returned as strings into numbers', () => {
|
||||
expect(
|
||||
formatCompositeFieldValue(
|
||||
'40.7532256',
|
||||
'addressLat',
|
||||
addressFieldMetadata,
|
||||
),
|
||||
).toBe(40.7532256);
|
||||
expect(
|
||||
formatCompositeFieldValue(
|
||||
'-73.99294600000002',
|
||||
'addressLng',
|
||||
addressFieldMetadata,
|
||||
),
|
||||
).toBe(-73.99294600000002);
|
||||
});
|
||||
|
||||
it('should keep coordinates that are already numbers unchanged', () => {
|
||||
expect(
|
||||
formatCompositeFieldValue(40.7532256, 'addressLat', addressFieldMetadata),
|
||||
).toBe(40.7532256);
|
||||
});
|
||||
|
||||
it('should not coerce text address subfields that look numeric', () => {
|
||||
expect(
|
||||
formatCompositeFieldValue(
|
||||
'10001',
|
||||
'addressPostcode',
|
||||
addressFieldMetadata,
|
||||
),
|
||||
).toBe('10001');
|
||||
});
|
||||
|
||||
it('should still parse currency amountMicros returned as a string', () => {
|
||||
expect(
|
||||
formatCompositeFieldValue(
|
||||
'5000000',
|
||||
'amountMicros',
|
||||
currencyFieldMetadata,
|
||||
),
|
||||
).toBe(5000000);
|
||||
});
|
||||
});
|
||||
+38
@@ -0,0 +1,38 @@
|
||||
import { isNonEmptyString } from '@sniptt/guards';
|
||||
import { FieldMetadataType } from 'twenty-shared/types';
|
||||
|
||||
import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
|
||||
|
||||
export const formatCompositeFieldValue = (
|
||||
value: unknown,
|
||||
compositePropertyName: string,
|
||||
fieldMetadata: FlatFieldMetadata,
|
||||
) => {
|
||||
switch (fieldMetadata.type) {
|
||||
case FieldMetadataType.CURRENCY: {
|
||||
if (compositePropertyName === 'amountMicros') {
|
||||
if (isNonEmptyString(value)) {
|
||||
return parseInt(value);
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
break;
|
||||
}
|
||||
case FieldMetadataType.ADDRESS: {
|
||||
if (
|
||||
compositePropertyName === 'addressLat' ||
|
||||
compositePropertyName === 'addressLng'
|
||||
) {
|
||||
if (isNonEmptyString(value)) {
|
||||
return parseFloat(value);
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
return value;
|
||||
};
|
||||
@@ -1,6 +1,6 @@
|
||||
import { isPlainObject } from '@nestjs/common/utils/shared.utils';
|
||||
|
||||
import { isNonEmptyString, isNull } from '@sniptt/guards';
|
||||
import { isNull } from '@sniptt/guards';
|
||||
import {
|
||||
FieldActorSource,
|
||||
FieldMetadataType,
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
type FieldMapsForObject,
|
||||
} from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util';
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
import { formatCompositeFieldValue } from 'src/engine/twenty-orm/utils/format-composite-field-value.util';
|
||||
import { getCompositeFieldMetadataCollection } from 'src/engine/twenty-orm/utils/get-composite-field-metadata-collection';
|
||||
import { isFieldMetadataEntityOfType } from 'src/engine/utils/is-field-metadata-of-type.util';
|
||||
|
||||
@@ -292,26 +293,6 @@ function transformCompositeFieldNullValue(
|
||||
);
|
||||
}
|
||||
|
||||
function formatCompositeFieldValue(
|
||||
value: unknown,
|
||||
compositePropertyName: string,
|
||||
fieldMetadata: FlatFieldMetadata,
|
||||
) {
|
||||
switch (fieldMetadata.type) {
|
||||
case FieldMetadataType.CURRENCY: {
|
||||
if (compositePropertyName === 'amountMicros') {
|
||||
if (isNonEmptyString(value)) {
|
||||
return parseInt(value);
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Handles composite fields with missing required subfields.
|
||||
* - For nullable fields: sets to null if all required subfields are null
|
||||
|
||||
Reference in New Issue
Block a user