PostHog / posthog-android

PostHog Android SDK
https://posthog.com/docs/libraries/android
MIT License
43 stars 23 forks source link

add referrer url automatically #186

Closed beradeep closed 1 month ago

beradeep commented 1 month ago

:bulb: Motivation and Context

138

:green_heart: How did you test it?

Added tests to include checks for $referrer and $referring_domain properties in case of referrals from apps and websites.

:pencil: Checklist

marandaneto commented 1 month ago

@beradeep whats about if the [Browser.EXTRA_APPLICATION_ID](https://stackoverflow.com/questions/9902225/browser-extra-application-id-do-not-work-in-the-tablet-do-you-know-other-work-a) is provided? wondering if this is possible to test it

beradeep commented 1 month ago

@beradeep whats about if the [Browser.EXTRA_APPLICATION_ID](https://stackoverflow.com/questions/9902225/browser-extra-application-id-do-not-work-in-the-tablet-do-you-know-other-work-a) is provided? wondering if this is possible to test it

I'm curious why do we need this? Is it for another prop like $browser?

marandaneto commented 1 month ago

@beradeep whats about if the [Browser.EXTRA_APPLICATION_ID](https://stackoverflow.com/questions/9902225/browser-extra-application-id-do-not-work-in-the-tablet-do-you-know-other-work-a) is provided? wondering if this is possible to test it

I'm curious why do we need this? Is it for another prop like $browser?

no, I am wondering if the app is opened via the browser, if we need to account for this intent name or not, maybe we should just ignore it, or maybe we should read from it as well since the docs isn't that clear when it's used.

marandaneto commented 1 month ago

happy to approve and merge after checking out this comment, and thanks for the PR o/

marandaneto commented 1 month ago

don't forget about the changelog entry.

beradeep commented 1 month ago

@beradeep whats about if the [Browser.EXTRA_APPLICATION_ID](https://stackoverflow.com/questions/9902225/browser-extra-application-id-do-not-work-in-the-tablet-do-you-know-other-work-a) is provided? wondering if this is possible to test it

I'm curious why do we need this? Is it for another prop like $browser?

no, I am wondering if the app is opened via the browser, if we need to account for this intent name or not, maybe we should just ignore it, or maybe we should read from it as well since the docs isn't that clear when it's used.

i'm still not able to understand this. when the app is opened via a link from the browser, the deep link url and the referrer site does get read and captured from the intent built automatically by the Android system. i think you meant that you want to test this feature. am i right?

marandaneto commented 1 month ago

@beradeep https://developer.android.com/reference/android/provider/Browser#EXTRA_APPLICATION_ID

The name of the extra data when starting the Browser from another application.

The value is a unique identification string that will be used to identify the calling application. The Browser will attempt to reuse the same window each time the application launches the Browser with the same identifier.

Constant Value: "com.android.browser.application_id"

Right now we don't do intent.data.getString(Browser.EXTRA_APPLICATION_ID), do we have to? for this specific use case as described above? that is the question I'd like to answer.

beradeep commented 1 month ago

@beradeep https://developer.android.com/reference/android/provider/Browser#EXTRA_APPLICATION_ID

The name of the extra data when starting the Browser from another application. The value is a unique identification string that will be used to identify the calling application. The Browser will attempt to reuse the same window each time the application launches the Browser with the same identifier. Constant Value: "com.android.browser.application_id"

Right now we don't do intent.data.getString(Browser.EXTRA_APPLICATION_ID), do we have to? for this specific use case as described above? that is the question I'd like to answer.

I don't think we need that here. This is only required when we need to get the name of the application calling the browser. Unless the client app is a browser app, I don't think we need that, do we?