segmentio / analytics-react-native

The hassle-free way to add analytics to your React-Native app.
https://segment.com/docs/sources/mobile/react-native/
MIT License
354 stars 181 forks source link

IDFA isn't updated properly #900

Closed buuuudzik closed 4 months ago

buuuudzik commented 7 months ago

We're using Segment and IdfaPlugin in our project. And recently I've found that IDFA isn't properly updated.

I'm using a default behaviour of this plugin in such a way:

const client = createClient({
    writeKey,
    trackAppLifecycleEvents: true,
    trackDeepLinks: true,
    collectDeviceId: true,
    flushInterval: 5,
    maxBatchSize: 15,
  });

client.add({ plugin: new IdfaPlugin() });

which means that shouldAskPermission is set to true and this.getTrackingStatus() from the constructor will be triggered immediately and this.analytics is undefined so context won't be updated even if the value has been received from the OS: this.analytics?.context.set({ device: { ...idfa } });

I've found that it works with such a patch:

const idfaPlugin = new IdfaPlugin();
  if (!idfaPlugin.analytics) {
    idfaPlugin.analytics = client;
  }
  client.add({ plugin: idfaPlugin });

Questions:

  1. Are you aware about it?
  2. Can you add also updating the IDFA in Segment context after returning from background (because the user could minimise the app and go to the Settings and disable "Allow tracking" for this specific app and return to the app).

Steps to reproduce Allow and cancel tracking permission

Expected behavior IDFA should be added to the events only if the user allowed for tracking and not cancelled this permission. IdfaPlugin should properly update Segment context in every scenarios.

Actual behavior IDFA is not updated properly. When I've had an application with permission granted in the past and then I've cancelled this permission even after fully closing and reopening the app further events still included IDFA.

I've found that this.analytics in this.getTrackingStatus() can be undefined. image

oscb commented 7 months ago

@buuuudzik thanks for this report! You're 100% in the right here, analytics shouldn't be called in the constructor, this should be more similar to the AdvertisingID.

I also like the suggestion to check on Background -> Foreground. We'll add that too as part of this fix.

buuuudzik commented 7 months ago

Thanks @oscb I really appreciate your quick response 👏

buuuudzik commented 4 months ago

@oscb What is the status of this issue? We need to fix it ASAP.

oscb commented 4 months ago

@buuuudzik The fix is released now in v0.7.1

buuuudzik commented 4 months ago

@oscb Great news! Big THANKS!

buuuudzik commented 4 months ago

@oscb I've just checked this version and unfortunately nothing changed. Should I upgrade also Segment to make it works?

buuuudzik commented 4 months ago

@oscb I've upgraded Segment to 2.19.0 and it also didn't help.

buuuudzik commented 4 months ago

I'm creating a segment client in such a way:

const client = createClient({
    writeKey,
    trackAppLifecycleEvents: true,
    trackDeepLinks: true,
    collectDeviceId: true,
    flushInterval: 5,
    maxBatchSize: 15, // 15 x 32kB = 480kB so it's under the batch size limit of 500kb
    // Link to docs about size limits: https://segment.com/docs/connections/sources/catalog/libraries/server/http-api/#batch
  });

  client.add({ plugin: new IdfaPlugin() });
buuuudzik commented 4 months ago

This patch can fix the issue (but it won't detect the change after a device will go background and then foreground):

  const client = createClient({
    writeKey,
    trackAppLifecycleEvents: true,
    trackDeepLinks: true,
    collectDeviceId: true,
    flushInterval: 5,
    maxBatchSize: 15, // 15 x 32kB = 480kB so it's under the batch size limit of 500kb
    // Link to docs about size limits: https://segment.com/docs/connections/sources/catalog/libraries/server/http-api/#batch
  });

  const idfaPlugin = new IdfaPlugin();
  if (!idfaPlugin.analytics) {
    idfaPlugin.analytics = client;
  }
  client.add({ plugin: idfaPlugin });
oscb commented 4 months ago

@buuuudzik my bad, I was mixing this with another IDFA fix we merged recently. Just fixed the problem in this PR will merge and release as soon as it passes tests.

oscb commented 4 months ago

@buuuudzik 0.7.2 should have the fix, let us know if it doesn't work

buuuudzik commented 4 months ago

@oscb It works! Great work! Thanks