-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Conversation
@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. |
Thank you for following the naming conventions for pull request titles! 🙏 |
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; | ||
} | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- what if the user wants the associated data of an object we don't support?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Screencast.from.15-07-24.03.58.13.PM.IST.webm |
There was a problem hiding this 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.
Description
adds support for fetching associated objects in Hubspot crm get apis.
Fixes #540
associtaions.webm
Type of change
How Has This Been Tested?
via postman
Checklist:
Any dependent changes have been merged and published in downstream modules