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

Mit 27 add attion crm #326

Merged
merged 8 commits into from
Mar 20, 2024
Merged

Mit 27 add attion crm #326

merged 8 commits into from
Mar 20, 2024

Conversation

naelob
Copy link
Contributor

@naelob naelob commented Mar 20, 2024

Summary by CodeRabbit

  • New Features
    • Integrated Attio as a new CRM provider, including authentication, contact synchronization, and mapping functionalities.
    • Updated environment configurations to support Attio integration and Magic Link domain changes.
  • Bug Fixes
    • Adjusted link generation logic in connection components to use the updated Magic Link domain.
  • Refactor
    • Removed Stytch authentication method and associated environment variables.
    • Consolidated domain environment variables for frontend consistency.
    • Enhanced OAuth handling for CRM integrations with new Attio support.
  • Chores
    • Updated Docker configurations and GitHub workflows to reflect new domain and environment variable changes.
  • Documentation
    • Added comments and logging for better clarity and future maintenance, particularly around OAuth and URL construction logic.

Copy link

changeset-bot bot commented Mar 20, 2024

⚠️ No Changeset found

Latest commit: a4553d2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Mar 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
panora-client-ts ❌ Failed (Inspect) Mar 20, 2024 9:06am

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ mit-27
❌ naelob
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

coderabbitai bot commented Mar 20, 2024

Walkthrough

This update signifies a shift in authentication and domain management strategies, notably moving away from Stytch for authentication towards Attio, alongside renaming and aligning domain-related environment variables for clarity. It involves updating environment variables, refining Docker and GitHub workflow configurations, and integrating Attio services into the API for CRM functionalities. The changes streamline authentication, domain handling, and CRM integration, enhancing the system's adaptability and coherence.

Changes

File Path Change Summary
.env.example, apps/.../.env.example, .../docker.yml, .../sh-build-push-docker.yml, .../Dockerfile, .../Dockerfile.dev Updated environment variables, removed STYTCH_PROJECT_ID and STYTCH_SECRET, added ATTIO_CLIENT_ID and ATTIO_CLIENT_SECRET, renamed NEXT_PUBLIC_FRONTEND_DOMAIN to NEXT_PUBLIC_MAGIC_LINK_DOMAIN.
apps/client-ts/src/.../ConnectionTable.tsx, CopyLinkInput.tsx, lib/config.ts Updated domain references from FRONT_DOMAIN to MAGIC_LINK_DOMAIN.
apps/client-ts/src/middleware.ts, packages/api/src/@core/auth/auth.service.ts, .../environment.service.ts Adjusted authentication flow and service configurations.
packages/api/src/@core/connections/crm/..., packages/api/src/crm/contact/... Integrated Attio services and updated CRM functionalities.
packages/api/src/..., packages/shared/src/... Removed Stytch dependencies, added Attio configurations, and updated enums and utility functions.

🐇✨
To the realms of code, where changes bound,
A rabbit hopped, with leaps profound.
From Stytch to Attio, it made its way,
Adjusting domains, by night and day.
🌟 With magic links, it weaved its spell,
In Docker's embrace, the updates dwell.
🚀 To the future, it looks, ever so bright,
With every commit, it aims just right.
✨🐇

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@changesets/[email protected] environment, filesystem, shell Transitive: eval +57 3.99 MB changesets-release-bot
npm/@nestjs/[email protected] None 0 74.3 kB nestjscore
npm/@nestjs/[email protected] Transitive: environment, eval, filesystem, network, shell +64 4.72 MB nestjscore
npm/@nestjs/[email protected] None +1 492 kB nestjscore
npm/@nestjs/[email protected] environment, filesystem 0 49.3 kB nestjscore
npm/@nestjs/[email protected] environment, unsafe Transitive: filesystem, network, shell +8 857 kB nestjscore
npm/@nestjs/[email protected] None 0 51 kB nestjscore
npm/@nestjs/[email protected] None 0 24.4 kB nestjscore
npm/@nestjs/[email protected] None 0 21.2 kB nestjscore
npm/@nestjs/[email protected] network Transitive: environment, eval, filesystem +21 984 kB nestjscore
npm/@nestjs/[email protected] None 0 46.6 kB nestjscore
npm/@nestjs/[email protected] Transitive: environment, eval, filesystem, network +19 2.41 MB nestjscore
npm/@nestjs/[email protected] None +1 2.92 MB nestjscore
npm/@nestjs/[email protected] None 0 34.3 kB nestjscore
npm/@nestjs/[email protected] None 0 202 kB nestjscore
npm/@ntegral/[email protected] None 0 359 kB ntegral
npm/@panora/[email protected] None 0 48.9 kB nael-panora
npm/@panora/[email protected] None 0 665 kB nael-panora
npm/@prisma/[email protected] environment, filesystem, shell 0 4.84 MB prismabot
npm/@sentry/[email protected] environment, filesystem, network, shell, unsafe +4 6.61 MB sentry-bot
npm/@sentry/[email protected] Transitive: network +4 4.72 MB sentry-bot
npm/@stytch/[email protected] network Transitive: environment +2 4.35 MB jbolduc-stytch
npm/@tanstack/[email protected] environment 0 53.3 kB tannerlinsley
npm/@tanstack/[email protected] None 0 103 kB tannerlinsley
npm/@tanstack/[email protected] environment +1 2.85 MB tannerlinsley
npm/@tanstack/[email protected] environment +1 3.77 MB tannerlinsley
npm/@tanstack/[email protected] Transitive: environment +1 4.09 MB tannerlinsley
npm/@types/[email protected] None 0 5.38 kB types
npm/@types/[email protected] Transitive: environment, filesystem +32 1.37 MB types
npm/@types/[email protected] None 0 4 MB types
npm/@types/[email protected] None 0 7.67 kB types
npm/@types/[email protected] None 0 5.41 kB types
npm/@types/[email protected] None 0 33.2 kB types
npm/@types/[email protected] None +3 1.68 MB types
npm/@types/[email protected] None +3 38.6 kB types
npm/@types/[email protected] None 0 6.74 kB types
npm/@typescript-eslint/[email protected] Transitive: environment, filesystem +29 6.63 MB jameshenry
npm/@typescript-eslint/[email protected] Transitive: environment, filesystem +25 6.44 MB jameshenry
npm/@typescript-eslint/[email protected] Transitive: environment, filesystem +18 1.79 MB jameshenry
npm/@vitejs/[email protected] Transitive: environment, filesystem, unsafe +48 10.5 MB vitebot
npm/[email protected] environment +18 64.9 MB chenshuai2144
npm/[email protected] environment Transitive: filesystem +4 2.59 MB ai
npm/[email protected] network Transitive: filesystem +5 1.96 MB jasonsaayman
npm/[email protected] Transitive: environment, filesystem, network, shell +5 407 kB amitosh
npm/[email protected] filesystem, shell Transitive: environment, network +7 1.14 MB manast
npm/[email protected] None 0 776 kB typestack-release-bot
npm/[email protected] None +1 5.2 MB typestack-release-bot
npm/[email protected] None +2 34.1 kB dougwilson
npm/[email protected] None 0 778 B ehsalazar
npm/[email protected] environment, filesystem 0 76 kB motdotla
npm/[email protected] None 0 19.9 kB lydell
npm/[email protected] None 0 13.5 kB arnaud-barre
npm/[email protected] environment, filesystem Transitive: eval, shell, unsafe +45 8.98 MB eslintbot
npm/[email protected] Transitive: environment, eval, filesystem, shell +37 2.74 MB carloscuesta
npm/[email protected] environment, filesystem, shell 0 6.44 kB typicode
npm/[email protected] None 0 30.5 kB benjamn
npm/[email protected] Transitive: environment, eval, filesystem, network, shell, unsafe +165 15.4 MB simenb

🚮 Removed packages: npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@nestjs/[email protected], npm/@prisma/[email protected], npm/@stytch/[email protected], npm/@tanstack/[email protected], npm/@tanstack/[email protected], npm/@tanstack/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/@typescript-eslint/[email protected], npm/@typescript-eslint/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Install scripts npm/[email protected]
Install scripts npm/[email protected]
  • Install script: install
  • Source: node-pre-gyp install --fallback-to-build
Install scripts npm/[email protected]

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 12

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between cedf587 and a4553d2.
Files ignored due to path filters (6)
  • apps/client-ts/public/providers/crm/attio.png is excluded by: !**/*.png
  • docker-compose.dev.yml is excluded by: !**/*.yml
  • docker-compose.source.yml is excluded by: !**/*.yml
  • docker-compose.yml is excluded by: !**/*.yml
  • packages/api/swagger/swagger-spec.json is excluded by: !**/*.json
  • pnpm-lock.yaml is excluded by: !**/*.yaml
Files selected for processing (35)
  • .env.example (3 hunks)
  • .github/workflows/docker.yml (1 hunks)
  • .github/workflows/sh-build-push-docker.yml (2 hunks)
  • apps/client-ts/.env.example (1 hunks)
  • apps/client-ts/Dockerfile (2 hunks)
  • apps/client-ts/src/components/Connection/ConnectionTable.tsx (1 hunks)
  • apps/client-ts/src/components/Connection/CopyLinkInput.tsx (2 hunks)
  • apps/client-ts/src/lib/config.ts (1 hunks)
  • apps/client-ts/src/middleware.ts (1 hunks)
  • apps/magic-link/.env.example (1 hunks)
  • apps/magic-link/Dockerfile (1 hunks)
  • apps/magic-link/Dockerfile.dev (1 hunks)
  • apps/magic-link/src/helpers/config.ts (1 hunks)
  • apps/magic-link/src/lib/ProviderModal.tsx (1 hunks)
  • packages/api/src/@core/auth/auth.controller.ts (1 hunks)
  • packages/api/src/@core/auth/auth.module.ts (2 hunks)
  • packages/api/src/@core/auth/auth.service.ts (3 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 (1 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/app.controller.ts (2 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/api/src/main.ts (1 hunks)
  • packages/shared/src/authUrl.ts (2 hunks)
  • packages/shared/src/enum.ts (2 hunks)
  • packages/shared/src/providers.ts (1 hunks)
  • packages/shared/src/utils.ts (5 hunks)
Files skipped from review due to trivial changes (2)
  • apps/magic-link/Dockerfile.dev
  • packages/api/src/main.ts
Additional comments: 31
apps/client-ts/src/lib/config.ts (1)
  • 3-3: Renaming FRONT_DOMAIN to MAGIC_LINK_DOMAIN aligns with the new domain naming conventions. Please verify that all references to the old FRONT_DOMAIN variable have been updated accordingly.
Verification successful

The renaming of FRONT_DOMAIN to MAGIC_LINK_DOMAIN appears to have been successfully applied across the codebase, with no remaining references to the old variable name found. This aligns with the transition to the new domain naming conventions and authentication flow.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for any remaining references to FRONT_DOMAIN in the codebase.
rg "FRONT_DOMAIN"

Length of output: 17

apps/client-ts/.env.example (1)
  • 2-2: The addition of NEXT_PUBLIC_MAGIC_LINK_DOMAIN and the renaming of NEXT_PUBLIC_FRONTEND_DOMAIN to NEXT_PUBLIC_MAGIC_LINK_DOMAIN are noted. Please ensure that all references to the old NEXT_PUBLIC_FRONTEND_DOMAIN variable have been updated accordingly.
Verification successful

The search for any remaining references to NEXT_PUBLIC_FRONTEND_DOMAIN in the codebase did not produce any results, indicating that all references to this environment variable have been successfully updated or removed as intended.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for any remaining references to NEXT_PUBLIC_FRONTEND_DOMAIN in the codebase.
rg "NEXT_PUBLIC_FRONTEND_DOMAIN"

Length of output: 32

packages/shared/src/enum.ts (1)
  • 18-18: Adding ATTIO to the CrmProviders enum aligns with the integration of Attio CRM into the system.
.github/workflows/docker.yml (1)
  • 21-21: Replacing NEXT_PUBLIC_FRONTEND_DOMAIN with NEXT_PUBLIC_MAGIC_LINK_DOMAIN in the GitHub workflow file aligns with the new domain naming conventions. Please ensure the workflow functions correctly with the new environment variable.
packages/shared/src/providers.ts (1)
  • 10-10: The addition of 'attio' to the CRM_PROVIDERS list is correctly implemented.
packages/api/src/crm/contact/contact.module.ts (1)
  • 37-37: The addition of AttioService to the providers list in ContactModule is correctly implemented.
packages/api/src/@core/connections/crm/crm.connection.module.ts (1)
  • 30-30: The addition of AttioConnectionService to the providers list in CrmConnectionModule is correctly implemented.
apps/magic-link/Dockerfile (1)
  • 27-36: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The removal of VITE_ML_FRONTEND_URL from the Dockerfile aligns with the PR objectives. Ensure to verify the impact on the deployment process.

Verification successful

The removal of VITE_ML_FRONTEND_URL from the Dockerfile does not appear to impact any deployment scripts or documentation, as no references to this environment variable were found in the codebase. This supports the initial assessment that the removal aligns with the PR objectives.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify if the removal of VITE_ML_FRONTEND_URL affects deployment scripts or documentation.
rg "VITE_ML_FRONTEND_URL" --files-with-matches

Length of output: 46

packages/api/src/crm/contact/types/mappingsTypes.ts (1)
  • 36-39: The integration of AttioContactMapper into contactUnificationMapping is correctly implemented and follows the established pattern.
packages/shared/src/authUrl.ts (1)
  • 19-19: Ensure that the return value of findProviderVertical(providerName) is handled correctly. If null is a valid return and signifies a specific condition, this should be clearly documented to avoid confusion.
packages/api/src/@core/environment/environment.service.ts (1)
  • 50-55: The addition of getAttioAuth() method aligns with the PR's objective to integrate Attio CRM. Ensure that the environment variables ATTIO_CLIENT_ID and ATTIO_CLIENT_SECRET are securely managed and properly documented.
.github/workflows/sh-build-push-docker.yml (1)
  • 51-51: The update to use NEXT_PUBLIC_MAGIC_LINK_DOMAIN instead of NEXT_PUBLIC_FRONTEND_DOMAIN is consistent with the PR's goal of updating domain references for the new authentication flow. Ensure that all references to the old domain are updated across the entire codebase to maintain consistency.
packages/api/src/crm/contact/services/attio/types.ts (1)
  • 1-121: The introduction of types for Attio CRM integration, such as AttioContact, NumberValueItem, TextValueItem, etc., is a good practice for type safety and clarity. Ensure that these types are used consistently across the Attio service implementation and that they align with the data structures expected by the Attio API.
apps/client-ts/Dockerfile (1)
  • 39-39: Renaming NEXT_PUBLIC_FRONTEND_DOMAIN to NEXT_PUBLIC_MAGIC_LINK_DOMAIN in the Dockerfile aligns with the PR's objective to update domain references for the new authentication flow. Ensure that this change is reflected in all relevant configuration files and documentation.
packages/api/src/crm/contact/services/attio/index.ts (1)
  • 30-71: The implementation of addContact method in AttioService correctly handles the process of adding a contact to Attio CRM. Ensure that error handling is robust and that sensitive information, such as access tokens, is securely managed throughout the process.
apps/client-ts/src/components/Connection/CopyLinkInput.tsx (2)
  • 18-18: Updating the reference to config.MAGIC_LINK_DOMAIN in the handleCopy function aligns with the PR's objective to update domain references for the new authentication flow. Ensure that all references to the old domain are updated across the entire frontend codebase to maintain consistency.
  • 31-31: The update to use config.MAGIC_LINK_DOMAIN in the defaultValue of the Input component is consistent with the PR's goal. This change ensures that the magic link domain is correctly displayed to the user.
packages/api/src/@core/utils/types/index.ts (4)
  • 34-34: The addition of attio to the domains object looks good and follows the established pattern for CRM provider URLs.
  • 47-47: The addition of attio to the customPropertiesUrls object is consistent with the pattern used for other CRM providers. Ensure the URL is accurate and points to the intended documentation or API endpoint.
  • 72-72: The addition of attio to the CrmProviders enum is appropriate for integrating a new CRM provider.
  • 91-91: Including attio in the CRM_PROVIDERS array is consistent with the addition of a new CRM provider and follows the established pattern.
packages/api/src/crm/contact/services/attio/mappers.ts (3)
  • 20-79: The desunify method implementation for transforming unified contact input to Attio-specific format appears correct. Ensure that error handling for edge cases, such as empty email or phone arrays, is managed appropriately upstream.
  • 82-98: The unify method's implementation for transforming Attio-specific contact output to a unified format is well-structured, correctly handling both single and multiple instances. It demonstrates good code reuse by utilizing mapSingleContactToUnified for individual mappings.
  • 101-134: The mapSingleContactToUnified method accurately maps a single Attio contact output to the unified format, including handling custom field mappings. It operates under the assumption of certain field presences and formats, which is reasonable given the controlled nature of the input.
apps/client-ts/src/components/Connection/ConnectionTable.tsx (1)
  • 115-115: The change to use MAGIC_LINK_DOMAIN instead of FRONT_DOMAIN in the onClick function aligns with the PR's objectives and the new authentication flow. Ensure this change is consistently applied across the application where relevant.
packages/api/src/@core/auth/auth.service.ts (1)
  • 54-54: The changes related to the removal of Stytch for authentication and the direct use of createUser for user registration align with the PR's objectives. Ensure that all references to Stytch are removed throughout the codebase to maintain consistency.
packages/api/src/crm/@utils/@types/index.ts (1)
  • 112-112: The addition of the export statement for Attio service types is appropriate and facilitates the use of Attio-specific types across the CRM module.
packages/shared/src/utils.ts (2)
  • 131-134: The update to the getDescription function is correctly implemented and continues to work generically for any provider, including the newly added 'attio'. The function correctly handles cases where a provider or vertical is not found.
  • 145-151: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [147-157]

The enhancements to the providersArray function are correctly implemented and continue to work generically for any provider, including the newly added 'attio'. The function correctly handles cases where a vertical is not found by returning an empty array.

apps/client-ts/src/middleware.ts (2)
  • 189-189: The change to return NextResponse.next() instead of { props: {} } upon successful JWT authentication is appropriate and aligns with Next.js middleware conventions. This enhances the clarity and correctness of the middleware's behavior.
  • 186-192: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-191]

The file contains several references to Stytch, including imports and usage within the middleware function. Given the PR's objective to integrate Attio CRM and replace Stytch functionalities, could you clarify if these references are expected to remain as part of a transitional phase, or if their presence is an oversight that needs addressing?

Comment on lines +27 to +33
'attio': {
clientId: '86cbbd35-e067-4353-940b-5abcdcb539dd',
scopes: 'record_permission:read',
authBaseUrl: 'https://app.attio.com/authorize',
logoPath: "https://asset.brandfetch.io/idZA7HYRWK/idYZS6Vp_r.png",
description: "Sync & Create contacts, deals, companies, notes, engagements, stages, tasks and users"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of the 'attio' provider to providersConfig is correctly implemented and aligns with the structure of other providers. However, ensure that sensitive information such as clientId is not hardcoded in the source code but instead securely managed, for example, through environment variables.

Comment on lines +15 to +16
console.log("State : ", JSON.stringify({ projectId, linkedUserId, providerName, returnUrl }))
console.log("encodedRedirect URL : ", encodedRedirectUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding logging statements like these can be helpful for debugging but consider the security implications of logging sensitive information such as projectId, linkedUserId, and returnUrl. Ensure that this does not lead to unintentional exposure of PII or other sensitive data.

Comment on lines +38 to +50
if (providerName == 'zoho') {
finalAuth = `${baseUrl}?response_type=code&client_id=${encodeURIComponent(clientId)}&scope=${encodeURIComponent(scopes)}&redirect_uri=${encodedRedirectUrl}&access_type=offline&state=${state}`
console.log(finalAuth);
} else if(providerName == "zendesk"){
} else if (providerName == "zendesk") {
finalAuth = `${baseUrl}?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodedRedirectUrl}&state=${state}`
} else if(providerName == "zendesk_tcg" || providerName=="front") {
} else if (providerName == "attio") {
finalAuth = `${baseUrl}?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodedRedirectUrl}&state=${state}`
} else if (providerName == "zendesk_tcg" || providerName == "front") {
finalAuth = `${baseUrl}?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodedRedirectUrl}&scope=${encodeURIComponent(scopes)}&state=${state}`
}else{
finalAuth = addScope ?
`${baseUrl}?client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodedRedirectUrl}&scope=${encodeURIComponent(scopes)}&state=${state}`
: `${baseUrl}?client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodedRedirectUrl}&state=${state}`;
} else {
finalAuth = addScope ?
`${baseUrl}?client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodedRedirectUrl}&scope=${encodeURIComponent(scopes)}&state=${state}`
: `${baseUrl}?client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodedRedirectUrl}&state=${state}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

The handling of different providers is done through a series of conditionals. Consider refactoring this to a more scalable solution, such as a strategy pattern, to improve maintainability and ease of adding new providers in the future.

}

console.log("Final Authentication : ", finalAuth)
Copy link
Contributor

Choose a reason for hiding this comment

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

Logging the final authentication URL can be useful for debugging but be cautious about logging URLs with sensitive information. Ensure that this does not lead to unintentional exposure of secrets or tokens.

@@ -21,6 +21,11 @@ export interface HubspotOAuthResponse {
access_token: string;
expires_in: number;
}

export interface AttioOAuthResponse {
access_token: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace the comma with a semicolon for consistency with the rest of the file.

-  access_token: string,
+  access_token: string;

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.

Suggested change
access_token: string,
access_token: string;

@@ -46,6 +46,7 @@
}

const serviceName = providerName.toLowerCase();

Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of whitespace does not impact the functionality but ensures better readability. However, there's an unused variable data in the handleCRMTokensRefresh method that should be addressed to clean up the code.

-      const data = await service.handleTokenRefresh(refreshOpts);
+      await service.handleTokenRefresh(refreshOpts);

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.

Suggested change

Comment on lines 13 to 20
DISTRIBUTION=selfhosted
OAUTH_REDIRECT_BASE=http://localhost:3000
SENTRY_DSN=
STYTCH_PROJECT_ID="Temp_STYTCH_PROJECT_ID"
STYTCH_SECRET="Temp_STYTCH_PROJECT_SECRET"
FRONT_END_URL=http://localhost


# ================================================
# Database
# ================================================
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The verification process has revealed that the changes to environment variables, specifically the removal of Stytch-related variables and the addition of Attio-specific ones, could significantly impact the application's configuration, deployment, and authentication mechanisms. The presence of these variables in Docker configurations and application code suggests that careful consideration and potential updates are required to ensure the application functions correctly with the new authentication provider. It is recommended to review and update the application's configuration, deployment scripts, and authentication logic to accommodate the transition from Stytch to Attio.

Analysis chain

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The changes to environment variables in .env.example are correctly implemented and align with the PR objectives. Ensure to verify the impact on the application configuration.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify if the changes to environment variables affect application configuration or documentation.
rg "NEXT_PUBLIC_FRONTEND_DOMAIN|STYTCH_PROJECT_ID" --files-with-matches

Length of output: 211



Script:

#!/bin/bash
# Inspect the usage of NEXT_PUBLIC_FRONTEND_DOMAIN and STYTCH_PROJECT_ID in the mentioned files
cat docker-compose.yml docker-compose.source.yml docker-compose.dev.yml apps/client-ts/Dockerfile apps/client-ts/src/lib/stytch/loadStytch.ts

Length of output: 20393

Comment on lines +31 to +110
async handleCallback(opts: CallbackParams) {
try {
console.log("Linked User iD : <MMMMKIIT")
const { linkedUserId, projectId, code } = opts;
this.logger.log(
'linkeduserid is ' + linkedUserId + ' inside callback attio',
);
const isNotUnique = await this.prisma.connections.findFirst({
where: {
id_linked_user: linkedUserId,
provider_slug: 'attio',
},
});
if (isNotUnique) return;
//reconstruct the redirect URI that was passed in the frontend it must be the same
const REDIRECT_URI = `${this.env.getOAuthRredirectBaseUrl()}/connections/oauth/callback`;
const formData = new URLSearchParams({
grant_type: 'authorization_code',
client_id: this.env.getAttioAuth().CLIENT_ID,
client_secret: this.env.getAttioAuth().CLIENT_SECRET,
redirect_uri: REDIRECT_URI,
code: code,
});

const res = await axios.post(
'https://app.attio.com/oauth/token',
formData.toString(),
{
headers: {
'Content-Type': 'application/x-www-form-urlencoded;charset=utf-8',
},
},
);

const data: AttioOAuthResponse = res.data;

// Saving the token of customer inside db
let db_res;
const connection_token = uuidv4();

if (isNotUnique) {
// Update existing connection
db_res = await this.prisma.connections.update({
where: {
id_connection: isNotUnique.id_connection,
},
data: {
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
},
});
} else {
// Create new connection
db_res = await this.prisma.connections.create({
data: {
id_connection: uuidv4(),
connection_token: connection_token,
provider_slug: 'attio',
token_type: 'oauth',
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
projects: {
connect: { id_project: projectId },
},
linked_users: {
connect: { id_linked_user: linkedUserId },
},
},
});
}
this.logger.log('Successfully added tokens inside DB ' + db_res);
return db_res;


} catch (error) {
handleServiceError(error, this.logger, 'attio', Action.oauthCallback);

}
Copy link
Contributor

Choose a reason for hiding this comment

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

The handleCallback method correctly implements the OAuth callback process for Attio, including token exchange and database operations. Consider replacing console.log with the LoggerService for consistent and configurable logging.

- console.log("Linked User iD : <MMMMKIIT")
+ this.logger.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.

Suggested change
async handleCallback(opts: CallbackParams) {
try {
console.log("Linked User iD : <MMMMKIIT")
const { linkedUserId, projectId, code } = opts;
this.logger.log(
'linkeduserid is ' + linkedUserId + ' inside callback attio',
);
const isNotUnique = await this.prisma.connections.findFirst({
where: {
id_linked_user: linkedUserId,
provider_slug: 'attio',
},
});
if (isNotUnique) return;
//reconstruct the redirect URI that was passed in the frontend it must be the same
const REDIRECT_URI = `${this.env.getOAuthRredirectBaseUrl()}/connections/oauth/callback`;
const formData = new URLSearchParams({
grant_type: 'authorization_code',
client_id: this.env.getAttioAuth().CLIENT_ID,
client_secret: this.env.getAttioAuth().CLIENT_SECRET,
redirect_uri: REDIRECT_URI,
code: code,
});
const res = await axios.post(
'https://app.attio.com/oauth/token',
formData.toString(),
{
headers: {
'Content-Type': 'application/x-www-form-urlencoded;charset=utf-8',
},
},
);
const data: AttioOAuthResponse = res.data;
// Saving the token of customer inside db
let db_res;
const connection_token = uuidv4();
if (isNotUnique) {
// Update existing connection
db_res = await this.prisma.connections.update({
where: {
id_connection: isNotUnique.id_connection,
},
data: {
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
},
});
} else {
// Create new connection
db_res = await this.prisma.connections.create({
data: {
id_connection: uuidv4(),
connection_token: connection_token,
provider_slug: 'attio',
token_type: 'oauth',
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
projects: {
connect: { id_project: projectId },
},
linked_users: {
connect: { id_linked_user: linkedUserId },
},
},
});
}
this.logger.log('Successfully added tokens inside DB ' + db_res);
return db_res;
} catch (error) {
handleServiceError(error, this.logger, 'attio', Action.oauthCallback);
}
async handleCallback(opts: CallbackParams) {
try {
this.logger.log("Linked User iD : <MMMMKIIT")
const { linkedUserId, projectId, code } = opts;
this.logger.log(
'linkeduserid is ' + linkedUserId + ' inside callback attio',
);
const isNotUnique = await this.prisma.connections.findFirst({
where: {
id_linked_user: linkedUserId,
provider_slug: 'attio',
},
});
if (isNotUnique) return;
//reconstruct the redirect URI that was passed in the frontend it must be the same
const REDIRECT_URI = `${this.env.getOAuthRredirectBaseUrl()}/connections/oauth/callback`;
const formData = new URLSearchParams({
grant_type: 'authorization_code',
client_id: this.env.getAttioAuth().CLIENT_ID,
client_secret: this.env.getAttioAuth().CLIENT_SECRET,
redirect_uri: REDIRECT_URI,
code: code,
});
const res = await axios.post(
'https://app.attio.com/oauth/token',
formData.toString(),
{
headers: {
'Content-Type': 'application/x-www-form-urlencoded;charset=utf-8',
},
},
);
const data: AttioOAuthResponse = res.data;
// Saving the token of customer inside db
let db_res;
const connection_token = uuidv4();
if (isNotUnique) {
// Update existing connection
db_res = await this.prisma.connections.update({
where: {
id_connection: isNotUnique.id_connection,
},
data: {
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
},
});
} else {
// Create new connection
db_res = await this.prisma.connections.create({
data: {
id_connection: uuidv4(),
connection_token: connection_token,
provider_slug: 'attio',
token_type: 'oauth',
access_token: this.cryptoService.encrypt(data.access_token),
status: 'valid',
created_at: new Date(),
projects: {
connect: { id_project: projectId },
},
linked_users: {
connect: { id_linked_user: linkedUserId },
},
},
});
}
this.logger.log('Successfully added tokens inside DB ' + db_res);
return db_res;
} catch (error) {
handleServiceError(error, this.logger, 'attio', Action.oauthCallback);
}

Comment on lines +114 to +116
async handleTokenRefresh(opts: RefreshParams) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The handleTokenRefresh method is correctly implemented as a no-op, aligning with Attio's lack of a refresh token mechanism. Consider removing the unused parameter opts to clean up the method signature.

- 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.

Suggested change
async handleTokenRefresh(opts: RefreshParams) {
return;
}
async handleTokenRefresh() {
return;
}

@@ -47,7 +47,7 @@ const ProviderModal = () => {

const { open, isReady } = useOAuth({
providerName: selectedProvider,
returnUrl: config.ML_FRONTEND_URL!,
returnUrl: "https://google.com", //TODO: handle the redirection URL (let customer put their confetti or success page redirect url ? )
Copy link
Contributor

Choose a reason for hiding this comment

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

The change to a hardcoded returnUrl ("https://google.com") in the useOAuth hook is noted. While this may be a temporary measure, consider implementing a more flexible solution for handling redirection URLs to avoid potential issues with hardcoded values. Ensure the TODO comment is addressed in future updates.

@naelob naelob merged commit 5f7f4b4 into main Mar 20, 2024
8 of 14 checks passed
@naelob naelob deleted the mit-27-add-attion-crm branch July 27, 2024 18:26
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.

3 participants