track Mac installations on Posthog #1737
Conversation
pelumy
requested review from
mplorentz,
joshuatbrown and
bryanmontz
as code owners
|
👀 |
Nos/Extensions/Bundle+Current.swift
Outdated
| @@ -33,16 +34,63 @@ extension Bundle { | |||
| /// Checks the app's receipt URL to determine if it contains the TestFlight-specific | |||
| /// "sandboxReceipt" identifier. | |||
| /// - Returns: `true` if the app was installed through TestFlight, `false` otherwise. | |||
| private var isTestFlight: Bool { | |||
| private var isIosTestFlight: Bool { | |||
There was a problem hiding this comment.
Could we maybe have a single isTestFlight and inside of it, check whether we're on iOS or macOS? Essentially combine isIosTestFlight and isMacTestFlight into one?
Additionally, I wonder if we need to check whether it's macOS first? https://gist.github.com/lukaskubanek/cbfcab29c0c93e0e9e0a16ab09586996?permalink_comment_id=4686545#gistcomment-4686545
There was a problem hiding this comment.
Thanks for the feedback @joshuatbrown. I will do the refactoring. I will also make sure to check for macOS first before iOS.
Do you also mean including comments about how to test it in the code?
Currently, to test, we need to merge the code, install the app on the Mac through TestFlight and see if it shows up in the downloads graph on post hog.
There was a problem hiding this comment.
Oh no -- I just mean I'm hoping for a way to test like by setting a breakpoint, but I don't know if that's possible. We may just have to deploy to TestFlight and test that way.
There was a problem hiding this comment.
I don't think we can test with the breakpoint, I was able to test the iOS one after merging.
| private var isTestFlight: Bool { | ||
| #if os(macOS) |
There was a problem hiding this comment.
Ah, I just found this in our NoteUITextViewRepresentable:
if ProcessInfo.processInfo.isiOSAppOnMac {
I think this is the way to detect whether we're running on macOS or not. I think the #if os(macOS) only works if we have a real Mac app, or maybe if we have a Catalyst app running on Mac.
There was a problem hiding this comment.
That is going to be a problem then, because the mac code doesn't work (throws errors) except there is #if os(macOS). It has to actually run on a Mac.
There was a problem hiding this comment.
Ah, right. I'll approve, we can merge and test, then decide next steps if needed.
|
@joshuatbrown , this got registered as iPadOS instead of macOS. |
Issues covered
#130
Description
Previos PR (#1735) tracks only
iOSinstallation. This is a continuation to trackmacOSinstallations too.How to test
Merge and check if events shows up on Posthog after app is downloaded on TestFlight and App Store.