From 09f0c9e29a2af89138c6a6c6fe92b5235daf5537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Sun, 14 Jun 2026 07:11:05 +0200 Subject: [PATCH] fix(address): coerce addressLat/addressLng to numbers in ORM result formatting (#21542) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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)_ Review in cubic --------- Co-authored-by: Claude --- .../format-composite-field-value.util.spec.ts | 61 +++++++++++++++++++ .../format-composite-field-value.util.ts | 38 ++++++++++++ .../twenty-orm/utils/format-result.util.ts | 23 +------ 3 files changed, 101 insertions(+), 21 deletions(-) create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/__tests__/format-composite-field-value.util.spec.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/format-composite-field-value.util.ts diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/format-composite-field-value.util.spec.ts b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/format-composite-field-value.util.spec.ts new file mode 100644 index 0000000000..787e29c6e5 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/format-composite-field-value.util.spec.ts @@ -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); + }); +}); diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/format-composite-field-value.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/format-composite-field-value.util.ts new file mode 100644 index 0000000000..86ab8c37df --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/format-composite-field-value.util.ts @@ -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; +}; diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts index 7f8814b864..68716ebe83 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts @@ -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