Merge pull request #365 from woocommerce/issue/361-analytics-blogid-fix

Analytics: exclude site information for some tracks events

Author: bummytime

WooCommerce/Classes/Analytics/TracksProvider.swift

@@ -57,11 +57,6 @@ private extension TracksProvider {
         userProperties[UserProperties.platformKey] = "iOS"
         userProperties[UserProperties.voiceOverKey] = UIAccessibility.isVoiceOverRunning
         userProperties[UserProperties.rtlKey] = (UIApplication.shared.userInterfaceLayoutDirection == .rightToLeft)
-        if StoresManager.shared.isAuthenticated {
-            let site = StoresManager.shared.sessionManager.defaultSite
-            userProperties[UserProperties.blogIDKey] = site?.siteID
-            userProperties[UserProperties.wpcomStoreKey] = site?.isWordPressStore
-        }
         tracksService.userProperties.removeAllObjects()
         tracksService.userProperties.addEntries(from: userProperties)
     }
@@ -80,7 +75,5 @@ private extension TracksProvider {
         static let platformKey          = "platform"
         static let voiceOverKey         = "accessibility_voice_over_enabled"
         static let rtlKey               = "is_rtl_language"
-        static let blogIDKey            = "blog_id"
-        static let wpcomStoreKey        = "is_wpcom_store"
     }
 }

WooCommerce/Classes/Analytics/WooAnalytics.swift

@@ -66,8 +66,8 @@ public extension WooAnalytics {
     ///   - properties: a collection of properties related to the event
     ///
     func track(_ stat: WooAnalyticsStat, withProperties properties: [AnyHashable: Any]?) {
-        if let properties = properties {
-            analyticsProvider.track(stat.rawValue, withProperties: properties)
+        if let updatedProperties = updatePropertiesIfNeeded(for: stat, properties: properties) {
+            analyticsProvider.track(stat.rawValue, withProperties: updatedProperties)
         } else {
             analyticsProvider.track(stat.rawValue)
         }
@@ -84,7 +84,8 @@ public extension WooAnalytics {
         let errorDictionary = [Constants.errorKeyCode: "\(err.code)",
                                Constants.errorKeyDomain: err.domain,
                                Constants.errorKeyDescription: err.description]
-        analyticsProvider.track(stat.rawValue, withProperties: errorDictionary)
+        let updatedProperties = updatePropertiesIfNeeded(for: stat, properties: errorDictionary)
+        analyticsProvider.track(stat.rawValue, withProperties: updatedProperties)
     }
 }
 
@@ -114,7 +115,21 @@ private extension WooAnalytics {
         }
 
         let timeInApp = round(Date().timeIntervalSince(applicationOpenedTime))
-        return [Constants.propertyKeyTimeInApp: timeInApp.description]
+        return [PropertyKeys.propertyKeyTimeInApp: timeInApp.description]
+    }
+
+    /// This function appends any additional properties to the provided properties dict if needed.
+    ///
+    func updatePropertiesIfNeeded(for stat: WooAnalyticsStat, properties: [AnyHashable: Any]?) -> [AnyHashable: Any]? {
+        guard stat.shouldSendSiteProperties, StoresManager.shared.isAuthenticated else {
+            return properties
+        }
+
+        var updatedProperties = properties ?? [:]
+        let site = StoresManager.shared.sessionManager.defaultSite
+        updatedProperties[PropertyKeys.blogIDKey] = site?.siteID
+        updatedProperties[PropertyKeys.wpcomStoreKey] = site?.isWordPressStore
+        return updatedProperties
     }
 }
 
@@ -127,7 +142,11 @@ private extension WooAnalytics {
         static let errorKeyCode         = "error_code"
         static let errorKeyDomain       = "error_domain"
         static let errorKeyDescription  = "error_description"
+    }
 
+    enum PropertyKeys {
         static let propertyKeyTimeInApp = "time_in_app"
+        static let blogIDKey            = "blog_id"
+        static let wpcomStoreKey        = "is_wpcom_store"
     }
 }

WooCommerce/Classes/Analytics/WooAnalyticsStat.swift

@@ -4,6 +4,9 @@ import WordPressShared
 /// This enum contains all of the events we track in the app. Please reference the "Woo Mobile Events Draft i2"
 /// spreadsheet for more details.
 ///
+/// Note: If you would like to exclude site properties (e.g. `blog_id`) for a given event, please
+/// add the event to the `WooAnalyticsStat.shouldSendSiteProperties` var.
+///
 public enum WooAnalyticsStat: String {
 
     // Application Events
@@ -124,6 +127,35 @@ public enum WooAnalyticsStat: String {
 
 public extension WooAnalyticsStat {
 
+
+    /// Indicates if site information should be included with this event when it's sent to the tracks server.
+    /// Returns `true` if it should be included, `false` otherwise.
+    ///
+    /// Note: Currently all application-level and authentication events will return false. If you wish
+    /// to include additional no-site-info events, please add them here.
+    ///
+    var shouldSendSiteProperties: Bool {
+        switch self {
+        // Application events
+        case .applicationClosed, .applicationOpened, .applicationUpgraded, .applicationInstalled:
+            return false
+        // Authentication Events
+        case .signedIn, .logout, .openedLogin, .loginFailed,
+             .loginAutoFillCredentialsFilled, .loginAutoFillCredentialsUpdated, .loginEmailFormViewed, .loginMagicLinkOpenEmailClientViewed,
+             .loginMagicLinkRequestFormViewed, .loginMagicLinkExited, .loginMagicLinkFailed, .loginMagicLinkOpened,
+             .loginMagicLinkRequested, .loginMagicLinkSucceeded, .loginPasswordFormViewed, .loginURLFormViewed,
+             .loginURLHelpScreenViewed, .loginUsernamePasswordFormViewed, .loginTwoFactorFormViewed, .loginEpilogueViewed,
+             .loginEpilogueStoresShown, .loginEpilogueContinueTapped, .loginProloguePaged, .loginPrologueViewed,
+             .loginPrologueContinueTapped, .loginPrologueJetpackInstructions, .loginForgotPasswordClicked, .loginSocialButtonClick,
+             .loginSocialButtonFailure, .loginSocialConnectSuccess, .loginSocialConnectFailure, .loginSocialSuccess,
+             .loginSocialFailure, .loginSocial2faNeeded, .loginSocialAccountsNeedConnecting, .loginSocialErrorUnknownUser,
+             .onePasswordFailed, .onePasswordLogin, .onePasswordSignup, .twoFactorCodeRequested, .twoFactorSentSMS:
+            return false
+        default:
+            return true
+        }
+    }
+
     /// Converts the provided WPAnalyticsStat into a WooAnalyticsStat.
     /// This whole process kinda stinks, but we need this for the `WordPressAuthenticatorDelegate`
     /// implementation. ☹️ Feel free to refactor later on!