🎉 Source Intercom: Update Intercom API to version 2.5 #15681
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unit Tests: |
alex-gron
changed the title
|
/test connector=connectors/source-intercom
Build FailedTest summary info:
Build FailedTest summary info: |
| @@ -119,7 +119,9 @@ | |||
| } | |||
| } | |||
| }, | |||
| "priority": { | |||
| "custom_attributes": { | |||
| "type": ["null", "object"] | |||
There was a problem hiding this comment.
Would we want to optionally specify in the connector spec (configuration) the custom JSON Schema for that field though? If left empty, it could default to object.
But if the user is able to describe what kind of data is being recorded there in the source API, then normalization would be able to handle the extra data in a more well-structured manner...
Is that worth exposing to the connector configuration?
There was a problem hiding this comment.
The ultimate goal is to bring in in all of the custom_attributes that are configured in Intercom (similar to how the Salesforce connector works to bring in all custom fields).
The data is stored as "key:value" in Intercom (source), so I don't think we need to allow for JSON in the config since it is relatively simple.
Let me know if I am misunderstanding here though!
There was a problem hiding this comment.
Is it possible for you to share an example of a record from intercom that includes data in the "custom_attributes" column?
Here in the test / expected results from this PR, it's left empty:
"custom_attributes": {},
So it's not super insightful on how it'd be presented and used by users afterwards
There was a problem hiding this comment.
Yes - we don't have any custom attributes created on our Test Intercom account, so that is why they are empty in the example and integration test.
Here is an example result from the prod Intercom account:
"custom_attributes":{"Needed Escalation":false,"Follow Up Required":false,"Chat Closed Reason":"Alpha Connector Issue"}
There was a problem hiding this comment.
It ends up in the database as a json column where you can extract the keys/values as follows:
select
json_extract_scalar(custom_attributes, "$.Needed Escalation") as needed_escalation,
json_extract_scalar(custom_attributes, "$.Follow Up Required") as follow_up_required,
json_extract_scalar(custom_attributes, "$.Chat Closed Reason") as chat_closed_reason
from conversations
There was a problem hiding this comment.
yes, so when running the discover command, if the catalog would be able to contain:
"custom_attributes": {
"type": ["null", "object"]
"properties": {
"Needed Escalation": {
"type": ["null", "string"]
},
"Follow Up Required": {
"type": ["null", "string"]
},
"Chat Closed Reason": {
"type": ["null", "string"]
}
}
}
then normalization would be able to generate the json_extract lines that you linked automatically too
Of course, just having the json column custom_attributes is already a good move forward already!
There was a problem hiding this comment.
The logic for custom_attributes should be applied, we need to nest all keys into "key" and all the value into "value", than modify the json_schema with this change and it will work correctly for all possible key:value pairs custom_attributes holds.
We can do this now or later on, if we move with the change like in this PR - it's totally ok, but there is a right way to do it, so probably we will make it in the future updates.
|
/test connector=connectors/source-intercom
Build PassedTest summary info: |
ChristopheDuong
approved these changes
Aug 18, 2022
bazarnov
approved these changes
Aug 18, 2022
github-actions
bot
added
the
area/documentation
|
/publish connector=connectors/source-intercom
if you have connectors that successfully published but failed definition generation, follow step 4 here |
alex-gron
changed the title
rodireich
pushed a commit
that referenced
this pull request
Aug 25, 2022
* upgrade intercom API to v2.5 * update conversations schema * update expected records * update change log * fix doc formatting * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Upgrade Intercom API from v2.2 to v2.5
Primary use case is to pull in
custom_attributeson the Conversation objectCloses #15141
How
Increment API version to 2.5
Update schema.json to include
custom_attributes🚨 User Impact 🚨
Backwards compatible with earlier API versions
Pre-merge Checklist
Expand the relevant checklist and delete the others.
Updating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereTests
Unit
See below
Integration
See below
Acceptance
See below