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

feat(intercom): upgrade intercom version from 1.4 to 2.10 #2867

Closed
wants to merge 30 commits into from

Conversation

mihir-4116
Copy link
Contributor

@mihir-4116 mihir-4116 commented Dec 4, 2023

Description of the change

  • This PR encompasses a significant upgrade to our Intercom integration from version 1.4 to 2.10 along with the migration of Intercom to CDK

Version Upgrade Highlights:

  1. Identify call
  • In the previous version (v1.4), user creation or update occurred via a single endpoint.
  • In v2.10, this approach has been deprecated. Now, the identify call follows a new flow:
    1. An intermediate call is made to search for the contact.
    2. If the contact is located, an update user call is made; otherwise, a create user call is initiated.
  1. Track call
  • The track call process remains unchanged across both versions. It continues to add events to the respective user.
  1. Group call
  • In v1.4, the response included two transformed outputs:
    1. Creation or update of the company.
    2. Addition of the user to the company.

  • With v2.10, the flow has been adjusted:
    1. Initially, a contact lookup based on email is performed.
    2. If a contact is found, an additional intermediate call is made to create and update the company. The final
    payload, adding the user to the company, is then returned to the server.
    3. If no contact is found, the second intermediate API call is skipped, and the response for creating or updating the
    company is directly returned to the server.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Fix #1

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers mentioned in a comment
  • Changes have been reviewed by at least one other engineer
  • Issue from task tracker has a link to this pull request

@mihir-4116 mihir-4116 self-assigned this Dec 4, 2023
@mihir-4116 mihir-4116 requested review from a team as code owners December 4, 2023 09:53
Copy link
Contributor

coderabbitai bot commented Dec 4, 2023

Important

Auto Review Skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

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.
  • 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 from git 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.

@mihir-4116 mihir-4116 changed the title feat(intercom): refactor cloud mode feat(INT-177): refactor cloud mode Dec 4, 2023
@mihir-4116 mihir-4116 changed the title feat(INT-177): refactor cloud mode feat(intercom): refactor cloud mode Dec 4, 2023
@mihir-4116 mihir-4116 requested review from sivashanmukh and a team as code owners December 7, 2023 07:39
@mihir-4116 mihir-4116 changed the title feat(intercom): refactor cloud mode feat(intercom): upgrade intercom version from 1.4 to 2.10 Dec 7, 2023
Copy link

codecov bot commented Dec 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b1df337) 87.28% compared to head (c9684ec) 87.16%.
Report is 2 commits behind head on develop.

❗ Current head c9684ec differs from pull request most recent head 561b4ce. Consider uploading reports for the commit 561b4ce to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2867      +/-   ##
===========================================
- Coverage    87.28%   87.16%   -0.12%     
===========================================
  Files          872      859      -13     
  Lines        29500    29227     -273     
  Branches      6865     6817      -48     
===========================================
- Hits         25749    25476     -273     
  Misses        3405     3405              
  Partials       346      346              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mihir-4116 mihir-4116 changed the title feat(intercom): upgrade intercom version from 1.4 to 2.10 feat(INT-177): upgrade intercom version from 1.4 to 2.10 Dec 7, 2023
@mihir-4116 mihir-4116 changed the title feat(INT-177): upgrade intercom version from 1.4 to 2.10 feat(intercom): upgrade intercom version from 1.4 to 2.10 Dec 7, 2023
src/v0/destinations/intercom/deleteUsers.js Outdated Show resolved Hide resolved
@@ -0,0 +1,57 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have some negative test cases as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you check processor tests, i have already added negative tests

Copy link
Contributor

@anantjain45823 anantjain45823 Dec 9, 2023

Choose a reason for hiding this comment

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

For intermediate API call failure there no tests from what I confer

@mihir-4116 mihir-4116 force-pushed the feat.intercom-refactor branch from 6128608 to 1fbdacc Compare December 9, 2023 06:02
let payload = .message.context.mappedToDestination === true ? $.outputs.rEtlPayload : $.outputs.groupEtlPayload;
const contactId = $.outputs.searchContact;
const companyId = await $.createOrUpdateCompany(payload, .destination);
$.assert(companyId, "Unable to add user to company");
Copy link
Member

Choose a reason for hiding this comment

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

We are creating/updating the company here and adding the user will be done from server. So we need to update the error messge.

Suggested change
$.assert(companyId, "Unable to add user to company");
$.assert(companyId, "Unable to create or update company");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

- name: prepareCreateOrUpdateCompanyPayload
condition: $.outputs.messageType === {{$.EventType.GROUP}} && !$.isDefinedAndNotNull($.outputs.searchContact)
template: |
$.assert(.message.groupId, "groupId is required for group call");
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is required because groupId is mapped with company_id and intercom does lookup based on company_id. so it's always recommended to send groupId

Comment on lines 47 to 52
if (apiServer === 'eu') {
return BASE_EU_ENDPOINT;
}
if (apiServer === 'au') {
return BASE_AU_ENDPOINT;
}
Copy link
Member

Choose a reason for hiding this comment

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

Use switch case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

* @param {*} destination
* @returns
*/
const searchContact = async (message, destination) => {
Copy link
Member

Choose a reason for hiding this comment

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

Have we checked the limit of the search API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nesting & Limitations

You can nest these filters in order to get even more granular insights that pinpoint exactly what you need. Example: (1 OR 2) AND (3 OR 4). There are some limitations to the amount of multiple's there can be:

There's a limit of max 2 nested filters
There's a limit of max 15 filters for each AND or OR group

this are their query limits which we are already following

rate limits is same and applicable to all endpoints as per below doc
https://developers.intercom.com/docs/references/rest-api/errors/rate-limiting/

@mihir-4116 mihir-4116 requested a review from ItsSudip December 20, 2023 05:29
@anantjain45823
Copy link
Contributor

I see some extra changes coming in .
@mihir-4116 can you check that once ?

Copy link

sonarqubecloud bot commented Jan 8, 2024

@mihir-4116
Copy link
Contributor Author

closing this pr as it contains too many other changes due to conflicts which is difficult to resolve. created new pr here

@mihir-4116 mihir-4116 closed this Jan 8, 2024
@devops-github-rudderstack devops-github-rudderstack deleted the feat.intercom-refactor branch April 9, 2024 01:24
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.

4 participants