From dd3e7f2d38280bc8a86e8f5e2f05240c0588709e Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 19 May 2024 16:44:24 -0700 Subject: [PATCH 01/11] Add `pproEligible` and `pproSubscriber`. --- .../JsonToRemoteMessageModelMapper.swift | 6 +- .../Matchers/UserAttributeMatcher.swift | 29 +++++--- .../Model/MatchingAttributes.swift | 67 +++++++++++++------ .../JsonToRemoteConfigModelMapperTests.swift | 16 ++++- .../Matchers/UserAttributeMatcherTests.swift | 33 ++++++--- .../RemoteMessagingConfigMatcherTests.swift | 40 ++++++----- .../RemoteMessagingConfigProcessorTests.swift | 10 +-- .../Resources/remote-messaging-config.json | 9 ++- 8 files changed, 140 insertions(+), 70 deletions(-) diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index 88af0abce..3a8c81eaf 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -36,8 +36,9 @@ private enum AttributesKey: String, CaseIterable { case favorites case appTheme case daysSinceInstalled - case isNetPWaitlistUser case daysSinceNetPEnabled + case pproEligible + case pproSubscriber func matchingAttribute(jsonMatchingAttribute: AnyDecodable) -> MatchingAttribute { switch self { @@ -56,8 +57,9 @@ private enum AttributesKey: String, CaseIterable { case .favorites: return FavoritesMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) case .appTheme: return AppThemeMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) case .daysSinceInstalled: return DaysSinceInstalledMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) - case .isNetPWaitlistUser: return IsNetPWaitlistUserMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) case .daysSinceNetPEnabled: return DaysSinceNetPEnabledMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) + case .pproEligible: return IsPrivacyProEligibleUserMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) + case .pproSubscriber: return IsPrivacyProSubscriberUserMatchingAttribute(jsonMatchingAttribute: jsonMatchingAttribute) } } } diff --git a/Sources/RemoteMessaging/Matchers/UserAttributeMatcher.swift b/Sources/RemoteMessaging/Matchers/UserAttributeMatcher.swift index a4043560f..f80f1d4ae 100644 --- a/Sources/RemoteMessaging/Matchers/UserAttributeMatcher.swift +++ b/Sources/RemoteMessaging/Matchers/UserAttributeMatcher.swift @@ -29,8 +29,9 @@ public struct UserAttributeMatcher: AttributeMatcher { private let bookmarksCount: Int private let favoritesCount: Int private let isWidgetInstalled: Bool - private let isNetPWaitlistUser: Bool private let daysSinceNetPEnabled: Int + private let isPrivacyProEligibleUser: Bool + private let isPrivacyProSubscriber: Bool public init(statisticsStore: StatisticsStore, variantManager: VariantManager, @@ -39,8 +40,9 @@ public struct UserAttributeMatcher: AttributeMatcher { favoritesCount: Int, appTheme: String, isWidgetInstalled: Bool, - isNetPWaitlistUser: Bool, - daysSinceNetPEnabled: Int + daysSinceNetPEnabled: Int, + isPrivacyProEligibleUser: Bool, + isPrivacyProSubscriber: Bool ) { self.statisticsStore = statisticsStore self.variantManager = variantManager @@ -49,8 +51,9 @@ public struct UserAttributeMatcher: AttributeMatcher { self.bookmarksCount = bookmarksCount self.favoritesCount = favoritesCount self.isWidgetInstalled = isWidgetInstalled - self.isNetPWaitlistUser = isNetPWaitlistUser self.daysSinceNetPEnabled = daysSinceNetPEnabled + self.isPrivacyProEligibleUser = isPrivacyProEligibleUser + self.isPrivacyProSubscriber = isPrivacyProSubscriber } // swiftlint:disable:next cyclomatic_complexity function_body_length @@ -97,18 +100,24 @@ public struct UserAttributeMatcher: AttributeMatcher { } return BooleanMatchingAttribute(value).matches(value: isWidgetInstalled) - case let matchingAttribute as IsNetPWaitlistUserMatchingAttribute: - guard let value = matchingAttribute.value else { - return .fail - } - - return BooleanMatchingAttribute(value).matches(value: isNetPWaitlistUser) case let matchingAttribute as DaysSinceNetPEnabledMatchingAttribute: if matchingAttribute.value != MatchingAttributeDefaults.intDefaultValue { return IntMatchingAttribute(matchingAttribute.value).matches(value: daysSinceNetPEnabled) } else { return RangeIntMatchingAttribute(min: matchingAttribute.min, max: matchingAttribute.max).matches(value: daysSinceNetPEnabled) } + case let matchingAttribute as IsPrivacyProEligibleUserMatchingAttribute: + guard let value = matchingAttribute.value else { + return .fail + } + + return BooleanMatchingAttribute(value).matches(value: isPrivacyProEligibleUser) + case let matchingAttribute as IsPrivacyProSubscriberUserMatchingAttribute: + guard let value = matchingAttribute.value else { + return .fail + } + + return BooleanMatchingAttribute(value).matches(value: isPrivacyProSubscriber) default: assertionFailure("Could not find matching attribute") return nil diff --git a/Sources/RemoteMessaging/Model/MatchingAttributes.swift b/Sources/RemoteMessaging/Model/MatchingAttributes.swift index 7f6551c18..7fc275706 100644 --- a/Sources/RemoteMessaging/Model/MatchingAttributes.swift +++ b/Sources/RemoteMessaging/Model/MatchingAttributes.swift @@ -608,7 +608,45 @@ struct RangeStringNumericMatchingAttribute: Equatable { } } -struct IsNetPWaitlistUserMatchingAttribute: MatchingAttribute, Equatable { +struct DaysSinceNetPEnabledMatchingAttribute: MatchingAttribute, Equatable { + var min: Int = MatchingAttributeDefaults.intDefaultValue + var max: Int = MatchingAttributeDefaults.intDefaultMaxValue + var value: Int = MatchingAttributeDefaults.intDefaultValue + var fallback: Bool? + + init(jsonMatchingAttribute: AnyDecodable) { + guard let jsonMatchingAttribute = jsonMatchingAttribute.value as? [String: Any] else { return } + + if let min = jsonMatchingAttribute[RuleAttributes.min] as? Int { + self.min = min + } + if let max = jsonMatchingAttribute[RuleAttributes.max] as? Int { + self.max = max + } + if let value = jsonMatchingAttribute[RuleAttributes.value] as? Int { + self.value = value + } + if let fallback = jsonMatchingAttribute[RuleAttributes.fallback] as? Bool { + self.fallback = fallback + } + } + + init(min: Int = MatchingAttributeDefaults.intDefaultValue, + max: Int = MatchingAttributeDefaults.intDefaultMaxValue, + value: Int = MatchingAttributeDefaults.intDefaultValue, + fallback: Bool?) { + self.min = min + self.max = max + self.value = value + self.fallback = fallback + } + + static func == (lhs: DaysSinceNetPEnabledMatchingAttribute, rhs: DaysSinceNetPEnabledMatchingAttribute) -> Bool { + return lhs.min == rhs.min && lhs.max == rhs.max && lhs.value == rhs.value && lhs.fallback == rhs.fallback + } +} + +struct IsPrivacyProEligibleUserMatchingAttribute: MatchingAttribute, Equatable { var value: Bool? var fallback: Bool? @@ -628,27 +666,19 @@ struct IsNetPWaitlistUserMatchingAttribute: MatchingAttribute, Equatable { self.fallback = fallback } - static func == (lhs: IsNetPWaitlistUserMatchingAttribute, rhs: IsNetPWaitlistUserMatchingAttribute) -> Bool { + static func == (lhs: IsPrivacyProEligibleUserMatchingAttribute, rhs: IsPrivacyProEligibleUserMatchingAttribute) -> Bool { return lhs.value == rhs.value && lhs.fallback == rhs.fallback } } -struct DaysSinceNetPEnabledMatchingAttribute: MatchingAttribute, Equatable { - var min: Int = MatchingAttributeDefaults.intDefaultValue - var max: Int = MatchingAttributeDefaults.intDefaultMaxValue - var value: Int = MatchingAttributeDefaults.intDefaultValue +struct IsPrivacyProSubscriberUserMatchingAttribute: MatchingAttribute, Equatable { + var value: Bool? var fallback: Bool? init(jsonMatchingAttribute: AnyDecodable) { guard let jsonMatchingAttribute = jsonMatchingAttribute.value as? [String: Any] else { return } - if let min = jsonMatchingAttribute[RuleAttributes.min] as? Int { - self.min = min - } - if let max = jsonMatchingAttribute[RuleAttributes.max] as? Int { - self.max = max - } - if let value = jsonMatchingAttribute[RuleAttributes.value] as? Int { + if let value = jsonMatchingAttribute[RuleAttributes.value] as? Bool { self.value = value } if let fallback = jsonMatchingAttribute[RuleAttributes.fallback] as? Bool { @@ -656,18 +686,13 @@ struct DaysSinceNetPEnabledMatchingAttribute: MatchingAttribute, Equatable { } } - init(min: Int = MatchingAttributeDefaults.intDefaultValue, - max: Int = MatchingAttributeDefaults.intDefaultMaxValue, - value: Int = MatchingAttributeDefaults.intDefaultValue, - fallback: Bool?) { - self.min = min - self.max = max + init(value: Bool?, fallback: Bool?) { self.value = value self.fallback = fallback } - static func == (lhs: DaysSinceNetPEnabledMatchingAttribute, rhs: DaysSinceNetPEnabledMatchingAttribute) -> Bool { - return lhs.min == rhs.min && lhs.max == rhs.max && lhs.value == rhs.value && lhs.fallback == rhs.fallback + static func == (lhs: IsPrivacyProSubscriberUserMatchingAttribute, rhs: IsPrivacyProSubscriberUserMatchingAttribute) -> Bool { + return lhs.value == rhs.value && lhs.fallback == rhs.fallback } } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 4cf9b2bc7..11e2b359d 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -131,14 +131,24 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let rule8 = config.rules.filter { $0.id == 8 }.first XCTAssertNotNil(rule8) XCTAssertNil(rule8?.targetPercentile) - XCTAssertTrue(rule8?.attributes.count == 2) + XCTAssertTrue(rule8?.attributes.count == 3) attribs = rule8?.attributes.filter { $0 is DaysSinceNetPEnabledMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? DaysSinceNetPEnabledMatchingAttribute, DaysSinceNetPEnabledMatchingAttribute(min: 5, fallback: nil)) - attribs = rule8?.attributes.filter { $0 is IsNetPWaitlistUserMatchingAttribute } + attribs = rule8?.attributes.filter { $0 is IsPrivacyProEligibleUserMatchingAttribute } XCTAssertEqual(attribs?.count, 1) - XCTAssertEqual(attribs?.first as? IsNetPWaitlistUserMatchingAttribute, IsNetPWaitlistUserMatchingAttribute(value: true, fallback: nil)) + XCTAssertEqual( + attribs?.first as? IsPrivacyProEligibleUserMatchingAttribute, + IsPrivacyProEligibleUserMatchingAttribute(value: true, fallback: nil) + ) + + attribs = rule8?.attributes.filter { $0 is IsPrivacyProSubscriberUserMatchingAttribute } + XCTAssertEqual(attribs?.count, 1) + XCTAssertEqual( + attribs?.first as? IsPrivacyProSubscriberUserMatchingAttribute, + IsPrivacyProSubscriberUserMatchingAttribute(value: true, fallback: nil) + ) let rule9 = config.rules.filter { $0.id == 9 }.first XCTAssertNotNil(rule9) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift index d8b63f122..0a9988f98 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift @@ -53,8 +53,9 @@ class UserAttributeMatcherTests: XCTestCase { favoritesCount: 88, appTheme: "default", isWidgetInstalled: true, - isNetPWaitlistUser: true, - daysSinceNetPEnabled: 3) + daysSinceNetPEnabled: 3, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false) } override func tearDownWithError() throws { @@ -205,25 +206,35 @@ class UserAttributeMatcherTests: XCTestCase { .fail) } - // MARK: - Network Protection Waitlist + // MARK: - Privacy Pro - func testWhenIsNetPWaitlistUserMatchesThenReturnMatch() throws { - XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsNetPWaitlistUserMatchingAttribute(value: true, fallback: nil)), + func testWhenDaysSinceNetPEnabledMatchesThenReturnMatch() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: DaysSinceNetPEnabledMatchingAttribute(min: 1, fallback: nil)), .match) } - func testWhenIsNetPWaitlistUserDoesNotMatchThenReturnFail() throws { - XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsNetPWaitlistUserMatchingAttribute(value: false, fallback: nil)), + func testWhenDaysSinceNetPEnabledDoesNotMatchThenReturnFail() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: DaysSinceNetPEnabledMatchingAttribute(min: 7, fallback: nil)), .fail) } - func testWhenDaysSinceNetPEnabledMatchesThenReturnMatch() throws { - XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: DaysSinceNetPEnabledMatchingAttribute(min: 1, fallback: nil)), + func testWhenIsPrivacyProEligibleUserMatchesThenReturnMatch() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsPrivacyProEligibleUserMatchingAttribute(value: true, fallback: nil)), .match) } - func testWhenDaysSinceNetPEnabledDoesNotMatchThenReturnFail() throws { - XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: DaysSinceNetPEnabledMatchingAttribute(min: 7, fallback: nil)), + func testWhenIsPrivacyProEligibleUserDoesNotMatchThenReturnFail() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsPrivacyProEligibleUserMatchingAttribute(value: false, fallback: nil)), + .fail) + } + + func testWhenIsPrivacyProSubscriberMatchesThenReturnMatch() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsPrivacyProSubscriberUserMatchingAttribute(value: true, fallback: nil)), + .match) + } + + func testWhenIsPrivacyProSubscriberDoesNotMatchThenReturnFail() throws { + XCTAssertEqual(userAttributeMatcher.evaluate(matchingAttribute: IsPrivacyProSubscriberUserMatchingAttribute(value: false, fallback: nil)), .fail) } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift index 8d904ed91..487a6cf1d 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift @@ -43,8 +43,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: [] ) @@ -127,8 +128,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) @@ -221,8 +223,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: ["1"]) @@ -258,8 +261,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) @@ -291,8 +295,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: percentileStore, dismissedMessageIds: []) @@ -322,8 +327,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: percentileStore, dismissedMessageIds: []) @@ -353,8 +359,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: percentileStore, dismissedMessageIds: []) @@ -384,8 +391,9 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: percentileStore, dismissedMessageIds: []) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift index c42f273c0..de2497f53 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift @@ -36,8 +36,9 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: [] ) @@ -65,8 +66,9 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { favoritesCount: 0, appTheme: "light", isWidgetInstalled: false, - isNetPWaitlistUser: false, - daysSinceNetPEnabled: -1), + daysSinceNetPEnabled: -1, + isPrivacyProEligibleUser: false, + isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index e7a3d8fab..c8b7ecdd8 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -237,11 +237,14 @@ { "id": 8, "attributes": { - "isNetPWaitlistUser": { - "value": true - }, "daysSinceNetPEnabled": { "min": 5 + }, + "pproEligible": { + "value": true + }, + "pproSubscriber": { + "value": true } } }, From 2f523947ad64b023f878c1d08738cd4231ac5d7a Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 19 May 2024 20:35:12 -0700 Subject: [PATCH 02/11] Add support for the `survey` action. --- .../JsonToRemoteConfigModelMapper.swift | 8 +++- .../JsonToRemoteMessageModelMapper.swift | 41 ++++++++++++++----- .../RemoteMessagingSurveyActionMapping.swift | 33 +++++++++++++++ .../Model/JsonRemoteMessagingConfig.swift | 2 +- .../Model/RemoteMessageModel.swift | 2 +- .../RemoteMessagingConfigMatcher.swift | 3 ++ .../RemoteMessagingConfigProcessor.swift | 5 ++- .../JsonToRemoteConfigModelMapperTests.swift | 8 ++-- .../MockRemoteMessageSurveyActionMapper.swift | 29 +++++++++++++ .../RemoteMessagingConfigMatcherTests.swift | 11 ++++- .../RemoteMessagingConfigProcessorTests.swift | 2 + .../Resources/remote-messaging-config.json | 7 +++- 12 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift create mode 100644 Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift index 56caf5c64..981db0ee1 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift @@ -21,8 +21,12 @@ import Foundation struct JsonToRemoteConfigModelMapper { - static func mapJson(remoteMessagingConfig: RemoteMessageResponse.JsonRemoteMessagingConfig) -> RemoteConfigModel { - let remoteMessages = JsonToRemoteMessageModelMapper.maps(jsonRemoteMessages: remoteMessagingConfig.messages) + static func mapJson(remoteMessagingConfig: RemoteMessageResponse.JsonRemoteMessagingConfig, + surveyActionMapper: RemoteMessagingSurveyActionMapping) -> RemoteConfigModel { + let remoteMessages = JsonToRemoteMessageModelMapper.maps( + jsonRemoteMessages: remoteMessagingConfig.messages, + surveyActionMapper: surveyActionMapper + ) os_log("remoteMessages mapped = %s", log: .remoteMessaging, type: .debug, String(describing: remoteMessages)) let rules = JsonToRemoteMessageModelMapper.maps(jsonRemoteRules: remoteMessagingConfig.rules) os_log("rules mapped = %s", log: .remoteMessaging, type: .debug, String(describing: rules)) diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index 3a8c81eaf..513bfc50f 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -67,11 +67,15 @@ private enum AttributesKey: String, CaseIterable { struct JsonToRemoteMessageModelMapper { - static func maps(jsonRemoteMessages: [RemoteMessageResponse.JsonRemoteMessage]) -> [RemoteMessageModel] { + static func maps(jsonRemoteMessages: [RemoteMessageResponse.JsonRemoteMessage], + surveyActionMapper: RemoteMessagingSurveyActionMapping) -> [RemoteMessageModel] { var remoteMessages: [RemoteMessageModel] = [] jsonRemoteMessages.forEach { message in var remoteMessage = RemoteMessageModel(id: message.id, - content: mapToContent(content: message.content), + content: mapToContent( + content: message.content, + surveyActionMapper: surveyActionMapper + ), matchingRules: message.matchingRules ?? [], exclusionRules: message.exclusionRules ?? []) @@ -85,7 +89,8 @@ struct JsonToRemoteMessageModelMapper { } // swiftlint:disable cyclomatic_complexity function_body_length - static func mapToContent(content: RemoteMessageResponse.JsonContent) -> RemoteMessageModelType? { + static func mapToContent(content: RemoteMessageResponse.JsonContent, + surveyActionMapper: RemoteMessagingSurveyActionMapping) -> RemoteMessageModelType? { switch RemoteMessageResponse.JsonMessageType(rawValue: content.messageType) { case .small: guard !content.titleText.isEmpty, !content.descriptionText.isEmpty else { @@ -105,7 +110,7 @@ struct JsonToRemoteMessageModelMapper { case .bigSingleAction: guard let primaryActionText = content.primaryActionText, !primaryActionText.isEmpty, - let action = mapToAction(content.primaryAction) + let action = mapToAction(content.primaryAction, surveyActionMapper: surveyActionMapper) else { return nil } @@ -118,10 +123,10 @@ struct JsonToRemoteMessageModelMapper { case .bigTwoAction: guard let primaryActionText = content.primaryActionText, !primaryActionText.isEmpty, - let primaryAction = mapToAction(content.primaryAction), + let primaryAction = mapToAction(content.primaryAction, surveyActionMapper: surveyActionMapper), let secondaryActionText = content.secondaryActionText, !secondaryActionText.isEmpty, - let secondaryAction = mapToAction(content.secondaryAction) + let secondaryAction = mapToAction(content.secondaryAction, surveyActionMapper: surveyActionMapper) else { return nil } @@ -136,7 +141,7 @@ struct JsonToRemoteMessageModelMapper { case .promoSingleAction: guard let actionText = content.actionText, !actionText.isEmpty, - let action = mapToAction(content.action) + let action = mapToAction(content.action, surveyActionMapper: surveyActionMapper) else { return nil } @@ -153,7 +158,8 @@ struct JsonToRemoteMessageModelMapper { } // swiftlint:enable cyclomatic_complexity function_body_length - static func mapToAction(_ jsonAction: RemoteMessageResponse.JsonMessageAction?) -> RemoteAction? { + static func mapToAction(_ jsonAction: RemoteMessageResponse.JsonMessageAction?, + surveyActionMapper: RemoteMessagingSurveyActionMapping) -> RemoteAction? { guard let jsonAction = jsonAction else { return nil } @@ -163,8 +169,23 @@ struct JsonToRemoteMessageModelMapper { return .share(value: jsonAction.value, title: jsonAction.additionalParameters?["title"]) case .url: return .url(value: jsonAction.value) - case .surveyURL: - return .surveyURL(value: jsonAction.value) + case .survey: + if let queryParamsString = jsonAction.additionalParameters?["queryParams"] as? String { + let queryParams = queryParamsString.components(separatedBy: ";") + let mappedQueryParams = queryParams.compactMap { param in + return RemoteMessagingSurveyActionParameter(rawValue: param) + } + + if mappedQueryParams.count == queryParams.count, let surveyURL = URL(string: jsonAction.value) { + let updatedURL = surveyActionMapper.add(parameters: mappedQueryParams, to: surveyURL) + return .survey(value: updatedURL.absoluteString) + } else { + // The message requires a parameter that isn't supported + return nil + } + } else { + return .survey(value: jsonAction.value) + } case .appStore: return .appStore case .dismiss: diff --git a/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift new file mode 100644 index 000000000..902d3fb35 --- /dev/null +++ b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift @@ -0,0 +1,33 @@ +// +// RemoteMessagingSurveyActionMapping.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +public enum RemoteMessagingSurveyActionParameter: String, CaseIterable { + case atb = "atb" + case atbVariant = "var" + case daysInstalled = "delta" + case osVersion = "osv" + case lastActiveDate = "da" +} + +public protocol RemoteMessagingSurveyActionMapping { + + func add(parameters: [RemoteMessagingSurveyActionParameter], to url: URL) -> URL + +} diff --git a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift index eda32d920..78b3e2a24 100644 --- a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift +++ b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift @@ -87,7 +87,7 @@ public enum RemoteMessageResponse { case url case appStore = "appstore" case dismiss - case surveyURL = "survey_url" + case survey = "survey" } enum JsonPlaceholder: String, CaseIterable { diff --git a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift index 2bf6a39e0..2c0773ad6 100644 --- a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift @@ -102,7 +102,7 @@ public enum RemoteMessageModelType: Codable, Equatable { public enum RemoteAction: Codable, Equatable { case share(value: String, title: String?) case url(value: String) - case surveyURL(value: String) + case survey(value: String) case appStore case dismiss } diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift index 23d4c9471..4df18d7a6 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift @@ -26,6 +26,7 @@ public struct RemoteMessagingConfigMatcher { private let userAttributeMatcher: UserAttributeMatcher private let percentileStore: RemoteMessagingPercentileStoring private let dismissedMessageIds: [String] + let surveyActionMapper: RemoteMessagingSurveyActionMapping private let matchers: [AttributeMatcher] @@ -33,11 +34,13 @@ public struct RemoteMessagingConfigMatcher { deviceAttributeMatcher: DeviceAttributeMatcher = DeviceAttributeMatcher(), userAttributeMatcher: UserAttributeMatcher, percentileStore: RemoteMessagingPercentileStoring, + surveyActionMapper: RemoteMessagingSurveyActionMapping, dismissedMessageIds: [String]) { self.appAttributeMatcher = appAttributeMatcher self.deviceAttributeMatcher = deviceAttributeMatcher self.userAttributeMatcher = userAttributeMatcher self.percentileStore = percentileStore + self.surveyActionMapper = surveyActionMapper self.dismissedMessageIds = dismissedMessageIds matchers = [appAttributeMatcher, deviceAttributeMatcher, userAttributeMatcher] diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift index 233864732..e8afc324d 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift @@ -42,7 +42,10 @@ public struct RemoteMessagingConfigProcessor { let isNewVersion = newVersion != currentVersion if isNewVersion || shouldProcessConfig(currentConfig) { - let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: jsonRemoteMessagingConfig) + let config = JsonToRemoteConfigModelMapper.mapJson( + remoteMessagingConfig: jsonRemoteMessagingConfig, + surveyActionMapper: remoteMessagingConfigMatcher.surveyActionMapper + ) let message = remoteMessagingConfigMatcher.evaluate(remoteConfig: config) os_log("Message to present next: %s", log: .remoteMessaging, type: .debug, message.debugDescription) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 11e2b359d..f752066a3 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -92,7 +92,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { descriptionText: "Survey Description", placeholder: .vpnAnnounce, actionText: "Survey Action", - action: .surveyURL(value: "https://duckduckgo.com/survey") + action: .survey(value: "https://duckduckgo.com/survey") ), matchingRules: [8], exclusionRules: []) @@ -183,8 +183,9 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { func testWhenJsonAttributeMissingThenUnknownIntoConfig() throws { let validJson = data.fromJsonFile("Resources/remote-messaging-config-malformed.json") let remoteMessagingConfig = try JSONDecoder().decode(RemoteMessageResponse.JsonRemoteMessagingConfig.self, from: validJson) + let surveyMapper = MockRemoteMessageSurveyActionMapper() XCTAssertNotNil(remoteMessagingConfig) - let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig) + let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig, surveyActionMapper: surveyMapper) XCTAssertTrue(config.rules.count == 2) let rule6 = config.rules.filter { $0.id == 6 }.first @@ -197,9 +198,10 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { func decodeAndMapJson(fileName: String) throws -> RemoteConfigModel { let validJson = data.fromJsonFile(fileName) let remoteMessagingConfig = try JSONDecoder().decode(RemoteMessageResponse.JsonRemoteMessagingConfig.self, from: validJson) + let surveyMapper = MockRemoteMessageSurveyActionMapper() XCTAssertNotNil(remoteMessagingConfig) - let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig) + let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig, surveyActionMapper: surveyMapper) XCTAssertNotNil(config) return config } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift new file mode 100644 index 000000000..9cb86c522 --- /dev/null +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift @@ -0,0 +1,29 @@ +// +// MockRemoteMessageSurveyActionMapper.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import RemoteMessaging + +class MockRemoteMessageSurveyActionMapper: RemoteMessagingSurveyActionMapping { + + func add(parameters: [RemoteMessaging.RemoteMessagingSurveyActionParameter], to url: URL) -> URL { + return url + } + +} + diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift index 487a6cf1d..ebaf2060c 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift @@ -47,6 +47,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: [] ) } @@ -132,6 +133,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -227,6 +229,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: ["1"]) let remoteConfig = RemoteConfigModel(messages: [ @@ -265,6 +268,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -299,6 +303,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: percentileStore, + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -331,6 +336,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: percentileStore, + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -363,6 +369,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: percentileStore, + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -395,6 +402,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: percentileStore, + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -443,9 +451,10 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { func decodeAndMapJson(fileName: String) throws -> RemoteConfigModel { let validJson = data.fromJsonFile(fileName) let remoteMessagingConfig = try JSONDecoder().decode(RemoteMessageResponse.JsonRemoteMessagingConfig.self, from: validJson) + let surveyMapper = MockRemoteMessageSurveyActionMapper() XCTAssertNotNil(remoteMessagingConfig) - let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig) + let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig, surveyActionMapper: surveyMapper) XCTAssertNotNil(config) return config } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift index de2497f53..bbfa2dd59 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift @@ -40,6 +40,7 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: [] ) @@ -70,6 +71,7 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { isPrivacyProEligibleUser: false, isPrivacyProSubscriber: false), percentileStore: MockRemoteMessagePercentileStore(), + surveyActionMapper: MockRemoteMessageSurveyActionMapper(), dismissedMessageIds: []) let processor = RemoteMessagingConfigProcessor(remoteMessagingConfigMatcher: remoteMessagingConfigMatcher) diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index c8b7ecdd8..c90e2bb71 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -143,8 +143,11 @@ "placeholder": "VPNAnnounce", "actionText": "Survey Action", "action": { - "type": "survey_url", - "value": "https://duckduckgo.com/survey" + "type": "survey", + "value": "https://duckduckgo.com/survey", + "additionalParameters": { + "queryParams": "atb;var;delta;iosv;ddgv;da" + } } }, "matchingRules": [ From 583ed5cfb4aeb97dcb50ce2e6ec341ee51d4ced4 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 19 May 2024 20:35:52 -0700 Subject: [PATCH 03/11] Add .swift.plist to .gitignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7aafd7b2d..fdb78a2ca 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ .DS_Store xcuserdata/ .vscode +*.swift.plist From 1ec4faf83f9da0386f7b3eb0f88c4c4464145905 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 19 May 2024 20:47:37 -0700 Subject: [PATCH 04/11] Fix SwiftLint. --- .../Mocks/MockRemoteMessageSurveyActionMapper.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift index 9cb86c522..07f9ec4d0 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessageSurveyActionMapper.swift @@ -26,4 +26,3 @@ class MockRemoteMessageSurveyActionMapper: RemoteMessagingSurveyActionMapping { } } - From 5560065a9fdf5081560cc026a0f02c56354c0f54 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 19 May 2024 20:49:30 -0700 Subject: [PATCH 05/11] Fix unit tests. --- .../RemoteMessaging/Matchers/UserAttributeMatcherTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift index 0a9988f98..683874057 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Matchers/UserAttributeMatcherTests.swift @@ -54,8 +54,8 @@ class UserAttributeMatcherTests: XCTestCase { appTheme: "default", isWidgetInstalled: true, daysSinceNetPEnabled: 3, - isPrivacyProEligibleUser: false, - isPrivacyProSubscriber: false) + isPrivacyProEligibleUser: true, + isPrivacyProSubscriber: true) } override func tearDownWithError() throws { From 74cea09bc215d51aa727fed4229e159380ba28ae Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 20 May 2024 14:46:20 -0700 Subject: [PATCH 06/11] Tweak available parameters. --- .../Mappers/RemoteMessagingSurveyActionMapping.swift | 1 + .../Resources/remote-messaging-config.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift index 902d3fb35..2c8f060b4 100644 --- a/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift +++ b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift @@ -23,6 +23,7 @@ public enum RemoteMessagingSurveyActionParameter: String, CaseIterable { case atbVariant = "var" case daysInstalled = "delta" case osVersion = "osv" + case appVersion = "ddgv" case lastActiveDate = "da" } diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index c90e2bb71..a8e50063a 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -146,7 +146,7 @@ "type": "survey", "value": "https://duckduckgo.com/survey", "additionalParameters": { - "queryParams": "atb;var;delta;iosv;ddgv;da" + "queryParams": "atb;var;delta;osv;ddgv;da" } } }, From 0bb46a5c37b5741ed1eb8a806fb498755cad92bc Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 20 May 2024 15:36:50 -0700 Subject: [PATCH 07/11] Add hardwareModel parameter. --- .../Mappers/RemoteMessagingSurveyActionMapping.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift index 2c8f060b4..243ee6a79 100644 --- a/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift +++ b/Sources/RemoteMessaging/Mappers/RemoteMessagingSurveyActionMapping.swift @@ -19,12 +19,13 @@ import Foundation public enum RemoteMessagingSurveyActionParameter: String, CaseIterable { + case appVersion = "ddgv" case atb = "atb" case atbVariant = "var" case daysInstalled = "delta" - case osVersion = "osv" - case appVersion = "ddgv" + case hardwareModel = "mo" case lastActiveDate = "da" + case osVersion = "osv" } public protocol RemoteMessagingSurveyActionMapping { From b080d80b2615fbd0c8e146a2d4ba3b10a0e9e325 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 20 May 2024 19:22:50 -0700 Subject: [PATCH 08/11] Test a message with unhandled survey parameters. --- .../JsonToRemoteConfigModelMapperTests.swift | 9 ++++++++- .../Resources/remote-messaging-config.json | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index f752066a3..44a767a35 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -26,7 +26,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { func testWhenValidJsonParsedThenMessagesMappedIntoRemoteConfig() throws { let config = try decodeAndMapJson(fileName: "Resources/remote-messaging-config.json") - XCTAssertEqual(config.messages.count, 8) + XCTAssertEqual(config.messages.count, 9) XCTAssertEqual(config.messages[0], RemoteMessageModel( id: "8274589c-8aeb-4322-a737-3852911569e3", @@ -98,6 +98,13 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { exclusionRules: []) ) + XCTAssertEqual(config.messages[8], RemoteMessageModel( + id: "9848E904-4345-09C8-FEAA-3B2C75DC285B", + content: nil, + matchingRules: [], + exclusionRules: []) + ) + } func testWhenValidJsonParsedThenRulesMappedIntoRemoteConfig() throws { diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index a8e50063a..ee5f75350 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -153,6 +153,23 @@ "matchingRules": [ 8 ] + }, + { + "id": "9848E904-4345-09C8-FEAA-3B2C75DC285B", + "content": { + "messageType": "promo_single_action", + "titleText": "Survey Title", + "descriptionText": "Survey Description", + "placeholder": "VPNAnnounce", + "actionText": "Survey Action", + "action": { + "type": "survey", + "value": "https://duckduckgo.com/survey", + "additionalParameters": { + "queryParams": "atb;var;delta;osv;ddgv;da;unknown_param_which_is_not_supported" + } + } + } } ], "rules": [ From 61342191d548b9d61e160144c289f36c3dd921c5 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 20 May 2024 20:04:46 -0700 Subject: [PATCH 09/11] =?UTF-8?q?Always=20process=20messages,=20since=20I?= =?UTF-8?q?=20can=E2=80=99t=20get=20BSK=20to=20work=20with=20iOS=20locally?= =?UTF-8?q?.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift index e8afc324d..1baa4957d 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift @@ -56,6 +56,9 @@ public struct RemoteMessagingConfigProcessor { } func shouldProcessConfig(_ currentConfig: RemoteMessagingConfig?) -> Bool { + // TODO: Remove before merging + return true + guard let currentConfig = currentConfig else { return true } From 6c2092a1b3a42e338bcd01bfb2878368fdd8931a Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 20 May 2024 20:50:53 -0700 Subject: [PATCH 10/11] Remove hardcoded message process return value. --- Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift index 1baa4957d..e8afc324d 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift @@ -56,9 +56,6 @@ public struct RemoteMessagingConfigProcessor { } func shouldProcessConfig(_ currentConfig: RemoteMessagingConfig?) -> Bool { - // TODO: Remove before merging - return true - guard let currentConfig = currentConfig else { return true } From 2a4f30cd03cd2d2ebed058302300aacb4cbeb26a Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Wed, 22 May 2024 09:03:14 -0700 Subject: [PATCH 11/11] Apply a suggested bug fix related to message parsing. Also, update tests to make sure that a message which cannot be read successfully does not impact subsequent messages. --- .../JsonToRemoteMessageModelMapper.swift | 9 +++++---- .../JsonToRemoteConfigModelMapperTests.swift | 9 +-------- .../Resources/remote-messaging-config.json | 18 +++++++++--------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index 513bfc50f..c84074299 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -71,11 +71,12 @@ struct JsonToRemoteMessageModelMapper { surveyActionMapper: RemoteMessagingSurveyActionMapping) -> [RemoteMessageModel] { var remoteMessages: [RemoteMessageModel] = [] jsonRemoteMessages.forEach { message in + guard let content = mapToContent( content: message.content, surveyActionMapper: surveyActionMapper) else { + return + } + var remoteMessage = RemoteMessageModel(id: message.id, - content: mapToContent( - content: message.content, - surveyActionMapper: surveyActionMapper - ), + content: content, matchingRules: message.matchingRules ?? [], exclusionRules: message.exclusionRules ?? []) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 44a767a35..f752066a3 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -26,7 +26,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { func testWhenValidJsonParsedThenMessagesMappedIntoRemoteConfig() throws { let config = try decodeAndMapJson(fileName: "Resources/remote-messaging-config.json") - XCTAssertEqual(config.messages.count, 9) + XCTAssertEqual(config.messages.count, 8) XCTAssertEqual(config.messages[0], RemoteMessageModel( id: "8274589c-8aeb-4322-a737-3852911569e3", @@ -98,13 +98,6 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { exclusionRules: []) ) - XCTAssertEqual(config.messages[8], RemoteMessageModel( - id: "9848E904-4345-09C8-FEAA-3B2C75DC285B", - content: nil, - matchingRules: [], - exclusionRules: []) - ) - } func testWhenValidJsonParsedThenRulesMappedIntoRemoteConfig() throws { diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index ee5f75350..e75d3a15d 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -135,7 +135,7 @@ } }, { - "id": "8E909844-C809-4543-AAFE-2C75DC285B3B", + "id": "9848E904-4345-09C8-FEAA-3B2C75DC285B", "content": { "messageType": "promo_single_action", "titleText": "Survey Title", @@ -146,16 +146,13 @@ "type": "survey", "value": "https://duckduckgo.com/survey", "additionalParameters": { - "queryParams": "atb;var;delta;osv;ddgv;da" + "queryParams": "atb;var;delta;osv;ddgv;da;unknown_param_which_is_not_supported" } } - }, - "matchingRules": [ - 8 - ] + } }, { - "id": "9848E904-4345-09C8-FEAA-3B2C75DC285B", + "id": "8E909844-C809-4543-AAFE-2C75DC285B3B", "content": { "messageType": "promo_single_action", "titleText": "Survey Title", @@ -166,10 +163,13 @@ "type": "survey", "value": "https://duckduckgo.com/survey", "additionalParameters": { - "queryParams": "atb;var;delta;osv;ddgv;da;unknown_param_which_is_not_supported" + "queryParams": "atb;var;delta;osv;ddgv;da" } } - } + }, + "matchingRules": [ + 8 + ] } ], "rules": [