-
Notifications
You must be signed in to change notification settings - Fork 166
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
Feature/180 custom visitor ID #181
Feature/180 custom visitor ID #181
Conversation
… current Visitor object.
… the User Defaults and also update the current visitor for the shared Tracker.
userDefaults.setValue(newValue, forKey: PiwikUserDefaults.Key.clientID) | ||
userDefaults.synchronize() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you change it this way, it will change the behavior of all old implementations. For all old Implementations the clientId
was saved under the key PiwikUserDefaults.Key.visitorID
. After this change, all existing installations will use the old clientId
as their new visitorId
. I thinks that's an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've adjusted this in a9e9bf3. I left my changes for using the correct "key name", but changing what value the key is using so it's using the old one. This should retain the existing functionality but be more clear in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that's the best solution in this case. It's a bit of a pity that it got introduced in the first place. Thanks for changing it this way.
import Foundation | ||
import PiwikTracker | ||
|
||
class UserIDViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks for adding this to the Example Application!
…his keeps compatibility with old versions of the SDK. Rename properties to be more specific about what the visitorID is used for.
Thanks @niksawtschuk, for this PR. I would like to add your GitHub handle to the CHANGELOG.md. Are you fine with that? |
No problem @brototyp. |
Small changes here to add a public variable which allows apps to set the custom user ID for the shared tracker's current visitor object. I used the existing structure that was defined already, so this PR is really just connecting the plumbing and enhancing the docs/example app.