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

Pass intercom app id in environment for Grunt build #1685

Open
wants to merge 777 commits into
base: master
Choose a base branch
from

Conversation

tosih
Copy link
Member

@tosih tosih commented Aug 24, 2016

} else {
INTERCOM_APP_ID = 'xs5g95pd'; // test ID
}
INTERCOM_APP_ID = process.env.INTERCOM_APP_ID || 'xs5g95pd';
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no processEnv here. Needs to be something in a JSON file that's generated. Did you e2e this for overriding the appID?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested this locally. Does the e2e cover this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean setting the intercom app id to "foo" for example. and we don't really have any e2e that works, just meant doing manual end to end testing

Copy link
Contributor

Choose a reason for hiding this comment

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

Simply put, serviceEventTracking should not have process.env....

@tosih tosih force-pushed the add_intercom_app_id_as_env branch from 9f0afd8 to 2c47d66 Compare August 24, 2016 22:56
@Nathan219
Copy link
Member

So I tried deploying it to gamma, and the intercom bubble doesn't show up

@tosih tosih force-pushed the add_intercom_app_id_as_env branch from 8aada44 to dcd669d Compare August 25, 2016 22:09
src = "https://widget.intercom.io/widget/xs5g95pd"
)
script(
src = "https://widget.intercom.io/widget/" + intercomAppId
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unsure if this bit will work. Does it?

Copy link
Contributor

@Myztiq Myztiq Sep 8, 2016

Choose a reason for hiding this comment

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

Shouldnt it be

script(src="https://widget.intercom.io/widget/#{locals.intercomAppId}")

Henry Mollman and others added 24 commits September 18, 2016 19:59
SAN-5016 Close Popover when Sidebar is closed
Stop unique events when user clicks buttons.
SAN-5072 Toggling Auto Launch - Changes Guide Level
runnabro and others added 29 commits September 26, 2016 13:13
Error when checking dockerfile paths
 Please enter the commit message for your changes. Lines starting
SAN-5027 Allow user to exit milestone 3
…into SAN-5100-setup-confirmation-modal-style
…al-style

Setup confirmation modal is too high
@Nathan219 Nathan219 reopened this Sep 28, 2016
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.

9 participants