5425 - Introducing support for all Composite Fields Import #5470

zaryanz · 2024-05-20T10:05:31Z

Adding support for all Composite Fields while using the "import" functionality. This includes:

Currency
Address

Edit :

Refactored a lot of types in the spreadsheet import module
Renamed a lot of functions, hooks and types that were not self-explanatory enough

…eadsheet feature

greptile-apps

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.

greptile-apps · 2024-05-20T10:06:01Z

...ages/twenty-front/src/modules/object-record/spreadsheet-import/useSpreadsheetRecordImport.ts

+                  isDefined(
+                    record[`${amountMicros} (${field.name})`] ||
+                      record[`${currencyCode} (${field.name})`],
+                  )


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 · 2024-05-20T10:16:35Z

@charlesBochet Hi! An observation while importing data using a spreadsheet:
$1,000,000 is being saved as $1 after submitting and anything less than $1,000,000 is being saved as $0. Is this an existing issue that we are aware of or should we raise an issue for the same?

FelixMalfait · 2024-05-20T10:31:34Z

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!

zaryanz · 2024-05-20T10:40:51Z

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.

FelixMalfait · 2024-05-20T10:47:46Z

Sure!

Linked issue: #5425

Address field: https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/metadata-modules/field-metadata/composite-types/address.composite-type.ts

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

charlesBochet

Thanks a lot for the help! I have left comments on the PR

charlesBochet · 2024-05-29T12:53:18Z

...ages/twenty-front/src/modules/object-record/spreadsheet-import/useSpreadsheetRecordImport.ts

@@ -80,6 +95,46 @@ export const useSpreadsheetRecordImport = (objectNameSingular: string) => {
          field.label + ' (ID)',
        ),
      });
+    } else if (field.type === FieldMetadataType.Currency) {
+      templateFields.push({


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?

charlesBochet · 2024-05-29T12:53:47Z

...ages/twenty-front/src/modules/object-record/spreadsheet-import/useSpreadsheetRecordImport.ts

@@ -108,22 +163,89 @@ export const useSpreadsheetRecordImport = (objectNameSingular: string) => {
          for (const field of fields) {


can we also extract this whole logic to an util and add unit tests?

zaryanz · 2024-05-30T17:33:35Z

Thanks for the comments! I'll have a look and make the required changes

greptile-apps

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}

zaryanz · 2024-06-27T14:19:29Z

Hey! Sorry for the delay here.. been too occupied.
Will work on the review comments this weekend. Thanks for the patience.

…o a custom hook and fieldMapping into a utility function

…mport

…sUtil

github-actions · 2024-07-23T14:24:27Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 3087:64B40:3019E42:5951264:669FD443.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Azaryanz%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Tue, 23 Jul 2024 16:03:15 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'3087:64B40:3019E42:5951264:669FD443'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1721750655'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 3087:64B40:3019E42:5951264:669FD443.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Azaryanz%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-4476b3b4.json

Generated by 🚫 dangerJS against 0804cc5

feat: introducing support for Currency composite fields in import spr…

9534d07

…eadsheet feature

greptile-apps bot reviewed May 20, 2024

View reviewed changes

zaryanz changed the title ~~5425 Introducing support for all Composite Fields Import~~ 5425 WIP - Introducing support for all Composite Fields Import May 20, 2024

zaryanz added 2 commits May 20, 2024 16:31

fix: use convertCurrencyMicrosToCurrency before persisting

14cd8e2

feat: introducing support for Address composite fields

b9557aa

zaryanz changed the title ~~5425 WIP - Introducing support for all Composite Fields Import~~ 5425 - Introducing support for all Composite Fields Import May 21, 2024

fix: updated unused variable key to _

7d6f07d

charlesBochet self-assigned this May 22, 2024

charlesBochet added the -PR: awaiting review label May 22, 2024

charlesBochet added 2 commits May 29, 2024 13:24

Merge branch 'main' into 5425-support-all-composite-fields-import

934ba95

Fix unrelated bug

3c9613e

charlesBochet reviewed May 29, 2024

View reviewed changes

charlesBochet added -PR: awaiting author and removed -PR: awaiting review labels May 29, 2024

charlesBochet mentioned this pull request May 29, 2024

Fixed: In CSV import now users are able to come back to the previous step. #5625

Merged

Merge main

455f3c4

greptile-apps bot reviewed Jun 27, 2024

View reviewed changes

zaryanz and others added 7 commits July 16, 2024 22:55

feat: moved creation of availableFields (formerly templateFields) int…

5b5f231

…o a custom hook and fieldMapping into a utility function

Merge branch 'twentyhq:main' into 5425-support-all-composite-fields-i…

fb34906

…mport

pull and merge main

21add21

fix: pull remote

1b50dfe

fix: updated templateFields variable name to availableFields in field…

195b09b

…sUtil

Merge branch 'main' into 5425-support-all-composite-fields-import

1626fdb

Fixed naming

882fc89

lucasbordeau added 8 commits July 23, 2024 11:05

Fixed naming

941ceeb

Fixed typing and naming

ce473da

Refactored

eb44dae

Export amount micros as amount

3b47094

Fix

be59c00

Fix

1990d25

Fix

37772a1

Fix

925c129

lucasbordeau approved these changes Jul 23, 2024

View reviewed changes

lucasbordeau added -PR: awaiting CI and removed -PR: awaiting author labels Jul 23, 2024

lucasbordeau assigned lucasbordeau and unassigned charlesBochet Jul 23, 2024

lucasbordeau added 3 commits July 23, 2024 15:31

Fix

fb3c0b3

Fix

2ec4725

Fixed tests

0804cc5

lucasbordeau merged commit 5c8fe02 into twentyhq:main Jul 23, 2024
11 checks passed

lucasbordeau mentioned this pull request Jul 25, 2024

Support all Composite Fields Import #5425

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5425 - Introducing support for all Composite Fields Import #5470

5425 - Introducing support for all Composite Fields Import #5470

zaryanz commented May 20, 2024 •

edited by lucasbordeau

Loading

greptile-apps bot left a comment

greptile-apps bot May 20, 2024

zaryanz commented May 20, 2024 •

edited

Loading

FelixMalfait commented May 20, 2024

zaryanz commented May 20, 2024

FelixMalfait commented May 20, 2024 •

edited

Loading

charlesBochet left a comment

charlesBochet May 29, 2024

charlesBochet May 29, 2024

zaryanz commented May 30, 2024

greptile-apps bot left a comment

zaryanz commented Jun 27, 2024

github-actions bot commented Jul 23, 2024 •

edited

Loading

		@@ -108,22 +163,89 @@ export const useSpreadsheetRecordImport = (objectNameSingular: string) => {
		for (const field of fields) {

5425 - Introducing support for all Composite Fields Import #5470

5425 - Introducing support for all Composite Fields Import #5470

Conversation

zaryanz commented May 20, 2024 • edited by lucasbordeau Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

Pull Request Summary

Notes

greptile-apps bot May 20, 2024

Choose a reason for hiding this comment

zaryanz commented May 20, 2024 • edited Loading

FelixMalfait commented May 20, 2024

zaryanz commented May 20, 2024

FelixMalfait commented May 20, 2024 • edited Loading

charlesBochet left a comment

Choose a reason for hiding this comment

charlesBochet May 29, 2024

Choose a reason for hiding this comment

charlesBochet May 29, 2024

Choose a reason for hiding this comment

zaryanz commented May 30, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

zaryanz commented Jun 27, 2024

github-actions bot commented Jul 23, 2024 • edited Loading

Log

zaryanz commented May 20, 2024 •

edited by lucasbordeau

Loading

zaryanz commented May 20, 2024 •

edited

Loading

FelixMalfait commented May 20, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading