Add Attio CRM integration #320
Conversation
|
WalkthroughThis update integrates Attio into the CRM Unified API, focusing on OAuth callbacks, database interactions, and contact syncing. It introduces the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 9
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
docker-compose.dev.ymlis excluded by:!**/*.ymldocker-compose.source.ymlis excluded by:!**/*.ymldocker-compose.ymlis excluded by:!**/*.yml
Files selected for processing (17)
- .env.example (2 hunks)
- packages/api/src/@core/connections/connections.controller.ts (1 hunks)
- packages/api/src/@core/connections/crm/crm.connection.module.ts (2 hunks)
- packages/api/src/@core/connections/crm/services/attio/attio.service.ts (1 hunks)
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts (2 hunks)
- packages/api/src/@core/connections/crm/types/index.ts (1 hunks)
- packages/api/src/@core/environment/environment.service.ts (2 hunks)
- packages/api/src/@core/utils/types/index.ts (4 hunks)
- packages/api/src/crm/@utils/@types/index.ts (1 hunks)
- packages/api/src/crm/contact/contact.module.ts (2 hunks)
- packages/api/src/crm/contact/services/attio/index.ts (1 hunks)
- packages/api/src/crm/contact/services/attio/mappers.ts (1 hunks)
- packages/api/src/crm/contact/services/attio/types.ts (1 hunks)
- packages/api/src/crm/contact/types/mappingsTypes.ts (2 hunks)
- packages/shared/src/authUrl.ts (2 hunks)
- packages/shared/src/enum.ts (2 hunks)
- packages/shared/src/utils.ts (4 hunks)
Check Runs (1)
Build and Test (18.x) failure (17)
- packages/api/src/@core/connections/crm/services/attio/attio.service.ts: 110-110: 'opts' is defined but never used
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts: 109-109: 'data' is assigned a value but never used
Additional comments: 20
packages/shared/src/enum.ts (1)
- 18-18: The addition of
ATTIOto theCrmProvidersenum is correctly implemented and follows the existing naming conventions.packages/api/src/@core/connections/crm/types/index.ts (1)
- 25-28: The
AttioOAuthResponseinterface is correctly defined and aligns with the structure and naming conventions of existing OAuth response interfaces in the file.packages/api/src/crm/contact/contact.module.ts (1)
- 37-37: The addition of
AttioServiceto theContactModuleproviders is correctly implemented, ensuring that the service is available for dependency injection within the module.packages/api/src/@core/connections/crm/crm.connection.module.ts (1)
- 30-30: The addition of
AttioConnectionServiceto theCrmConnectionModuleproviders is correctly implemented, ensuring that the service is available for dependency injection within the module.packages/api/src/crm/contact/types/mappingsTypes.ts (1)
- 36-39: The addition of
AttioContactMapperand its integration intocontactUnificationMappingis correctly implemented, ensuring compatibility between the system and Attio CRM for contact data conversion..env.example (2)
- 45-47: The addition of Attio configuration settings (
ATTIO_CLIENT_IDandATTIO_CLIENT_SECRET) is correctly implemented, ensuring the system can authenticate with Attio's OAuth service.- 61-62: The update to
NEXT_PUBLIC_FRONTEND_DOMAINand the addition ofNEXT_PUBLIC_ML_FRONTED_DOMAINare correctly implemented, aligning with the system's configuration requirements.packages/shared/src/authUrl.ts (2)
- 15-16: Consider refining the use of console logs for debugging purposes. While helpful during development, it's best to replace them with a more sophisticated logging mechanism for production to avoid exposing sensitive information.
- 38-51: The modifications to the logic for constructing finalAuth URLs based on
providerNameare correctly implemented, supporting the integration with Attio and potentially other providers.packages/api/src/@core/environment/environment.service.ts (1)
- 50-55: The addition of the
getAttioAuth()method is correctly implemented, allowing the system to retrieve Attio's OAuth credentials from environment variables in a manner consistent with other providers.packages/api/src/crm/contact/services/attio/types.ts (1)
- 1-127: The interfaces defined for Attio's contact data structure are well-structured and adhere to TypeScript best practices. However, consider documenting each interface and its properties to improve code readability and maintainability, especially for complex structures like these.
packages/api/src/@core/utils/types/index.ts (4)
- 34-34: Adding
attioto thedomainsobject with its API base URL is a good practice for centralizing configuration settings. Ensure that the URL is correct and accessible.- 47-47: Including
attioin thecustomPropertiesUrlsobject with a specific documentation URL is helpful for developers. Verify that this URL points to the correct documentation section for Attio's standard objects and people.- 72-72: Adding
attioto theCrmProvidersenum is necessary for handling different CRM providers consistently. This change looks good.- 91-91: Including
attioin theCRM_PROVIDERSarray is essential for iterating over supported CRM providers. This addition is correctly implemented.packages/api/src/crm/contact/services/attio/mappers.ts (1)
- 13-131: The
AttioContactMapperclass provides a clear and structured approach to mapping between unified contact models and Attio-specific models. However, consider adding comments to explain the mapping logic, especially for complex transformations, to improve maintainability and understandability of the code.packages/api/src/crm/@utils/@types/index.ts (1)
- 112-112: Exporting Attio service types is a good practice for modular code organization and reuse. This change facilitates the integration of Attio-specific types across the application.
packages/shared/src/utils.ts (3)
- 133-133: The
getDescriptionfunction correctly retrieves the description of a provider based on its name. However, consider adding error handling or logging for cases where the provider name does not exist in the configuration. This will improve the debuggability and robustness of the function.Consider adding error logging for unmatched provider names to aid in debugging.
- 149-149: The
providersArrayfunction is well-implemented for converting the providers' configuration into an array format. To enhance the function's usability, consider including thedescriptionfield in the returned objects if it's deemed useful for the consuming code. This would provide a more comprehensive overview of each provider directly from the array.Consider including the
descriptionfield in the objects returned byprovidersArrayif it aligns with the function's usage context.
- 20-34: Minor formatting changes in the
providersConfigobject and the utility functions improve readability and maintainability. These changes are generally positive, enhancing the clarity of the code.The formatting changes and updates to the
providersConfigobject are well-executed, contributing to the code's overall readability and maintainability.
packages/shared/src/utils.ts
Outdated
| // Add client id in their respective crm | ||
| 'hubspot': { | ||
| clientId: 'ba591170-a7c7-4fca-8086-1bd178c6b14d', | ||
| clientId: 'Add hubspot requested client id', | ||
| scopes: 'crm.objects.contacts.read crm.objects.contacts.write crm.schemas.deals.read crm.schemas.deals.write crm.objects.deals.read crm.objects.deals.write crm.objects.companies.read crm.objects.companies.write crm.objects.owners.read settings.users.read settings.users.write settings.users.teams.read settings.users.teams.write', | ||
| authBaseUrl: 'https://app-eu1.hubspot.com/oauth/authorize', | ||
| logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png", | ||
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | ||
| }, | ||
| 'attio': { | ||
| clientId: '<Add Attio requested client id>', | ||
| scopes: 'record_permission:read', | ||
| authBaseUrl: 'https://app.attio.com/authorize', | ||
| logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png", | ||
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | ||
| }, |
There was a problem hiding this comment.
The addition of the clientId placeholder for Attio and Hubspot in the providersConfig object is a crucial step for integrating these CRMs. However, there are a few points to consider:
- For Attio's
clientId, it's mentioned as<Add Attio requested client id>. Ensure that this placeholder is replaced with the actualclientIdobtained from Attio during deployment or through an environment variable to avoid hardcoding sensitive information. - The
logoPathfor Attio is currently using Hubspot's logo URL. This should be updated to reflect Attio's actual logo to avoid confusion and ensure accurate representation in the UI.
- logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png",
+ logoPath: "<Add Attio logo URL>",Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // Add client id in their respective crm | |
| 'hubspot': { | |
| clientId: 'ba591170-a7c7-4fca-8086-1bd178c6b14d', | |
| clientId: 'Add hubspot requested client id', | |
| scopes: 'crm.objects.contacts.read crm.objects.contacts.write crm.schemas.deals.read crm.schemas.deals.write crm.objects.deals.read crm.objects.deals.write crm.objects.companies.read crm.objects.companies.write crm.objects.owners.read settings.users.read settings.users.write settings.users.teams.read settings.users.teams.write', | |
| authBaseUrl: 'https://app-eu1.hubspot.com/oauth/authorize', | |
| logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png", | |
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | |
| }, | |
| 'attio': { | |
| clientId: '<Add Attio requested client id>', | |
| scopes: 'record_permission:read', | |
| authBaseUrl: 'https://app.attio.com/authorize', | |
| logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png", | |
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | |
| }, | |
| // Add client id in their respective crm | |
| 'hubspot': { | |
| clientId: 'Add hubspot requested client id', | |
| scopes: 'crm.objects.contacts.read crm.objects.contacts.write crm.schemas.deals.read crm.schemas.deals.write crm.objects.deals.read crm.objects.deals.write crm.objects.companies.read crm.objects.companies.write crm.objects.owners.read settings.users.read settings.users.write settings.users.teams.read settings.users.teams.write', | |
| authBaseUrl: 'https://app-eu1.hubspot.com/oauth/authorize', | |
| logoPath: "https://assets-global.website-files.com/6421a177cdeeaf3c6791b745/64d61202dd99e63d40d446f6_hubspot%20logo.png", | |
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | |
| }, | |
| 'attio': { | |
| clientId: '<Add Attio requested client id>', | |
| scopes: 'record_permission:read', | |
| authBaseUrl: 'https://app.attio.com/authorize', | |
| logoPath: "<Add Attio logo URL>", | |
| description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users" | |
| }, |
| console.log("In connection controller - Mit") | ||
| this.logger.log("in connection controller - Mit") |
There was a problem hiding this comment.
Using both console.log and this.logger.log for logging the same message is redundant. It's recommended to stick with this.logger.log for consistency and to leverage the logging framework's capabilities, such as log levels and formatting.
- console.log("In connection controller - Mit")
- this.logger.log("in connection controller - Mit")
+ this.logger.log("In connection controller - Mit")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| console.log("In connection controller - Mit") | |
| this.logger.log("in connection controller - Mit") | |
| this.logger.log("In connection controller - Mit") |
| this.logger.log("callback called") | ||
| console.log("callback called") |
There was a problem hiding this comment.
Similar to the previous file, using both console.log and this.logger.log for logging the same message is redundant. Prefer using this.logger.log for consistency across the application.
- this.logger.log("callback called")
- console.log("callback called")
+ this.logger.log("Callback called")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| this.logger.log("callback called") | |
| console.log("callback called") | |
| this.logger.log("Callback called") |
| zohoLocation?: string, | ||
| ) { | ||
| try { | ||
| this.logger.log("callback called") | ||
| console.log("callback called") | ||
| if (!code) { | ||
| throw new NotFoundError(`no ${providerName} code found, found ${code}`); | ||
| } | ||
|
|
||
| const serviceName = providerName.toLowerCase(); | ||
| this.logger.log("callback in connection service - Mit") | ||
|
|
||
| const service = this.serviceRegistry.getService(serviceName); | ||
|
|
||
| if (!service) { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [51-64]
The console.log statement here seems to be used for debugging purposes. It's recommended to remove it or replace it with this.logger.log if you intend to keep it for logging purposes in production.
- console.log("In callback2")📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [109-109]
The variable data is assigned a value but never used. If the intention was to log or process this data, ensure that it's being utilized appropriately. Otherwise, consider removing this assignment to clean up the code.
| console.log("Before Axios") | ||
| console.log(this.cryptoService.decrypt(connection.access_token)) |
There was a problem hiding this comment.
Debugging statements using console.log should be replaced with this.logger.log for consistency and to leverage the logging framework's capabilities. Additionally, consider removing or commenting out these logs before merging into production code.
- console.log("Before Axios")
- console.log(this.cryptoService.decrypt(connection.access_token))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| console.log("Before Axios") | |
| console.log(this.cryptoService.decrypt(connection.access_token)) |
|
|
||
| ); | ||
|
|
||
| console.log("After Axios") |
There was a problem hiding this comment.
As with the previous comment, replace console.log with this.logger.log for logging purposes, or remove this debugging statement before merging.
- console.log("After Axios")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| console.log("After Axios") |
|
|
||
| async handleCallback(opts: CallbackParams) { | ||
| try { | ||
| console.log("Linked User iD : <MMMMKIIT") |
There was a problem hiding this comment.
The console.log statement here appears to be a debugging statement. It's recommended to remove it or replace it with this.logger.log if you intend to keep it for logging purposes in production.
- console.log("Linked User iD : <MMMMKIIT")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| console.log("Linked User iD : <MMMMKIIT") |
| } catch (error) { | ||
| handleServiceError(error, this.logger, 'attio', Action.oauthCallback); | ||
|
|
||
| } |
There was a problem hiding this comment.
The opts parameter in the handleTokenRefresh method is defined but never used. Since Attio does not require token refreshes as noted in the comment, you can remove the opts parameter to avoid confusion and align with the method's purpose.
- async handleTokenRefresh(opts: RefreshParams) {
+ async handleTokenRefresh() {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| } | |
| async handleTokenRefresh() { |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts (2 hunks)
Check Runs (1)
Build and Test (18.x) failure (17)
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts: 107-107: 'data' is assigned a value but never used
Additional comments: 1
packages/api/src/@core/connections/crm/services/crm.connection.service.ts (1)
- 49-49: Adding a logging statement here is good for tracking the callback flow, but consider using a more descriptive message that includes dynamic data such as
providerNameorprojectIdfor better debugging and monitoring.
| @@ -57,6 +59,7 @@ | |||
| code: code, | |||
| location: zohoLocation || null, | |||
| }; | |||
| console.log("In callback2") | |||
There was a problem hiding this comment.
Using console.log for logging is not recommended in production code as it lacks the flexibility and control provided by more sophisticated logging frameworks like the one already used in this service (this.logger). Consider replacing console.log with this.logger.log for consistency and better log management.
- console.log("In callback2")
+ this.logger.log("In callback2")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| console.log("In callback2") | |
| this.logger.log("In callback2") |
| } | ||
|
|
||
| const serviceName = providerName.toLowerCase(); | ||
| this.logger.log("callback in connection service - Mit") | ||
|
|
||
| const service = this.serviceRegistry.getService(serviceName); | ||
|
|
||
| if (!service) { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [107-107]
The variable data is assigned a value but never used within the handleCRMTokensRefresh method. If the intention was to log or process this data, ensure that it is utilized appropriately. Otherwise, consider removing the assignment to avoid confusion and potential memory overhead.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
apps/client-ts/public/providers/crm/attio.pngis excluded by:!**/*.png
Files selected for processing (4)
- .npmrc (1 hunks)
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts (1 hunks)
- packages/shared/src/providers.ts (1 hunks)
- packages/shared/src/utils.ts (4 hunks)
Files skipped from review due to trivial changes (1)
- .npmrc
Files skipped from review as they are similar to previous changes (2)
- packages/api/src/@core/connections/crm/services/crm.connection.service.ts
- packages/shared/src/utils.ts
| @@ -7,6 +7,7 @@ export const CRM_PROVIDERS = [ | |||
| 'zendesk', | |||
| 'hubspot', | |||
| 'pipedrive', | |||
| 'attio', | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The usage of CRM_PROVIDERS in packages/api/src/crm/contact/sync/sync.service.ts suggests dynamic handling of CRM providers, potentially allowing for the automatic inclusion of new entries like 'attio'. However, the impact of not explicitly handling 'attio' depends on the filter criteria used. Further investigation into these criteria would be necessary for a definitive assessment.
Analysis chain
The addition of 'attio' to the CRM_PROVIDERS list aligns with the PR objectives and is consistent with the existing structure. Ensure that all system components using CRM_PROVIDERS can handle the new entry without issues.
Given the findings, it's crucial to further investigate how CRM_PROVIDERS is utilized within the listed files to understand the implications of not including 'attio'. Let's examine the usage of CRM_PROVIDERS in one of the files to assess the potential impact.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all occurrences of CRM_PROVIDERS in the codebase properly handle the new 'attio' entry.
rg 'CRM_PROVIDERS' --files-with-matches | xargs rg 'attio' --files-without-match
Length of output: 467
Script:
#!/bin/bash
# Examine how CRM_PROVIDERS is used in one of the affected files.
rg 'CRM_PROVIDERS' packages/api/src/crm/contact/sync/sync.service.ts
Length of output: 183
|
Hello @rflihxyz, This pull request exclusively contains contacts from Attio CRM. Would you like me to also implement it for the Company object? |
Im checking the validity of the code on my side, i'll let you know within 1 hour ! |
|
@mit-27 Could you in the meantime, spot all missing things you had to check by yourself when building this integration ? Thanky ou very much ! |
|
Sure ;) |
btw could you share with me the .env vars for ATTIO ? You can send that over slack in private (send me your address mail so i can invite you on slack) |
It is [email protected] |
Pending Tasks
Summary by CodeRabbit
Summary by CodeRabbit