Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for skipping sending usage pixels for remote messages #902

Merged
merged 4 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1540"
version = "1.7">
version = "1.8">
<BuildAction
parallelizeBuildables = "YES"
buildImplicitDependencies = "YES"
Expand Down Expand Up @@ -413,6 +413,20 @@
ReferencedContainer = "container:">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
buildForProfiling = "YES"
buildForArchiving = "YES"
buildForAnalyzing = "YES">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "CxxCrashHandler"
BuildableName = "CxxCrashHandler"
BlueprintName = "CxxCrashHandler"
ReferencedContainer = "container:">
</BuildableReference>
</BuildActionEntry>
Comment on lines +416 to +429
Copy link
Member

Choose a reason for hiding this comment

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

Is this part of the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My Xcode keeps bugging me about adding this. It's an update to the otherwise auto-generated scheme, so I reckon it doesn't hurt to include it.

</BuildActionEntries>
</BuildAction>
<TestAction
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1540"
version = "1.7">
version = "1.8">
<BuildAction
parallelizeBuildables = "YES"
buildImplicitDependencies = "YES"
Expand Down
1 change: 1 addition & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ let package = Package(
resources: [
.copy("Resources/remote-messaging-config-example.json"),
.copy("Resources/remote-messaging-config-malformed.json"),
.copy("Resources/remote-messaging-config-metrics.json"),
.copy("Resources/remote-messaging-config-unsupported-items.json"),
.copy("Resources/remote-messaging-config.json"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,13 @@ struct JsonToRemoteMessageModelMapper {
return
}

var remoteMessage = RemoteMessageModel(id: message.id,
content: content,
matchingRules: message.matchingRules ?? [],
exclusionRules: message.exclusionRules ?? [])
var remoteMessage = RemoteMessageModel(
id: message.id,
content: content,
matchingRules: message.matchingRules ?? [],
exclusionRules: message.exclusionRules ?? [],
isMetricsEnabled: message.isMetricsEnabled
)

if let translation = getTranslation(from: message.translations, for: Locale.current) {
remoteMessage.localizeContent(translation: translation)
Expand Down
14 changes: 14 additions & 0 deletions Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,24 @@ public enum RemoteMessageResponse {
let content: JsonContent
let translations: [String: JsonContentTranslation]?
let matchingRules, exclusionRules: [Int]?
let metrics: JsonMetrics?

static func == (lhs: JsonRemoteMessage, rhs: JsonRemoteMessage) -> Bool {
return lhs.id == rhs.id
}

var isMetricsEnabled: Bool {
metrics?.state.flatMap(JsonMetrics.MetricsState.init) != .disabled
}
}

struct JsonMetrics: Decodable {
let state: String?

enum MetricsState: String, Decodable {
case disabled
case enabled
}
Comment on lines +39 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual change – parse metrics object within the message definition if it exists and decode state property. If "disabled", disable metrics.

}

struct JsonContent: Decodable {
Expand Down
33 changes: 18 additions & 15 deletions Sources/RemoteMessaging/Model/RemoteMessageModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,31 @@ public struct RemoteMessageModel: Equatable, Codable {
public var content: RemoteMessageModelType?
public let matchingRules: [Int]
public let exclusionRules: [Int]
public let isMetricsEnabled: Bool

public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int]) {
public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int], isMetricsEnabled: Bool) {
self.id = id
self.content = content
self.matchingRules = matchingRules
self.exclusionRules = exclusionRules
self.isMetricsEnabled = isMetricsEnabled
}

public static func == (lhs: RemoteMessageModel, rhs: RemoteMessageModel) -> Bool {
if lhs.id != rhs.id {
return false
}
if lhs.content != rhs.content {
return false
}
if lhs.matchingRules != rhs.matchingRules {
return false
}
if lhs.exclusionRules != rhs.exclusionRules {
return false
}
return true
enum CodingKeys: CodingKey {
case id
case content
case matchingRules
case exclusionRules
case isMetricsEnabled
}

public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.id = try container.decode(String.self, forKey: .id)
self.content = try container.decodeIfPresent(RemoteMessageModelType.self, forKey: .content)
self.matchingRules = try container.decode([Int].self, forKey: .matchingRules)
self.exclusionRules = try container.decode([Int].self, forKey: .exclusionRules)
self.isMetricsEnabled = try container.decodeIfPresent(Bool.self, forKey: .isMetricsEnabled) ?? true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ?? true part is why we require custom init function

}

mutating func localizeContent(translation: RemoteMessageResponse.JsonContentTranslation) {
Expand Down
16 changes: 14 additions & 2 deletions Sources/RemoteMessaging/RemoteMessagingStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,13 @@ extension RemoteMessagingStore {
continue
}

scheduledRemoteMessage = RemoteMessageModel(id: id, content: remoteMessage.content, matchingRules: [], exclusionRules: [])
scheduledRemoteMessage = RemoteMessageModel(
id: id,
content: remoteMessage.content,
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: remoteMessage.isMetricsEnabled
)
break
}
}
Expand Down Expand Up @@ -255,7 +261,13 @@ extension RemoteMessagingStore {
continue
}

remoteMessage = RemoteMessageModel(id: id, content: remoteMessageMapped.content, matchingRules: [], exclusionRules: [])
remoteMessage = RemoteMessageModel(
id: id,
content: remoteMessageMapped.content,
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: remoteMessageMapped.isMetricsEnabled
)
break
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,33 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase {
content: .bigSingleAction(titleText: "title", descriptionText: "description", placeholder: .announce,
primaryActionText: "Ok", primaryAction: .url(value: "https://duckduckgo.com")),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[1], RemoteMessageModel(
id: "8274589c-8aeb-4322-a737-3852911569e3",
content: .bigSingleAction(titleText: "Kedvenc hozzáadása", descriptionText: "Kedvenc eltávolítása", placeholder: .ddgAnnounce,
primaryActionText: "Ok", primaryAction: .url(value: "https://duckduckgo.com")),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[2], RemoteMessageModel(
id: "26780792-49fe-4e25-ae27-aa6a2e6f013b",
content: .small(titleText: "Here goes a title", descriptionText: "description"),
matchingRules: [5, 6],
exclusionRules: [7, 8, 9])
exclusionRules: [7, 8, 9],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[3], RemoteMessageModel(
id: "c3549d64-b388-41d8-9649-33e6e2674e8e",
content: .medium(titleText: "Here goes a title", descriptionText: "description", placeholder: .criticalUpdate),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[4], RemoteMessageModel(
Expand All @@ -62,7 +66,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase {
primaryActionText: "Ok", primaryAction: .appStore,
secondaryActionText: "Cancel", secondaryAction: .dismiss),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[5], RemoteMessageModel(
Expand All @@ -72,15 +77,17 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase {
title: "Share Title"),
secondaryActionText: "Cancel", secondaryAction: .dismiss),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[6], RemoteMessageModel(
id: "6E58D3DA-AB9D-47A4-87B7-B8AF830BFB5E",
content: .promoSingleAction(titleText: "Promo Title", descriptionText: "Promo Description", placeholder: .newForMacAndWindows,
actionText: "Promo Action", action: .dismiss),
matchingRules: [],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[7], RemoteMessageModel(
Expand All @@ -93,7 +100,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase {
action: .survey(value: "https://duckduckgo.com/survey")
),
matchingRules: [8],
exclusionRules: [])
exclusionRules: [],
isMetricsEnabled: true)
)

}
Expand Down Expand Up @@ -225,6 +233,43 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase {
XCTAssertEqual(rule6?.attributes.filter { $0 is UnknownMatchingAttribute }.count, 1)
}

func testThatMetricsAreEnabledWhenStatedInJSONOrMissing() throws {
let config = try decodeAndMapJson(fileName: "remote-messaging-config-metrics.json")
XCTAssertEqual(config.messages.count, 4)

XCTAssertEqual(config.messages[0], RemoteMessageModel(
id: "1",
content: .small(titleText: "title", descriptionText: "description"),
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[1], RemoteMessageModel(
id: "2",
content: .small(titleText: "title", descriptionText: "description"),
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: true)
)

XCTAssertEqual(config.messages[2], RemoteMessageModel(
id: "3",
content: .small(titleText: "title", descriptionText: "description"),
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: false)
)

XCTAssertEqual(config.messages[3], RemoteMessageModel(
id: "4",
content: .small(titleText: "title", descriptionText: "description"),
matchingRules: [],
exclusionRules: [],
isMetricsEnabled: true)
)
}

func decodeAndMapJson(fileName: String) throws -> RemoteConfigModel {
let resourceURL = Bundle.module.resourceURL!.appendingPathComponent(fileName, conformingTo: .json)
let validJson = try Data(contentsOf: resourceURL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,10 @@ class RemoteMessagingConfigMatcherTests: XCTestCase {

func mediumMessage(id: String = "1", matchingRules: [Int], exclusionRules: [Int]) -> RemoteMessageModel {
return RemoteMessageModel(id: id,
content: .medium(titleText: "title", descriptionText: "description", placeholder: .announce),
matchingRules: matchingRules,
exclusionRules: exclusionRules
content: .medium(titleText: "title", descriptionText: "description", placeholder: .announce),
matchingRules: matchingRules,
exclusionRules: exclusionRules,
isMetricsEnabled: true
)
}
}
2 changes: 1 addition & 1 deletion Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class RemoteMessagingStoreTests: XCTestCase {
XCTAssertEqual(config?.version, processorResult.version)
guard let remoteMessage = store.fetchScheduledRemoteMessage() else {
XCTFail("No remote message found")
return RemoteMessageModel(id: "", content: nil, matchingRules: [], exclusionRules: [])
return RemoteMessageModel(id: "", content: nil, matchingRules: [], exclusionRules: [], isMetricsEnabled: true)
}

XCTAssertNotNil(remoteMessage)
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file contains all supported cases for metrics object, including no metrics object at all.

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"version": 1,
"messages": [
{
"id": "1",
"content": {
"messageType": "small",
"titleText": "title",
"descriptionText": "description"
}
},
{
"id": "2",
"content": {
"messageType": "small",
"titleText": "title",
"descriptionText": "description"
},
"metrics": {
"state": "enabled"
}
},
{
"id": "3",
"content": {
"messageType": "small",
"titleText": "title",
"descriptionText": "description"
},
"metrics": {
"state": "disabled"
}
},
{
"id": "4",
"content": {
"messageType": "small",
"titleText": "title",
"descriptionText": "description"
},
"metrics": {
"state": "unsupported-value"
}
}
],
"rules": []
}
Loading