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

Feature/180 custom visitor ID #181

Merged

Conversation

niksawtschuk
Copy link
Contributor

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.

userDefaults.setValue(newValue, forKey: PiwikUserDefaults.Key.clientID)
userDefaults.synchronize()
}
}
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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

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.
@brototyp brototyp merged commit 752dd81 into matomo-org:develop Sep 11, 2017
@brototyp
Copy link
Member

Thanks @niksawtschuk, for this PR. I would like to add your GitHub handle to the CHANGELOG.md. Are you fine with that?

@niksawtschuk
Copy link
Contributor Author

No problem @brototyp.

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.

None yet

2 participants