Skip to content
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

Fix zapier tests #7568

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Fix zapier tests #7568

merged 1 commit into from
Oct 10, 2024

Conversation

martmull
Copy link
Contributor

See title

@martmull martmull marked this pull request as ready for review October 10, 2024 14:15
Copy link
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.

PR Summary

This pull request updates the Zapier integration tests and related utilities, focusing on improved flexibility and support for new data structures in the Twenty CRM system.

  • Modified packages/twenty-zapier/src/test/authentication.test.ts to be less brittle by checking for a defined error message instead of a specific one
  • Updated packages/twenty-zapier/src/test/creates/crud_record.test.ts to accommodate new data formats for linkedinLink, xLink, and phones fields
  • Added list and placeholder properties in packages/twenty-zapier/src/test/utils/computeInputFields.test.ts and packages/twenty-zapier/src/utils/computeInputFields.ts
  • Removed test file exclusions in packages/twenty-zapier/tsconfig.json, allowing TypeScript compilation of test files

5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 62 to 63
expect(response.data).toHaveProperty('currentWorkspace');
expect(response.data.currentWorkspace).toHaveProperty('displayName');
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: These expectations are unreachable if an error is thrown. Consider moving them before the try-catch block or removing them.

Comment on lines +18 to +24
linkedinLink: {
primaryLinkUrl: '/linkedin_url',
primaryLinkLabel: 'Test linkedinUrl',
secondaryLinks: [
'{ url: "/linkedin_url2", label: "Test linkedinUrl2" }',
],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The secondaryLinks are stored as a string representation of an object, which may cause issues when parsing. Consider using a proper object structure instead of a string.

Comment on lines +25 to +29
xLink: {
primaryLinkUrl: '/x_url',
primaryLinkLabel: 'Test xUrl',
secondaryLinks: ['{ url: "/x_url2", label: "Test xUrl2" }'],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Similar to the linkedinLink, the xLink secondaryLinks are stored as a string. Ensure this is the intended format and that it can be properly parsed.

Comment on lines +61 to +65
phones: {
primaryPhoneNumber: '610203040',
primaryPhoneCountryCode: '+33',
additionalPhones: ['{number: "610203041", countryCode: "+33"}'],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: The additionalPhones field is using a string representation of an object. This might cause parsing issues. Consider using a proper object structure.

@martmull martmull enabled auto-merge (squash) October 10, 2024 14:23
@martmull martmull merged commit c6a676e into main Oct 10, 2024
5 checks passed
@martmull martmull deleted the fix-zapier-tests branch October 10, 2024 14:34
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
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