Skip to content

Commit

Permalink
chore: update review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
erenbesel committed Jan 23, 2024
1 parent 6ceb9ce commit 842fb79
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 42 deletions.
8 changes: 4 additions & 4 deletions Adyen.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@
A0D48FB827109B0200C0B82C /* ArrayHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0D48FB727109B0200C0B82C /* ArrayHelpers.swift */; };
A0DB48662AFD020400348C83 /* AdyenAnalyticsRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB48652AFD020400348C83 /* AdyenAnalyticsRequest.swift */; };
A0DB48682AFD068400348C83 /* AdyenAnalytics.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB48672AFD068400348C83 /* AdyenAnalytics.swift */; };
A0DB486A2AFD0BDC00348C83 /* AdyenAnalyticsEvent.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsEvent.swift */; };
A0DB486A2AFD0BDC00348C83 /* AdyenAnalyticsInfo.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsInfo.swift */; };
A0DB486C2AFD0BEE00348C83 /* AdyenAnalyticsLog.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB486B2AFD0BEE00348C83 /* AdyenAnalyticsLog.swift */; };
A0DB486E2AFD0BFC00348C83 /* AdyenAnalyticsError.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DB486D2AFD0BFC00348C83 /* AdyenAnalyticsError.swift */; };
A0DDA6A72A6162F500EBD6AF /* AdyenSessionResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0DDA6A62A6162F500EBD6AF /* AdyenSessionResult.swift */; };
Expand Down Expand Up @@ -1521,7 +1521,7 @@
A0D48FB727109B0200C0B82C /* ArrayHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArrayHelpers.swift; sourceTree = "<group>"; };
A0DB48652AFD020400348C83 /* AdyenAnalyticsRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalyticsRequest.swift; sourceTree = "<group>"; };
A0DB48672AFD068400348C83 /* AdyenAnalytics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalytics.swift; sourceTree = "<group>"; };
A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsEvent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalyticsEvent.swift; sourceTree = "<group>"; };
A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsInfo.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalyticsInfo.swift; sourceTree = "<group>"; };
A0DB486B2AFD0BEE00348C83 /* AdyenAnalyticsLog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalyticsLog.swift; sourceTree = "<group>"; };
A0DB486D2AFD0BFC00348C83 /* AdyenAnalyticsError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenAnalyticsError.swift; sourceTree = "<group>"; };
A0DDA6A62A6162F500EBD6AF /* AdyenSessionResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdyenSessionResult.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3130,7 +3130,7 @@
children = (
C9B6683627C8D7FB006950B9 /* TelemetryData.swift */,
A0DB48672AFD068400348C83 /* AdyenAnalytics.swift */,
A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsEvent.swift */,
A0DB48692AFD0BDC00348C83 /* AdyenAnalyticsInfo.swift */,
A0DB486B2AFD0BEE00348C83 /* AdyenAnalyticsLog.swift */,
A0DB486D2AFD0BFC00348C83 /* AdyenAnalyticsError.swift */,
);
Expand Down Expand Up @@ -6662,7 +6662,7 @@
E2C0E096220B04F0008616F6 /* FormViewController.swift in Sources */,
F9FE25D62626CD49001874BB /* ReadyToSubmitPaymentComponentDelegate.swift in Sources */,
F99D4556262717A500880D72 /* FormErrorItemView.swift in Sources */,
A0DB486A2AFD0BDC00348C83 /* AdyenAnalyticsEvent.swift in Sources */,
A0DB486A2AFD0BDC00348C83 /* AdyenAnalyticsInfo.swift in Sources */,
F96286BD256BDEB000043FE3 /* CoreBundleExtension.swift in Sources */,
E7085D7D262EEF6200D0153B /* AllRegions.swift in Sources */,
F919DF9124E682480027976E /* ClientKeyResponse.swift in Sources */,
Expand Down
20 changes: 8 additions & 12 deletions Adyen/Analytics/AnalyticsProvider/AnalyticsProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,10 @@ public struct AdditionalAnalyticsFields {
}

@_spi(AdyenInternal)
public protocol InitialTelemetryProtocol {
public protocol AnalyticsProviderProtocol {

/// Sends the initial data and retrieves the checkout attempt id as a response.
func fetchCheckoutAttemptId(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?)
}

@_spi(AdyenInternal)
public protocol AnalyticsProviderProtocol: InitialTelemetryProtocol {
func sendInitialAnalytics(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?)

var checkoutAttemptId: String? { get }
}
Expand All @@ -65,7 +61,7 @@ internal final class AnalyticsProvider: AnalyticsProviderProtocol {

private var batchTimer: Timer?

private var events: [AdyenAnalytics.Event] = []
private var infos: [AdyenAnalytics.Info] = []
private var logs: [AdyenAnalytics.Log] = []
private var errors: [AdyenAnalytics.Error] = []

Expand All @@ -82,7 +78,7 @@ internal final class AnalyticsProvider: AnalyticsProviderProtocol {

// MARK: - Internal

internal func fetchCheckoutAttemptId(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?) {
internal func sendInitialAnalytics(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?) {
guard configuration.isEnabled, configuration.isTelemetryEnabled else {
checkoutAttemptId = "do-not-track"
return
Expand All @@ -105,8 +101,8 @@ internal final class AnalyticsProvider: AnalyticsProviderProtocol {
}
}

internal func send(event: AdyenAnalytics.Event) {
events.append(event)
internal func send(info: AdyenAnalytics.Info) {
infos.append(info)
}

internal func send(log: AdyenAnalytics.Log) {
Expand All @@ -129,7 +125,7 @@ internal final class AnalyticsProvider: AnalyticsProviderProtocol {
let checkoutAttemptId else { return }
var request = AdyenAnalyticsRequest(checkoutAttemptId: checkoutAttemptId)

request.events = events
request.infos = infos
request.logs = logs
request.errors = errors

Expand All @@ -140,7 +136,7 @@ internal final class AnalyticsProvider: AnalyticsProviderProtocol {
}

private func clearAll() {
events = []
infos = []
logs = []
errors = []
}
Expand Down
5 changes: 5 additions & 0 deletions Adyen/Analytics/Models/AdyenAnalytics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ public final class AdyenAnalytics {
internal protocol AdyenAnalyticsCommonFields: Encodable {
var commonFields: AdyenAnalytics.CommonFields { get }
}

//@_spi(AdyenInternal)
//public protocol AdyenAnalyticEvent {
// var commonFields: AdyenAnalytics.CommonFields { get }
//}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import Foundation

extension AdyenAnalytics {

/// Represents an event in the analytics scheme that can occur
/// Represents an info event in the analytics scheme that can occur
/// many times during the checkout flow, such as input field focus/unfocus etc.
internal struct Event: AdyenAnalyticsCommonFields {
internal struct Info: AdyenAnalyticsCommonFields {

internal var commonFields: AdyenAnalytics.CommonFields

internal var type: EventType
internal var type: InfoType

internal var target: String

Expand All @@ -27,7 +27,7 @@ extension AdyenAnalytics {
internal var validationErrorMessage: String
}

internal enum EventType: String, Encodable {
internal enum InfoType: String, Encodable {
case selected = "Selected"
case focus = "Focus"
case unfocus = "Unfocus"
Expand Down
4 changes: 2 additions & 2 deletions Adyen/Analytics/Requests/AdyenAnalyticsRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal struct AdyenAnalyticsRequest: APIRequest {

internal var channel: String = "iOS"

internal var events: [AdyenAnalytics.Event] = []
internal var infos: [AdyenAnalytics.Info] = []

internal var logs: [AdyenAnalytics.Log] = []

Expand All @@ -37,7 +37,7 @@ internal struct AdyenAnalyticsRequest: APIRequest {

private enum CodingKeys: String, CodingKey {
case channel
case events
case infos = "info"
case logs
case errors

Expand Down
3 changes: 2 additions & 1 deletion Adyen/Core/Components/InstantPaymentComponent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public final class InstantPaymentComponent: PaymentComponent {

/// Generate the payment details and invoke PaymentsComponentDelegate method.
public func initiatePayment() {
// we can attempt to fetch the checkoutAttemptId but it won't be ready for the payment
// We are not attempting to fetch the checkoutAttemptId as it won't be ready for the payment
// and we don't want to block it for an analytics call.
submit(data: paymentData)
}
}
Expand Down
2 changes: 1 addition & 1 deletion Adyen/Core/Core Protocols/PresentableComponent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ extension TrackableComponent where Self: PaymentMethodAware {
let flavor: TelemetryFlavor = _isDropIn ? .dropInComponent : .components(type: paymentMethod.type)
let amount = context.payment?.amount
let additionalFields = AdditionalAnalyticsFields(amount: amount, sessionId: AdyenAnalytics.sessionId)
context.analyticsProvider.fetchCheckoutAttemptId(with: flavor,
context.analyticsProvider.sendInitialAnalytics(with: flavor,
additionalFields: additionalFields)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal class BACSDirectDebitComponentTracker: BACSDirectDebitComponentTrackerP
let flavor: TelemetryFlavor = isDropIn ? .dropInComponent : .components(type: paymentMethod.type)
let amount = context.payment?.amount
let additionalFields = AdditionalAnalyticsFields(amount: amount, sessionId: AdyenAnalytics.sessionId)
context.analyticsProvider.fetchCheckoutAttemptId(with: flavor,
context.analyticsProvider.sendInitialAnalytics(with: flavor,
additionalFields: additionalFields)
}

Expand Down
2 changes: 1 addition & 1 deletion AdyenDropIn/DropInComponentExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ extension DropInComponent: PaymentMethodListComponentDelegate {
let flavor = TelemetryFlavor.dropIn(paymentMethods: paymentMethodTypes)
let amount = context.payment?.amount
let additionalFields = AdditionalAnalyticsFields(amount: amount, sessionId: AdyenAnalytics.sessionId)
context.analyticsProvider.fetchCheckoutAttemptId(with: flavor, additionalFields: additionalFields)
context.analyticsProvider.sendInitialAnalytics(with: flavor, additionalFields: additionalFields)
}

internal func didSelect(_ component: PaymentComponent,
Expand Down
1 change: 0 additions & 1 deletion AdyenSession/AdyenSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public final class AdyenSession {
sessionContext: sessionContext)
session.delegate = delegate
session.presentationDelegate = presentationDelegate
// Unfortunately needed by telemetry and this is the only way atm.
AdyenAnalytics.sessionId = sessionContext.identifier
completion(.success(session))
case let .failure(error):
Expand Down
2 changes: 1 addition & 1 deletion Tests/Adyen Tests/Analytics/AnalyticsProviderMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class AnalyticsProviderMock: AnalyticsProviderProtocol {

// MARK: - checkoutAttemptId

func fetchCheckoutAttemptId(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?) {
func sendInitialAnalytics(with flavor: TelemetryFlavor, additionalFields: AdditionalAnalyticsFields?) {
initialTelemetryEventCallsCount += 1
checkoutAttemptId = _checkoutAttemptId
}
Expand Down
16 changes: 8 additions & 8 deletions Tests/Adyen Tests/Analytics/AnalyticsProviderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class AnalyticsProviderTests: XCTestCase {
let fetchCheckoutAttemptIdExpection = expectation(description: "checkoutAttemptId completion")

// When
sut.fetchCheckoutAttemptId(with: .components(type: .achDirectDebit), additionalFields: nil)
sut.sendInitialAnalytics(with: .components(type: .achDirectDebit), additionalFields: nil)
fetchCheckoutAttemptIdExpection.fulfill()

wait(for: .milliseconds(200))
Expand All @@ -58,7 +58,7 @@ class AnalyticsProviderTests: XCTestCase {
let sut = AnalyticsProvider(apiClient: apiClient, configuration: analyticsConfiguration)

// When
sut.fetchCheckoutAttemptId(with: .components(type: .affirm), additionalFields: nil)
sut.sendInitialAnalytics(with: .components(type: .affirm), additionalFields: nil)
XCTAssertEqual(sut.checkoutAttemptId, "do-not-track")
}

Expand All @@ -79,7 +79,7 @@ class AnalyticsProviderTests: XCTestCase {
let fetchCheckoutAttemptIdExpection = expectation(description: "checkoutAttemptId completion")

// When
sut.fetchCheckoutAttemptId(with: .components(type: .achDirectDebit), additionalFields: nil)
sut.sendInitialAnalytics(with: .components(type: .achDirectDebit), additionalFields: nil)
fetchCheckoutAttemptIdExpection.fulfill()

wait(for: .milliseconds(200))
Expand All @@ -104,7 +104,7 @@ class AnalyticsProviderTests: XCTestCase {
apiClient.mockedResults = [checkoutAttemptIdResult]

// When
sut.fetchCheckoutAttemptId(with: .dropInComponent, additionalFields: nil)
sut.sendInitialAnalytics(with: .dropInComponent, additionalFields: nil)
// Then
XCTAssertNil(sut.checkoutAttemptId, "The checkoutAttemptId is not nil.")
}
Expand All @@ -126,7 +126,7 @@ class AnalyticsProviderTests: XCTestCase {
let fetchCheckoutAttemptIdExpection = expectation(description: "checkoutAttemptId completion")

// When
sut.fetchCheckoutAttemptId(with: .components(type: .atome), additionalFields: nil)
sut.sendInitialAnalytics(with: .components(type: .atome), additionalFields: nil)
fetchCheckoutAttemptIdExpection.fulfill()

wait(for: .milliseconds(200))
Expand All @@ -150,7 +150,7 @@ class AnalyticsProviderTests: XCTestCase {
apiClient.mockedResults = [checkoutAttemptIdResult]

// When
sut.fetchCheckoutAttemptId(with: .components(type: .affirm), additionalFields: nil)
sut.sendInitialAnalytics(with: .components(type: .affirm), additionalFields: nil)
// Then
XCTAssertEqual(sut.checkoutAttemptId, "do-not-track")
}
Expand Down Expand Up @@ -182,7 +182,7 @@ class AnalyticsProviderTests: XCTestCase {

// When

analyticsProvider.fetchCheckoutAttemptId(with: .components(type: .achDirectDebit), additionalFields: nil)
analyticsProvider.sendInitialAnalytics(with: .components(type: .achDirectDebit), additionalFields: nil)

wait(for: [telemetryExpectation], timeout: 10)
}
Expand Down Expand Up @@ -219,7 +219,7 @@ class AnalyticsProviderTests: XCTestCase {

// When
let additionalFields = AdditionalAnalyticsFields(amount: amount)
analyticsProvider.fetchCheckoutAttemptId(with: .components(type: .achDirectDebit), additionalFields: additionalFields)
analyticsProvider.sendInitialAnalytics(with: .components(type: .achDirectDebit), additionalFields: additionalFields)

wait(for: [telemetryExpectation], timeout: 10)
}
Expand Down
7 changes: 5 additions & 2 deletions Tests/Adyen Tests/Analytics/TelemetryTrackerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import XCTest
class TelemetryTrackerTests: XCTestCase {

var apiClient: APIClientMock!
var sut: InitialTelemetryProtocol!
var sut: AnalyticsProviderProtocol!

override func setUpWithError() throws {
try super.setUpWithError()
Expand All @@ -31,7 +31,7 @@ class TelemetryTrackerTests: XCTestCase {
}

private func sendInitialTelemetry(flavor: TelemetryFlavor = .components(type: .achDirectDebit)) {
sut.fetchCheckoutAttemptId(with: flavor, additionalFields: nil)
sut.sendInitialAnalytics(with: flavor, additionalFields: nil)
}

func testSendTelemetryEventGivenAnalyticsIsDisabledAndTelemetryIsEnabledShouldNotSendAnyRequest() throws {
Expand Down Expand Up @@ -63,6 +63,7 @@ class TelemetryTrackerTests: XCTestCase {

// Then
XCTAssertEqual(expectedRequestCalls, apiClient.counter, "One or more telemetry requests were sent.")
XCTAssertEqual(sut.checkoutAttemptId, "do-not-track")
}

func testSendTelemetryEventGivenTelemetryIsEnabledAndFlavorIsDropInComponentShouldNotSendAnyRequest() throws {
Expand All @@ -79,6 +80,7 @@ class TelemetryTrackerTests: XCTestCase {

// Then
XCTAssertEqual(expectedRequestCalls, apiClient.counter, "One or more telemetry requests were sent.")
XCTAssertNil(sut.checkoutAttemptId)
}

func testSendTelemetryEventGivenTelemetryIsEnabledAndFlavorIsComponentsShouldSendTelemetryRequest() throws {
Expand All @@ -99,6 +101,7 @@ class TelemetryTrackerTests: XCTestCase {
// Then
wait(for: .milliseconds(1))
XCTAssertEqual(expectedRequestCalls, apiClient.counter, "Invalid request number made.")
XCTAssertEqual(sut.checkoutAttemptId, "cb3eef98-978e-4f6f-b299-937a4450be1f1648546838056be73d8f38ee8bcc3a65ec14e41b037a59f255dcd9e83afe8c06bd3e7abcad993")
}

func testSendTelemetryEventGivenTelemetryIsEnabledAndFlavorIsDropInShouldSendTelemetryRequest() throws {
Expand Down
6 changes: 2 additions & 4 deletions Tests/Adyen Tests/Core/AdyenContextTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ class AdyenContextTests: XCTestCase {
payment: .init(amount: oneEUR, countryCode: "NL")
)

// XCTAssertEqual(context.analyticsProvider.additionalFields?().amount, oneEUR)

XCTAssertEqual(context.payment?.amount, oneEUR)
context.update(payment: Payment(amount: twoEUR, countryCode: "NL"))

// XCTAssertEqual(context.analyticsProvider.additionalFields?().amount, twoEUR)
XCTAssertEqual(context.payment?.amount, twoEUR)
}
}

0 comments on commit 842fb79

Please sign in to comment.