-
Notifications
You must be signed in to change notification settings - Fork 247
chore(intercom): Add request to the update server to determine if Intercom integration is allowed COMPASS-9371 #6982
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
Changes from all commits
e506fcb
d93446c
84a3048
2278f19
85ed755
c2ad054
5868a43
bb0847b
1e07960
3e165e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,14 +36,26 @@ export async function setupIntercom( | |
| app_stage: process.env.NODE_ENV, | ||
| }; | ||
|
|
||
| if (enableFeedbackPanel) { | ||
| async function toggleEnableFeedbackPanel(enableFeedbackPanel: boolean) { | ||
| if (enableFeedbackPanel && (await isIntercomAllowed())) { | ||
| debug('loading intercom script'); | ||
| intercomScript.load(metadata); | ||
| } else { | ||
| debug('unloading intercom script'); | ||
| intercomScript.unload(); | ||
| } | ||
| } | ||
|
|
||
| const shouldLoad = enableFeedbackPanel && (await isIntercomAllowed()); | ||
|
|
||
| if (shouldLoad) { | ||
| // In some environment the network can be firewalled, this is a safeguard to avoid | ||
| // uncaught errors when injecting the script. | ||
| debug('testing intercom availability'); | ||
|
|
||
| const intercomWidgetUrl = buildIntercomScriptUrl(metadata.app_id); | ||
|
|
||
| const response = await window.fetch(intercomWidgetUrl).catch((e) => { | ||
| const response = await fetch(intercomWidgetUrl).catch((e) => { | ||
| debug('fetch failed', e); | ||
| return null; | ||
| }); | ||
|
|
@@ -56,27 +68,82 @@ export async function setupIntercom( | |
| debug('intercom is reachable, proceeding with the setup'); | ||
| } else { | ||
| debug( | ||
| 'not testing intercom connectivity because enableFeedbackPanel == false' | ||
| 'not testing intercom connectivity because enableFeedbackPanel == false || isAllowed == false' | ||
| ); | ||
| } | ||
|
|
||
| const toggleEnableFeedbackPanel = (enableFeedbackPanel: boolean) => { | ||
| if (enableFeedbackPanel) { | ||
| debug('loading intercom script'); | ||
| intercomScript.load(metadata); | ||
| } else { | ||
| debug('unloading intercom script'); | ||
| intercomScript.unload(); | ||
| } | ||
| }; | ||
|
|
||
| toggleEnableFeedbackPanel(!!enableFeedbackPanel); | ||
| try { | ||
| await toggleEnableFeedbackPanel(shouldLoad); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't have a good suggestion from the top of my head, but I did find it a bit mind bending that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a valid concern and I had the response cached in a previous commit. I reverted it because it felt a bit premature, but thinking more about it, I do think it makes sense - this is going to be fetched by every render process and we might as well do it once instead of twice. |
||
| } catch (error) { | ||
| debug('initial toggle failed', { | ||
| error, | ||
| }); | ||
| } | ||
|
|
||
| preferences.onPreferenceValueChanged( | ||
| 'enableFeedbackPanel', | ||
| (enableFeedbackPanel) => { | ||
| debug('enableFeedbackPanel changed'); | ||
| toggleEnableFeedbackPanel(enableFeedbackPanel); | ||
| void toggleEnableFeedbackPanel(enableFeedbackPanel); | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| let isIntercomAllowedPromise: Promise<boolean> | null = null; | ||
|
|
||
| function isIntercomAllowed(): Promise<boolean> { | ||
| if (!isIntercomAllowedPromise) { | ||
| isIntercomAllowedPromise = fetchIntegrations().then( | ||
| ({ intercom }) => intercom, | ||
| (error) => { | ||
| debug( | ||
| 'Failed to fetch intercom integration status, defaulting to false', | ||
| { error } | ||
| ); | ||
| return false; | ||
| } | ||
| ); | ||
| } | ||
| return isIntercomAllowedPromise; | ||
| } | ||
|
|
||
| export function resetIntercomAllowedCache(): void { | ||
| isIntercomAllowedPromise = null; | ||
| } | ||
|
|
||
| /** | ||
| * TODO: Move this to a shared package if we start using it to toggle other integrations. | ||
| */ | ||
| function getAutoUpdateEndpoint() { | ||
| const { HADRON_AUTO_UPDATE_ENDPOINT, HADRON_AUTO_UPDATE_ENDPOINT_OVERRIDE } = | ||
| process.env; | ||
| const result = | ||
| HADRON_AUTO_UPDATE_ENDPOINT_OVERRIDE || HADRON_AUTO_UPDATE_ENDPOINT; | ||
| if (!result) { | ||
| throw new Error( | ||
| 'Expected HADRON_AUTO_UPDATE_ENDPOINT or HADRON_AUTO_UPDATE_ENDPOINT_OVERRIDE to be set' | ||
| ); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| /** | ||
| * Fetches the integrations configuration from the update server. | ||
| * TODO: Move this to a shared package if we start using it to toggle other integrations. | ||
| */ | ||
| async function fetchIntegrations(): Promise<{ intercom: boolean }> { | ||
| const url = `${getAutoUpdateEndpoint()}/api/v2/integrations`; | ||
| debug('requesting integrations status', { url }); | ||
| const response = await fetch(url); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Expected an OK response, got ${response.status} '${response.statusText}'` | ||
| ); | ||
| } | ||
| const result = await response.json(); | ||
| debug('got integrations response', { result }); | ||
| if (typeof result.intercom !== 'boolean') { | ||
| throw new Error(`Expected 'intercom' to be a boolean`); | ||
| } | ||
| return result; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,7 +89,7 @@ export function injectCSP() { | |
| extraAllowed.push('ws://localhost:*'); | ||
| // Used by proxy tests, since Chrome does not like proxying localhost | ||
| // (this does not result in actual outgoing HTTP requests) | ||
| extraAllowed.push('http://compass.mongodb.com/'); | ||
| extraAllowed.push('http://proxy-test-compass.mongodb.com/'); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm changing this to avoid conflicts with the actual compass update server URL called when setting up intercom. |
||
| } | ||
| const cspContent = | ||
| Object.entries(defaultCSP) | ||
|
|
||
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.
Using this makes it possible to restore afterwards.