search

matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
388 stars 162 forks source link

Tracker.trackEvent should default URL to the last viewed #92

Closed derek9gag closed 8 years ago

derek9gag commented 8 years ago

Referring to
https://github.com/piwik/piwik-sdk-android/blob/master/piwik-sdk/src/main/java/org/piwik/sdk/Tracker.java#L321 and https://github.com/piwik/piwik-sdk-android/blob/master/piwik-sdk/src/main/java/org/piwik/sdk/Tracker.java#L686

The events tracked by Tracker.trackEvent() is likely to be defaulted to "http://HOST/", since trackEvent() does not take URL as argument. This is very likely to be wrong, and this behaviour is inconsistent with Javascript and the iOS SDKs.

We should retain the URL of the last screen view, and use it for all subsequent events. This is implemented in iOS SDK (see https://github.com/piwik/piwik-sdk-ios/blob/0ddb5078f5ed4248bcac424639ff89e5b5c30aa7/PiwikTracker/PiwikTracker.m#L485)

d4rken commented 8 years ago

I see what issue you mean. Isn't that an issue with all methods except maybe the trackScreenView ones? We can't use the URL of the last screen as the Tracker instance might be used by multiple threads.

I would suggest to change the method(s) to require an url argument or take a (optional?) TrackMe and then in another PR consider adding some helper class for this.

d4rken commented 8 years ago

@dotsbb You did not start on this yet, right?

dotsbb commented 8 years ago

I didn't start yet. Feel free to go ahead with this one.

d4rken commented 8 years ago

Initial issue fixed by #93 Further SDK API changes in #94 to make this easier for devs in the future.

derek9gag commented 8 years ago

Thank you!