From e721e3c147b6c320b4eb1cc47da870259e9abe97 Mon Sep 17 00:00:00 2001 From: Ajay Subramanya Date: Wed, 29 May 2024 13:40:21 -0500 Subject: [PATCH 1/5] Not returning if not inited and fixed tests --- Sources/KlaviyoSwift/StateManagement.swift | 1 - .../StateManagementEdgeCaseTests.swift | 24 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Sources/KlaviyoSwift/StateManagement.swift b/Sources/KlaviyoSwift/StateManagement.swift index 048c6165..edbaf501 100644 --- a/Sources/KlaviyoSwift/StateManagement.swift +++ b/Sources/KlaviyoSwift/StateManagement.swift @@ -112,7 +112,6 @@ struct KlaviyoReducer: ReducerProtocol { if action.requiresInitialization, case .uninitialized = state.initalizationState { environment.emitDeveloperWarning("SDK must be initialized before usage.") - return .none } switch action { diff --git a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift index 6acf4426..9b94b5b0 100644 --- a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift +++ b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift @@ -131,7 +131,9 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setEmail("test@blob.com")) + _ = await store.send(.setEmail("test@blob.com")) { + $0.pendingRequests = [.setEmail("test@blob.com")] + } await fulfillment(of: [expection]) } @@ -178,7 +180,9 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setExternalId("external-blob-id")) + _ = await store.send(.setExternalId("external-blob-id")) { + $0.pendingRequests = [.setExternalId("external-blob-id")] + } } @MainActor @@ -223,7 +227,9 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setPhoneNumber("1-800-Blobs4u")) + _ = await store.send(.setPhoneNumber("1-800-Blobs4u")) { + $0.pendingRequests = [.setPhoneNumber("1-800-Blobs4u")] + } } @MainActor @@ -267,7 +273,9 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setPushToken("blob_token", .authorized)) + _ = await store.send(.setPushToken("blob_token", .authorized)) { + $0.pendingRequests = [.pushToken("blob_token", .authorized)] + } } @MainActor @@ -377,7 +385,9 @@ class StateManagementEdgeCaseTests: XCTestCase { } let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) let event = Event(name: .OpenedPush) - _ = await store.send(.enqueueEvent(event)) + _ = await store.send(.enqueueEvent(event)) { + $0.pendingRequests = [.event(event)] + } await fulfillment(of: [expection]) } @@ -392,7 +402,9 @@ class StateManagementEdgeCaseTests: XCTestCase { } let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) let profile = Profile(email: "foo") - _ = await store.send(.enqueueProfile(profile)) + _ = await store.send(.enqueueProfile(profile)) { + $0.pendingRequests = [.profile(profile)] + } await fulfillment(of: [expection]) } From 30492c61a0697cbc75201ece73ccd6f27228de43 Mon Sep 17 00:00:00 2001 From: Ajay Subramanya Date: Thu, 30 May 2024 11:39:32 -0500 Subject: [PATCH 2/5] added opened event case to requires initlizatiom --- Sources/KlaviyoSwift/StateManagement.swift | 7 ++++- .../StateManagementEdgeCaseTests.swift | 26 ++++--------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/Sources/KlaviyoSwift/StateManagement.swift b/Sources/KlaviyoSwift/StateManagement.swift index edbaf501..a1ff76fb 100644 --- a/Sources/KlaviyoSwift/StateManagement.swift +++ b/Sources/KlaviyoSwift/StateManagement.swift @@ -92,7 +92,11 @@ enum KlaviyoAction: Equatable { var requiresInitialization: Bool { switch self { - case .setEmail, .setPhoneNumber, .setExternalId, .setPushToken, .enqueueEvent, .enqueueProfile, .setProfileProperty, .resetProfile, .resetStateAndDequeue: + case let .enqueueEvent(event): + // if event metric is opened push we DON'T require initilization in all other event metric cases we DO. + return event.metric.name == .OpenedPush ? false : true + + case .setEmail, .setPhoneNumber, .setExternalId, .setPushToken, .enqueueProfile, .setProfileProperty, .resetProfile, .resetStateAndDequeue: return true case .initialize, .completeInitialization, .deQueueCompletedResults, .networkConnectivityChanged, .flushQueue, .sendRequest, .stop, .start, .cancelInFlightRequests, .requestFailed: @@ -112,6 +116,7 @@ struct KlaviyoReducer: ReducerProtocol { if action.requiresInitialization, case .uninitialized = state.initalizationState { environment.emitDeveloperWarning("SDK must be initialized before usage.") + return .none } switch action { diff --git a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift index 9b94b5b0..3756e319 100644 --- a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift +++ b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift @@ -131,9 +131,7 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setEmail("test@blob.com")) { - $0.pendingRequests = [.setEmail("test@blob.com")] - } + _ = await store.send(.setEmail("test@blob.com")) await fulfillment(of: [expection]) } @@ -180,9 +178,7 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setExternalId("external-blob-id")) { - $0.pendingRequests = [.setExternalId("external-blob-id")] - } + _ = await store.send(.setExternalId("external-blob-id")) } @MainActor @@ -227,9 +223,7 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setPhoneNumber("1-800-Blobs4u")) { - $0.pendingRequests = [.setPhoneNumber("1-800-Blobs4u")] - } + _ = await store.send(.setPhoneNumber("1-800-Blobs4u")) } @MainActor @@ -273,9 +267,7 @@ class StateManagementEdgeCaseTests: XCTestCase { flushing: false) let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) - _ = await store.send(.setPushToken("blob_token", .authorized)) { - $0.pendingRequests = [.pushToken("blob_token", .authorized)] - } + _ = await store.send(.setPushToken("blob_token", .authorized)) } @MainActor @@ -378,17 +370,11 @@ class StateManagementEdgeCaseTests: XCTestCase { @MainActor func testEnqueueEventUninitialized() async throws { - let expection = XCTestExpectation(description: "fatal error expected") - environment.emitDeveloperWarning = { _ in - // Would really runTimeWarn - not happening because we can't do that in tests so we fake it. - expection.fulfill() - } let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) let event = Event(name: .OpenedPush) _ = await store.send(.enqueueEvent(event)) { $0.pendingRequests = [.event(event)] } - await fulfillment(of: [expection]) } // MARK: - set profile uninitialized @@ -402,9 +388,7 @@ class StateManagementEdgeCaseTests: XCTestCase { } let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) let profile = Profile(email: "foo") - _ = await store.send(.enqueueProfile(profile)) { - $0.pendingRequests = [.profile(profile)] - } + _ = await store.send(.enqueueProfile(profile)) await fulfillment(of: [expection]) } From e14719554bbec554bb3095b7230c280202af5fd5 Mon Sep 17 00:00:00 2001 From: Ajay Subramanya Date: Thu, 30 May 2024 11:54:48 -0500 Subject: [PATCH 3/5] added some more tests --- .../StateManagementEdgeCaseTests.swift | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift index 3756e319..0693a6a1 100644 --- a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift +++ b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift @@ -116,7 +116,7 @@ class StateManagementEdgeCaseTests: XCTestCase { // MARK: - Set Email @MainActor - func testSetEmailUninitialized() async throws { + func testSetEmailUninitializedDoesNotAddToPendingRequest() async throws { let expection = XCTestExpectation(description: "fatal error expected") environment.emitDeveloperWarning = { _ in // Would really fatalError - not happening because we can't do that in tests so we fake it. @@ -168,7 +168,7 @@ class StateManagementEdgeCaseTests: XCTestCase { // MARK: - Set External Id @MainActor - func testSetExternalIdUninitialized() async throws { + func testSetExternalIdUninitializedDoesNotAddToPendingRequest() async throws { let apiKey = "fake-key" let initialState = KlaviyoState(apiKey: apiKey, anonymousId: environment.analytics.uuid().uuidString, @@ -213,7 +213,7 @@ class StateManagementEdgeCaseTests: XCTestCase { // MARK: - Set Phone number @MainActor - func testSetPhoneNumberUninitialized() async throws { + func testSetPhoneNumberUninitializedDoesNotAddToPendingRequest() async throws { let apiKey = "fake-key" let initialState = KlaviyoState(apiKey: apiKey, anonymousId: environment.analytics.uuid().uuidString, @@ -257,7 +257,7 @@ class StateManagementEdgeCaseTests: XCTestCase { // MARK: - Set Push Token @MainActor - func testSetPushTokenUninitialized() async throws { + func testSetPushTokenUninitializedDoesNotAddToPendingRequest() async throws { let apiKey = "fake-key" let initialState = KlaviyoState(apiKey: apiKey, anonymousId: environment.analytics.uuid().uuidString, @@ -369,7 +369,7 @@ class StateManagementEdgeCaseTests: XCTestCase { // MARK: - set enqueue event uninitialized @MainActor - func testEnqueueEventUninitialized() async throws { + func testOpenedPushEventUninitializedAddsToPendingRequests() async throws { let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) let event = Event(name: .OpenedPush) _ = await store.send(.enqueueEvent(event)) { @@ -377,6 +377,25 @@ class StateManagementEdgeCaseTests: XCTestCase { } } + @MainActor + func testEnqueueNonOpenedPushEventUninitializedDoesNotAddToPendingRequest() async throws { + let expection = XCTestExpectation(description: "fatal error expected") + environment.emitDeveloperWarning = { _ in + // Would really runTimeWarn - not happening because we can't do that in tests so we fake it. + expection.fulfill() + } + let store = TestStore(initialState: .init(queue: []), reducer: KlaviyoReducer()) + + let nonOpenedPushEvents = Event.EventName.allCases.filter { $0 != .OpenedPush } + + for event in nonOpenedPushEvents { + let event = Event(name: event) + _ = await store.send(.enqueueEvent(event)) + } + + await fulfillment(of: [expection]) + } + // MARK: - set profile uninitialized @MainActor @@ -419,3 +438,9 @@ class StateManagementEdgeCaseTests: XCTestCase { } } } + +extension Event.EventName: CaseIterable { + public static var allCases: [KlaviyoSwift.Event.EventName] { + [.OpenedPush, .OpenedAppMetric, .ViewedProductMetric, .AddedToCartMetric, .StartedCheckoutMetric, .CustomEvent("someEvent")] + } +} From fa9293f0bdbb686f29fd736bba495e4cec793425 Mon Sep 17 00:00:00 2001 From: Ajay Subramanya Date: Thu, 30 May 2024 13:43:52 -0500 Subject: [PATCH 4/5] remove ?: --- Sources/KlaviyoSwift/StateManagement.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/KlaviyoSwift/StateManagement.swift b/Sources/KlaviyoSwift/StateManagement.swift index a1ff76fb..82cb2097 100644 --- a/Sources/KlaviyoSwift/StateManagement.swift +++ b/Sources/KlaviyoSwift/StateManagement.swift @@ -94,7 +94,7 @@ enum KlaviyoAction: Equatable { switch self { case let .enqueueEvent(event): // if event metric is opened push we DON'T require initilization in all other event metric cases we DO. - return event.metric.name == .OpenedPush ? false : true + return event.metric.name != .OpenedPush case .setEmail, .setPhoneNumber, .setExternalId, .setPushToken, .enqueueProfile, .setProfileProperty, .resetProfile, .resetStateAndDequeue: return true From 4e1dc18d84075dd28687ac49271b9c780f9ea972 Mon Sep 17 00:00:00 2001 From: Ajay Subramanya Date: Thu, 30 May 2024 14:29:57 -0500 Subject: [PATCH 5/5] using case where instead of conditionals --- Sources/KlaviyoSwift/StateManagement.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/KlaviyoSwift/StateManagement.swift b/Sources/KlaviyoSwift/StateManagement.swift index 82cb2097..b2e2c4b5 100644 --- a/Sources/KlaviyoSwift/StateManagement.swift +++ b/Sources/KlaviyoSwift/StateManagement.swift @@ -92,11 +92,11 @@ enum KlaviyoAction: Equatable { var requiresInitialization: Bool { switch self { - case let .enqueueEvent(event): - // if event metric is opened push we DON'T require initilization in all other event metric cases we DO. - return event.metric.name != .OpenedPush + // if event metric is opened push we DON'T require initilization in all other event metric cases we DO. + case let .enqueueEvent(event) where event.metric.name == .OpenedPush: + return false - case .setEmail, .setPhoneNumber, .setExternalId, .setPushToken, .enqueueProfile, .setProfileProperty, .resetProfile, .resetStateAndDequeue: + case .setEmail, .setPhoneNumber, .setExternalId, .setPushToken, .enqueueProfile, .setProfileProperty, .resetProfile, .resetStateAndDequeue, .enqueueEvent: return true case .initialize, .completeInitialization, .deQueueCompletedResults, .networkConnectivityChanged, .flushQueue, .sendRequest, .stop, .start, .cancelInFlightRequests, .requestFailed: