Skip to content

Conversation

@pooyaj
Copy link
Contributor

@pooyaj pooyaj commented Sep 6, 2022

What does this PR do?
Adding an apiBase option for intercom. Defaulting to empty, but if user chose to pick an API region, then we will respect that value when initializing intercom.

Are there breaking changes in this PR?
No breaking change

Testing
Testing completed successfully on local using local integrations tester

Any background context you want to provide?
Intercom is adding regional support for data processing, so we are providing an option to pick region for Intercom customers.

Is there parity with the server-side/android/iOS integration components (if applicable)?
NA

Does this require a new integration setting? If so, please explain how the new setting works
Yes, we need to add apiBase to partner portal. It will be a choice between the following values:

      choices: [
        {
          label: 'US',
          value: 'https://api-iam.intercom.io'
        },
        {
          label: 'EU',
          value: 'https://api-iam.eu.intercom.io'
        },
        {
          label: 'Australia',
          value: 'https://api-iam.au.intercom.io'
        }
      ],

Links to helpful docs and other external resources
NA

@pooyaj pooyaj requested a review from a team September 6, 2022 20:07
Copy link
Contributor

@chrisradek chrisradek left a comment

Choose a reason for hiding this comment

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

Looks great! Had 1 question but it's not a blocker either way.

.option('activator', '#IntercomDefaultWidget')
.option('appId', '')
.option('richLinkProperties', [])
.option('apiBase', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

So Intercom isn't planning to make this required right? Wondering if we should default to the current value (US?) - what does the option look like in the app if you have 3 choices but none selected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually originally added the default (https://api-iam.intercom.io) here, but then thought that's not 100% backwards compatible, because it forces users to use https://api-iam.intercom.io, while the old behavior doesn't specify the API host explicitly. Although I'm not 100% sure if defaulting to https://api-iam.intercom.io would cause any issues, but if somehow some users override the API host, and then we override it again it will be bad.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, makes sense!

@pooyaj pooyaj merged commit 4c7f06d into master Sep 7, 2022
@pooyaj pooyaj deleted the pj/intercom_regional_api branch September 7, 2022 15:54
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