segment-integrations / analytics.js-integration-intercom

The Intercom analytics.js integration.
https://segment.com/docs/integrations/intercom/
MIT License
7 stars 12 forks source link

Single page app login/logout #6

Open f2prateek opened 8 years ago

f2prateek commented 8 years ago

From https://github.com/segmentio/integration-intercom/issues/6

I'm using segment with intercom on a single page app that allows the user to login/logout without refreshing the page. It doesn't look like this use case was thought through though (or I'm using things incorrectly).

When I call window.analytics.user().reset() to clear the user data for a logout, the standard intercom shutdown method (window.Intercom('shutdown')) isn't called. I've worked around this by intentionally invoking the shutdown method.

However, that's created a bug with logging back in where calling analytics.identify(...) doesn't invoke window.Intercom('boot', ...). I can't as easily manually call this because I don't know where the intercom key is.

Would y'all take a look at supporting this use case?

f2prateek commented 8 years ago

@akrikos I think it's just simply because we don't map the reset method in the integration. Wanna submit a PR? Here's a an example of how we do it for Taplytics https://github.com/segment-integrations/analytics.js-integration-taplytics/blob/a18ca216ff81bf721bbfcdd3bc436afa5d590ff7/lib/index.js#L125

ghost commented 8 years ago

@f2prateek I put together a first attempt at a PR based on the example you provided but something is not quite working. Mind taking a look and pointing me in the right direction?

On a related note, I have noticed that if I manually call Intercom('shutdown') and then call Intercom('boot') again from the console, Intercom sometimes does not reboot. Is there maybe some Intercom behavior here that's interfering with this implementation?

f2prateek commented 8 years ago

@joshuarh Ah I see, I think if you add another line in the reset method: this.booted = false; (opposite of https://github.com/segment-integrations/analytics.js-integration-intercom/blob/master/lib/index.js#L171).

Then when the user logs out, call reset as you are doing now, then page for wherever the user is taken to next. The page will reinitalize the Intercom lib as seen here https://github.com/segment-integrations/analytics.js-integration-intercom/blob/master/lib/index.js#L61

ghost commented 8 years ago

Thanks for the guidance @f2prateek. Something in the shutdown call seems to be removing Intercom from the window. Simply calling api('shutdown') results in the following error:

  Intercom
    ✓ should have the right settings
    1) "after each" hook for "should have the right settings"

  1 passing (7ms)
  1 failing

  1) Intercom "after each" hook for "should have the right settings":
     TypeError: 'undefined' is not an object (evaluating 'window.Intercom.apply')
      at api (http://localhost:49323/build.js:11546)
      at http://localhost:49323/build.js:11523
      at http://localhost:49323/build.js:118
      at callFn (http://localhost:49323/node_modules/mocha/mocha.js:4202)
      at http://localhost:49323/node_modules/mocha/mocha.js:4195
      at next (http://localhost:49323/node_modules/mocha/mocha.js:4555)
      at http://localhost:49323/node_modules/mocha/mocha.js:4559
      at timeslice (http://localhost:49323/node_modules/mocha/mocha.js:12326)
f2prateek commented 8 years ago

ah weird, I wonder what the pattern of instrumenting this without segment is. I've reached out to their support describing the case, hopefully they can shed more light on it.

akrikos commented 8 years ago

Not sure if this will help, but what I had to do in my workaround was:

analytics.identify(userId, user, integrationData)
analytics.group(accountId, account)
window.Intercom('boot', intercomData)
window.Intercom('reattach_activator')
f2prateek commented 8 years ago

@akrikos when did you call reset in your example?

ghost commented 8 years ago

Also throwing into the mix that I am totally new to this codebase, so I could be messing up something in testing.

First step, it looks at though simply overriding Intercom.prototype.reset causes a problem (even with this.booted = false or this.loaded = false):

  1) Intercom loading should load:
     Error: Expected `integration.loaded()` to be false before loading.
      at http://localhost:51185/build.js:10291
      at http://localhost:51185/build.js:10114
      at http://localhost:51185/build.js:153
      at callFnAsync (http://localhost:51185/node_modules/mocha/mocha.js:4234)
      at http://localhost:51185/node_modules/mocha/mocha.js:4177
      at http://localhost:51185/node_modules/mocha/mocha.js:4661
      at http://localhost:51185/node_modules/mocha/mocha.js:4790
      at next (http://localhost:51185/node_modules/mocha/mocha.js:4581)
      at http://localhost:51185/node_modules/mocha/mocha.js:4591
      at next (http://localhost:51185/node_modules/mocha/mocha.js:4523)
      at http://localhost:51185/node_modules/mocha/mocha.js:4559
      at timeslice (http://localhost:51185/node_modules/mocha/mocha.js:12326)
akrikos commented 8 years ago

@f2prateek ah, good catch. I do actually have code in my logout that's custom as well. Here's the code that's called when the user logs out.

if (window.analytics) {
  window.analytics.user().reset()
  if (window.intercom) {
    window.Intercom.('shutdown')
  }
}
akrikos commented 8 years ago

I'll go see if I can figure out what's going on too.

akrikos commented 8 years ago

Yeah, I hit the same wall as @joshuarh. The usual things I'd start doing in this situation aren't feasible with my current knowledge of the code base: I don't know how to:

  1. get a debugger console
  2. get debug output
  3. set up this integration in non-minified form for use in jsbin or plunkr

Is there some documentation you can point me to that explains any of that?

f2prateek commented 8 years ago

@akrikos You can run make test-browser to and use your regular dev tools. I don't think we have a solution for 3 though :(

ghost commented 8 years ago

@akrikos The manual boot and reattach_activator workaround worked for me too, thanks!

tanzim commented 8 years ago

@f2prateek Any news on getting this fixed?

charleshan commented 8 years ago

+1. Seems to me like a privacy issue for me but intercom.io is doing the same thing on their website.

esseb commented 8 years ago

Is there a current recommended workaround for this issue I can do while this issue remains open?

anthonysexton commented 7 years ago

Anyone made any progress with this? We've tried a few versions of the suggestion here but we can't get it to function in any normal way.

ncjones commented 6 years ago

This doesn't just affect single page applications; when a new user hits the login or welcome pages then the intercom messenger shows the previous user session.

ncjones commented 6 years ago

The analytics._integrations property contains Intercom app id that we need to reboot the Intercom messenger:

     analytics.reset()
     analytics.page('Welcome')
     analytics.ready(() => {
       const intercomAppId = analytics._integrations.Intercom.options.appId
       Intercom('shutdown')
       Intercom('boot', { app_id: intercomAppId })
     })
SegmentDestinationsBot commented 4 years ago

Hi @f2prateek, as part of the monorepo migration, this issue has been moved to new issue. Our engineers have been notified and will prioritize and work on it ASAP. Thank you!

For more information, see README.md.