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: Support for Hubspot associations API #595

Merged
merged 7 commits into from
Jul 30, 2024

Conversation

manish-singh-bisht
Copy link
Contributor

@manish-singh-bisht manish-singh-bisht commented Jul 14, 2024

Description

adds support for fetching associated objects in Hubspot crm get apis.

Fixes #540

associtaions.webm

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

via postman

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Any dependent changes have been merged and published in downstream modules

Copy link

vercel bot commented Jul 14, 2024

@manish-singh-bisht is attempting to deploy a commit to the OpenInt Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

github-actions bot commented Jul 14, 2024

Thank you for following the naming conventions for pull request titles! 🙏

@manish-singh-bisht manish-singh-bisht changed the title feat: Support for Hubspot associations API #540 feat: Support for Hubspot associations API Jul 14, 2024
Comment on lines 177 to 199
associatedData[objectType] =
associatedObjectType && associatedObjectType !== null
? await Promise.all(
fullBatchData.map(
async (item: any) =>
await unifyObject<any, any>({
obj: {
...item,
...item?.properties,
},
tpId: thirdPartyId,
objType: associatedObjectType,
tenantSchemaMappingId: connection.schema_mapping_id,
accountFieldMappingConfig: account.accountFieldMappingConfig,
})
)
)
: fullBatchData;
}
}
}
}

Copy link
Member

@Nabhag8848 Nabhag8848 Jul 15, 2024

Choose a reason for hiding this comment

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

@manish-singh-bisht currently it looks like if objectType isn't supported by us, we are returning as it is without unifiying. instead what we want is in the response, we want the warning but api call should not fail for rest, ie: something like an error object which includes what all associations type which we don't support, have an message for those if you want support contact us.

also what happens when for eg: we pass associations= or associations=undefined or something similar, etc, are we handling those cases ?

Copy link
Member

Choose a reason for hiding this comment

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

@manish-singh-bisht makes sense user won't use but what if they pass associations= or anything similar, that will make a call to hubspot right ? + also can you check what hubspot api responds with when we pass anything random or something similar that might crash things, ain't it ?

Copy link
Contributor Author

@manish-singh-bisht manish-singh-bisht Jul 15, 2024

Choose a reason for hiding this comment

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

  1. what if the user wants the associated data of an object we don't support?

Copy link
Member

@Nabhag8848 Nabhag8848 Jul 15, 2024

Choose a reason for hiding this comment

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

@manish-singh-bisht as suggested by jatin, we should give warning about it that we don't support this object in response somewhere, note we shouldn't fail the call + don't make api call to hubspot for that association and have the message something like we don't support this, contact us.

Copy link
Contributor Author

@manish-singh-bisht manish-singh-bisht Jul 15, 2024

Choose a reason for hiding this comment

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

  1. it will work for associatons= ,but for associations=undefined it will use "undefined" as the the value of associations parameter resulting in a error as there is nothing like "undefined" in hubspot.

Copy link
Contributor Author

@manish-singh-bisht manish-singh-bisht Jul 15, 2024

Choose a reason for hiding this comment

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

@manish-singh-bisht makes sense user won't use but what if they pass associations= or anything similar, that will make a call to hubspot right ? + also can you check what hubspot api responds with when we pass anything random or something similar that might crash things, ain't it ?

when we pass something to associations that is not recognized by hubspot it will throw error, the user will see "internal server error" from our side.

Copy link
Member

Choose a reason for hiding this comment

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

@manish-singh-bisht cool, so inorder to not crash (500 error). don't make the call if there is undefined/null/or empty string for that association to hubspot itself + in response let users know someway that,
no such association exist in hubspot or if that exist and we are not supporting contact us

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Nabhag8848 now no calls will be there to hubspot other than the objects we support, for every other object it will send an error message (this will not stop the flow of the call for the other supported objects).

undefined and null will not crash.

empty too will not throw error

@manish-singh-bisht
Copy link
Contributor Author

Screencast.from.15-07-24.03.58.13.PM.IST.webm

Copy link
Member

@Nabhag8848 Nabhag8848 left a comment

Choose a reason for hiding this comment

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

LGTM ! @manish-singh-bisht also i see you use any type, fine for now. you can checkout https://www.totaltypescript.com/tutorials and https://typehero.dev/ for writing better types and remove any type from your fingertips and enjoy the beauty of typescript.

@Nabhag8848 Nabhag8848 merged commit d4cdc8c into revertinc:main Jul 30, 2024
1 of 3 checks passed
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.

[ISSUE-REVERT]: Support for Hubspot associations API
2 participants