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

add referrer url automatically #186

Merged
merged 8 commits into from
Oct 2, 2024

Conversation

beradeep
Copy link
Contributor

@beradeep beradeep commented Oct 1, 2024

💡 Motivation and Context

#138

💚 How did you test it?

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

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@beradeep beradeep requested a review from marandaneto as a code owner October 1, 2024 08:13
@marandaneto
Copy link
Member

@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
Copy link
Contributor Author

beradeep commented Oct 1, 2024

@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
Copy link
Member

@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
Copy link
Member

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

@marandaneto
Copy link
Member

don't forget about the changelog entry.

@beradeep
Copy link
Contributor Author

beradeep commented Oct 1, 2024

@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
Copy link
Member

@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
Copy link
Contributor Author

beradeep commented Oct 1, 2024

@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?

@marandaneto marandaneto merged commit 8e044e4 into PostHog:main Oct 2, 2024
4 checks passed
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.

2 participants