Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Counting App Users For Billing #1603

Merged
merged 7 commits into from
May 20, 2015
Merged

Counting App Users For Billing #1603

merged 7 commits into from
May 20, 2015

Conversation

bleege
Copy link
Contributor

@bleege bleege commented May 19, 2015

For #1595

@bleege bleege added the iOS Mapbox Maps SDK for iOS label May 19, 2015
@bleege bleege added this to the iOS Beta 2 milestone May 19, 2015
[MGLMapboxEvents sharedManager];
// NOTE: This is (likely) the initial setup of MGLMapboxEvents (via implicit call to +[MGLMapboxEvents sharedManager])
// Turn the Mapbox Turnstile to Count App Users
[MGLMapboxEvents pushEvent:MGLEventTypeAppUserTurnstile withAttributes:nil];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-setAccessToken: can be called more than once. In particular, with #1553, if the developer decides to set the access token programmatically, -setAccessToken: will be called once with nil from within +load and again when the developer gets around to calling +setAccessToken:. You don’t want two turnstile events in that case.

A cleaner approach would be to leave the call to +[MGLMapboxEvents sharedManager] as is and just push a turnstile event at the end of -[MGLMapboxEvents init].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTK about the evolving different ways into the SDK. I just moved it to -[MGLMapboxEvents init] and it works great. Thanks!

if ( ! strongSelf) return;


if ([MGLEventTypeAppUserTurnstile isEqualToString:event]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking my earlier feedback into account. Now that I look at this, I’m thinking it would be cleaner to have a separate, specialized method called -pushTurnstileEvent just for this chunk of code (and all the weak/strong/dispatch scaffolding above). You’d call that method from within -init. That way there are no special cases and no possibility of somehow accidentally turning the turnstile more than once.

Not critical though: I think this PR looks good overall.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like that idea quite a bit. It further separates the Turnstile tick from the stock Events API conceptually as well as gets rid of the "special cases" which tbh wasn't really sitting well with me. 👍

…ucture. Adding event, created, and appBundleId to posted Turnstile Event data model.
bleege added a commit that referenced this pull request May 20, 2015
@bleege bleege merged commit 8f86319 into master May 20, 2015
@1ec5 1ec5 deleted the 1595-internal-billing branch May 20, 2015 20:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants