-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
7203 support emails links phones in zapier inputs 2 #7562
7203 support emails links phones in zapier inputs 2 #7562
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request enhances the Twenty Zapier integration by adding support for new field types, including EMAILS, PHONES, LINKS, RICH_TEXT, POSITION, and ARRAY.
- Updated
computeInputFields.ts
to handle new field types, improving type safety withFieldMetadataType
enum - Added
formatArrayInputData
function inhandleQueryParams.ts
for processing complex array inputs - Expanded
FieldMetadataType
enum and updatedNodeField
andInputField
types indata.types.ts
- Modified test files to cover new field types and their expected behaviors
- Added 'dotenv' dependency to
package.json
for environment variable management
8 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
label: 'Whatsapp: Secondary Lings', | ||
type: 'string', | ||
helpText: 'Contact’s Whatsapp account: Link Label', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: 'Secondary Lings' is misspelled, should be 'Secondary Links'
whatsapp__secondaryLinks: [ | ||
"{url: '/secondary_whatsapp_url',label: 'Secondary Whatsapp Link'}", | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The whatsapp__secondaryLinks array contains a string instead of an object. This might cause issues when parsing the data.
phones: { | ||
primaryPhoneNumber: '322110011', | ||
primaryPhoneCountryCode: '+33', | ||
additionalPhones: ["{ phoneNumber: '322110012', countryCode: '+33' }"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The additionalPhones array contains a string representation of an object instead of an actual object. This might lead to parsing issues.
'domainName: "Company Domain Name", ' + | ||
'linkedinUrl: {url: "/linkedin_url", label: "Test linkedinUrl"}, ' + | ||
'whatsapp: {primaryLinkUrl: "/whatsapp_url", primaryLinkLabel: "Whatsapp Link", secondaryLinks: [{url: \'/secondary_whatsapp_url\',label: \'Secondary Whatsapp Link\'}]}, ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The expected result for whatsapp.secondaryLinks contains escaped single quotes. Ensure this matches the actual output of handleQueryParams.
'domainName: "Company Domain Name", ' + | ||
'linkedinUrl: {url: "/linkedin_url", label: "Test linkedinUrl"}, ' + | ||
'whatsapp: {primaryLinkUrl: "/whatsapp_url", primaryLinkLabel: "Whatsapp Link", secondaryLinks: [{url: \'/secondary_whatsapp_url\',label: \'Secondary Whatsapp Link\'}]}, ' + | ||
'emails: {primaryEmail: "[email protected]", additionalEmails: ["[email protected]"]}, ' + | ||
'phones: {primaryPhoneNumber: "322110011", primaryPhoneCountryCode: "+33", additionalPhones: [{ phoneNumber: \'322110012\', countryCode: \'+33\' }]}, ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The expected result for phones.additionalPhones contains an object literal without quotes. Verify if this matches the actual output of handleQueryParams.
@@ -141,6 +148,84 @@ const get_subfieldsFromField = (nodeField: NodeField): NodeField[] => { | |||
}; | |||
return [address1, address2, city, state, postalCode, country]; | |||
} | |||
case FieldMetadataType.PHONES: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The structure for PHONES is more complex than other fields. Consider extracting this into a separate function for better readability
@@ -1,5 +1,17 @@ | |||
import { InputData } from '../utils/data.types'; | |||
|
|||
const OBJECT_SUBFIELD_NAMES = ['secondaryLinks', 'additionalPhones']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a more descriptive name for OBJECT_SUBFIELD_NAMES, such as ARRAY_OBJECT_SUBFIELD_NAMES, to better reflect its purpose
if (Array.isArray(formattedInputData[key])) { | ||
result = result.concat( | ||
`${key}: [${formatArrayInputData(key, formattedInputData)}], `, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Ensure that formatArrayInputData is called with the correct arguments. The second argument should be formattedInputData[key], not the entire formattedInputData object
"**/*.spec.ts", | ||
"**/*.test.ts", | ||
"**/*.spec.tsx", | ||
"**/*.test.tsx", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a single pattern '**/*.{spec,test}.{ts,tsx}' to cover all test files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
## Done - add `EMAILS`, `PHONES`, `LINKS`, `RICH_TEXT`, `POSITION`, and `ARRAY` field support in Twenty zapier integration - fix `twenty-zapier` package tests and requirements ## Emails <img width="791" alt="image" src="https://github.com/user-attachments/assets/7987a1a2-6076-4715-9221-d4a1898b7634"> ## Links <img width="797" alt="image" src="https://github.com/user-attachments/assets/b94ce972-fae2-4953-b9e8-79c0478f5f60"> ## Phones <img width="789" alt="image" src="https://github.com/user-attachments/assets/7234eaaf-40b8-4772-8880-c58ba47618c5"> ## Array <img width="834" alt="image" src="https://github.com/user-attachments/assets/99cb6795-e428-40ea-9c3a-d52561c2c6e1">
Done
EMAILS
,PHONES
,LINKS
,RICH_TEXT
,POSITION
, andARRAY
field support in Twenty zapier integrationtwenty-zapier
package tests and requirementsEmails
Links
Phones
Array