Skip to content

Currency input field - fix#16140

Merged
etiennejouan merged 2 commits intomainfrom
ej/fix-currency-input
Nov 27, 2025
Merged

Currency input field - fix#16140
etiennejouan merged 2 commits intomainfrom
ej/fix-currency-input

Conversation

@etiennejouan
Copy link
Copy Markdown
Contributor

Currency field used to have default value, but default value on field is not mandatory. Defaulf default value logic has been removed.

Also test all field type input when empty. ✅

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

Fixed a runtime error in CurrencyFieldInput by adding optional chaining when accessing defaultValue.currencyCode, allowing currency fields to work correctly when no default value is defined.

  • Added ?. operator to safely access defaultValue.currencyCode
  • Prevents TypeError when currency fields lack default values

Confidence Score: 4/5

  • This PR is safe to merge with one minor fix needed for the optional chaining
  • The fix correctly addresses the undefined defaultValue issue, but the optional chaining doesn't fully protect against errors since .replace() will still throw if currencyCode is undefined. This is a simple syntax issue that needs one more ? added.
  • Review the optional chaining on line 31 in CurrencyFieldInput.tsx - needs ?.replace() instead of .replace()

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/input/components/CurrencyFieldInput.tsx 4/5 Added optional chaining to defaultValue?.currencyCode to handle cases where defaultValue is undefined, preventing runtime errors when currency fields don't have default values

Sequence Diagram

sequenceDiagram
    participant User
    participant CurrencyFieldInput
    participant useCurrencyField
    participant FieldDefinition
    participant CurrencyInput

    User->>CurrencyFieldInput: Render component
    CurrencyFieldInput->>useCurrencyField: Get field data
    useCurrencyField-->>CurrencyFieldInput: Returns draftValue, defaultValue
    
    alt defaultValue is undefined
        CurrencyFieldInput->>CurrencyFieldInput: Optional chaining prevents error
        Note over CurrencyFieldInput: defaultCurrencyCodeWithoutSQLQuotes = undefined
    else defaultValue exists
        CurrencyFieldInput->>CurrencyFieldInput: Extract currencyCode and remove SQL quotes
        Note over CurrencyFieldInput: defaultCurrencyCodeWithoutSQLQuotes = value
    end
    
    CurrencyFieldInput->>CurrencyFieldInput: Determine currencyCode (draft > default > USD)
    CurrencyFieldInput->>CurrencyInput: Render with currencyCode
    
    User->>CurrencyInput: Enter amount
    CurrencyInput->>CurrencyFieldInput: handleChange(newValue)
    CurrencyFieldInput->>useCurrencyField: setDraftValue({amount, currencyCode})

Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 27, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:22768

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@etiennejouan etiennejouan enabled auto-merge (squash) November 27, 2025 14:45
@etiennejouan etiennejouan merged commit 65480eb into main Nov 27, 2025
61 checks passed
@etiennejouan etiennejouan deleted the ej/fix-currency-input branch November 27, 2025 14:53
NotYen pushed a commit to NotYen/twenty-ym that referenced this pull request Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants