5425 - Introducing support for all Composite Fields Import #5470
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Summary
- Introduced support for importing currency fields in the spreadsheet import functionality.
- Added logic to handle 'Currency' field types, including template fields for 'Currency Code' and 'Amount Micros'.
- Enhanced data import capabilities, particularly for financial data.
Notes
- This change improves the comprehensiveness of data imports, especially for financial records.
| isDefined( | ||
| record[`${amountMicros} (${field.name})`] || | ||
| record[`${currencyCode} (${field.name})`], | ||
| ) |
There was a problem hiding this comment.
The condition isDefined(record[${amountMicros} (${field.name})] || record[${currencyCode} (${field.name})]) should be simplified to isDefined(record[${amountMicros} (${field.name})]) || isDefined(record[${currencyCode} (${field.name})]) for better readability and correctness.
zaryanz
changed the title
|
@charlesBochet Hi! An observation while importing data using a spreadsheet: |
|
Thanks @zaryanz! yes it's a best practise to store amounts in "micros" when dealing with currencies since database tend to be flaky with floats - but that's a database stuff that shouldn't be exposed to non-technical users that try to import via csv, so it would be great if you could make sure to automatically convert it by multiplying by 10^6. Thanks! |
|
Thanks @FelixMalfait for pointing that out! Silly of me to overlook the "micros" part. Could you also please point me to the different Composite Fields for "Address"? As I couldn't mind any information on that in the issue. |
|
Sure! Linked issue: #5425 Add it as a custom field type on an object to make sure it works :). It's not provisioned in any standard object for now |
zaryanz
changed the title
| @@ -80,6 +95,46 @@ export const useSpreadsheetRecordImport = (objectNameSingular: string) => { | |||
| field.label + ' (ID)', | |||
| ), | |||
| }); | |||
| } else if (field.type === FieldMetadataType.Currency) { | |||
| templateFields.push({ | |||
There was a problem hiding this comment.
can we extract the templateFields array build to an util?
I'm also not a big fan of templateFields naming. I would say availableFields
Once extracted to an util, could you also add unit tests?
| @@ -108,22 +163,89 @@ export const useSpreadsheetRecordImport = (objectNameSingular: string) => { | |||
| for (const field of fields) { | |||
There was a problem hiding this comment.
can we also extract this whole logic to an util and add unit tests?
|
Thanks for the comments! I'll have a look and make the required changes |
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added support for importing all composite fields, including Currency and Address.
- Updated ESLint rules to enforce the use of
<Link>components over navigate functions. - Refactored Chrome extension to handle cookies, side panel behavior, and removed OAuth-related code.
- Deleted extensive documentation files and related configurations, indicating a shift in documentation strategy.
- Introduced new components and utilities for email templates and frontend skeleton loaders.
246 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
|
Hey! Sorry for the delay here.. been too occupied. |
Adding support for all Composite Fields while using the "import" functionality. This includes: