Conversation
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Fixed validation guard to accept empty string as valid currencyCode value, preventing display issues when currency fields have empty currency codes from the backend.
- Modified zod schema in
isFieldCurrencyValueto acceptz.literal('')in addition to validCurrencyCodeenum values - Allows the field to pass validation and render properly with USD fallback (as implemented in
CurrencyFieldInput.tsx)
Issue found:
- Type mismatch:
FieldCurrencyValuetype definition declarescurrencyCode: CurrencyCode(non-nullable, no empty string), but the validation guard now accepts empty strings at runtime. This creates inconsistency between TypeScript types and runtime behavior.
Confidence Score: 3/5
- This PR fixes the immediate display issue but introduces a type safety concern that should be addressed
- The fix solves the rendering problem effectively, but creates a mismatch between the TypeScript type definition and runtime validation. The
FieldCurrencyValuetype should be updated to reflect that empty strings are valid values. This won't cause runtime errors due to the fallback logic inCurrencyFieldInput, but it weakens type safety and could lead to unexpected behavior in other parts of the codebase that rely on the type definition. - Check that
FieldCurrencyValuetype definition inFieldMetadata.tsis updated to include empty string as validcurrencyCodevalue
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-front/src/modules/object-record/record-field/ui/types/guards/isFieldCurrencyValue.ts | 4/5 | Added empty string literal to currency code validation schema to handle edge case where currencyCode is empty string instead of null or valid currency code |
Sequence Diagram
sequenceDiagram
participant Backend
participant CurrencyFieldInput
participant isFieldCurrencyValue
participant Display
Backend->>CurrencyFieldInput: fieldValue {currencyCode: '', amountMicros: 1000}
CurrencyFieldInput->>CurrencyFieldInput: defaultValue.currencyCode.replace(/'/g, '')
Note over CurrencyFieldInput: Removes SQL quotes, empty string remains ''
CurrencyFieldInput->>isFieldCurrencyValue: validate(fieldValue)
alt Before fix
isFieldCurrencyValue-->>CurrencyFieldInput: validation fails (empty string not in enum)
CurrencyFieldInput-->>Display: Field not rendered properly
else After fix
isFieldCurrencyValue-->>CurrencyFieldInput: validation passes (empty string accepted)
CurrencyFieldInput->>CurrencyFieldInput: Use fallback CurrencyCode.USD
CurrencyFieldInput-->>Display: Field rendered with USD fallback
end
1 file reviewed, 1 comment
|
|
||
| const currencySchema = z.object({ | ||
| currencyCode: z.enum(CurrencyCode).nullable(), | ||
| currencyCode: z.union([z.enum(CurrencyCode), z.literal('')]).nullable(), |
There was a problem hiding this comment.
logic: The FieldCurrencyValue type (line 237-240 in FieldMetadata.ts) defines currencyCode: CurrencyCode (non-nullable, no empty string), but this validation now accepts empty strings. This creates a type mismatch where runtime validation permits values that TypeScript type system forbids.
Consider updating the type definition to match reality:
export type FieldCurrencyValue = {
currencyCode: CurrencyCode | '';
amountMicros: number | null;
};Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/object-record/record-field/ui/types/guards/isFieldCurrencyValue.ts
Line: 7:7
Comment:
**logic:** The `FieldCurrencyValue` type (line 237-240 in FieldMetadata.ts) defines `currencyCode: CurrencyCode` (non-nullable, no empty string), but this validation now accepts empty strings. This creates a type mismatch where runtime validation permits values that TypeScript type system forbids.
Consider updating the type definition to match reality:
```typescript
export type FieldCurrencyValue = {
currencyCode: CurrencyCode | '';
amountMicros: number | null;
};
```
How can I resolve this? If you propose a fix, please make it concise.|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:61413 This environment will automatically shut down when the PR is closed or after 5 hours. |
Context
Amount are not displayed if the currency code is an empty string. This is not consistent everywhere so I'm removing the limitation until we plan to make the API more robuste if we decide to validate currencyCode (should be non nullable imho)
Before
After