Address code review feedback

duckduckgo · Jul 8, 2024 · 023f695 · 023f695
1 parent 62db286
commit 023f695
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 22 deletions.
diff --git a/DuckDuckGo/RemoteMessaging/ActiveRemoteMessageModel.swift b/DuckDuckGo/RemoteMessaging/ActiveRemoteMessageModel.swift
@@ -96,18 +96,23 @@ final class ActiveRemoteMessageModel: ObservableObject {
         store()?.dismissRemoteMessage(withID: remoteMessage.id)
         self.remoteMessage = nil
 
-        let pixelParameters = ["message": remoteMessage.id]
-        switch action {
-        case .close:
-            PixelKit.fire(GeneralPixel.remoteMessageDismissed, withAdditionalParameters: pixelParameters)
-        case .action:
-            PixelKit.fire(GeneralPixel.remoteMessageActionClicked, withAdditionalParameters: pixelParameters)
-        case .primaryAction:
-            PixelKit.fire(GeneralPixel.remoteMessagePrimaryActionClicked, withAdditionalParameters: pixelParameters)
-        case .secondaryAction:
-            PixelKit.fire(GeneralPixel.remoteMessageSecondaryActionClicked, withAdditionalParameters: pixelParameters)
-        default:
-            break
+        let pixel: GeneralPixel? = {
+            switch action {
+            case .close:
+                return GeneralPixel.remoteMessageDismissed
+            case .action:
+                return GeneralPixel.remoteMessageActionClicked
+            case .primaryAction:
+                return GeneralPixel.remoteMessagePrimaryActionClicked
+            case .secondaryAction:
+                return GeneralPixel.remoteMessageSecondaryActionClicked
+            default:
+                return nil
+            }
+        }()
+
+        if let pixel {
+            PixelKit.fire(pixel, withAdditionalParameters: ["message": remoteMessage.id])
         }
     }
 

diff --git a/DuckDuckGo/RemoteMessaging/RemoteMessagingClient.swift b/DuckDuckGo/RemoteMessaging/RemoteMessagingClient.swift
@@ -111,18 +111,23 @@ final class RemoteMessagingClient: RemoteMessagingProcessing {
         }
     }
 
-    func startRefreshingRemoteMessages() {
-        guard remoteMessagingAvailabilityProvider.isRemoteMessagingAvailable else {
+    /**
+     * Starts a periodical remote messages refresh.
+     *
+     * It checks for the feature flag (via `remoteMessagingAvailabilityProvider`) before starting a timer
+     * and if it finds the feature flag to be disabled, it actually ensures that timer is disabled and
+     * returns early.
+     *
+     * Starting the refresh timer can be forced – used when called from `isRemoteMessagingAvailablePublisher`
+     * event handler, where the new value (true) is emitted but it's not yet available from
+     * `remoteMessagingAvailabilityProvider.isRemoteMessagingAvailable` property getter.
+     */
+    func startRefreshingRemoteMessages(force: Bool = false) {
+        guard force || remoteMessagingAvailabilityProvider.isRemoteMessagingAvailable else {
             stopRefreshingRemoteMessages()
             return
         }
-        /// Put the actual timer start into a separate function to allow it to be called unconditionally from
-        /// `isRemoteMessagingAvailablePublisher` event handler, where the new value (true) is emitted but it's
-        /// not yet available from `remoteMessagingAvailabilityProvider.isRemoteMessagingAvailable` property getter.
-        startMessagesRefreshTimer()
-    }
 
-    private func startMessagesRefreshTimer() {
         scheduledRefreshCancellable = Timer.publish(every: Constants.minimumConfigurationRefreshInterval, on: .main, in: .default)
             .autoconnect()
             .prepend(Date())
@@ -137,12 +142,11 @@ final class RemoteMessagingClient: RemoteMessagingProcessing {
     }
 
     private func subscribeToFeatureFlagChanges() {
-
         featureFlagCancellable = remoteMessagingAvailabilityProvider.isRemoteMessagingAvailablePublisher
             .sink { [weak self] isRemoteMessagingAvailable in
                 if isRemoteMessagingAvailable {
                     self?.initializeDatabaseIfNeeded()
-                    self?.startMessagesRefreshTimer()
+                    self?.startRefreshingRemoteMessages(force: true)
                 } else {
                     self?.stopRefreshingRemoteMessages()
                 }