-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[NT-483] View manage pledge event #932
Conversation
…view-manage-pledge-event
…view-manage-pledge-event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, @cdolm92 . I left a couple of suggestions that might improve readability.
Library/Koala/Koala.swift
Outdated
@@ -216,6 +216,26 @@ public final class Koala { | |||
} | |||
} | |||
|
|||
public enum ManagePledgeMenuCTAType { | |||
case updatePledge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you alphabetize the enum?
Library/Koala/Koala.swift
Outdated
|
||
var trackingString: String { | ||
switch self { | ||
case .updatePledge: return "update_pledge" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
let properties = client.properties.last | ||
XCTAssertEqual(["Manage Pledge Option Clicked"], client.events) | ||
XCTAssertEqual("cancel_pledge", properties?["cta"] as? String) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since all the ManagePledgeOptionClicked
related tests are basically the same, here we could save some code and refactor by creating a private func
private func assertManagePledgeOptionClickedProperties(of type: Koala.ManagePledgeMenuCTAType, property: String) {
let client = MockTrackingClient()
let loggedInUser = User.template
let koala = Koala(client: client, loggedInUser: loggedInUser)
koala.trackManagePledgeOptionClicked(project: .template, managePledgeMenuCTA: type)
let properties = client.properties.last
XCTAssertEqual(["Manage Pledge Option Clicked"], client.events)
XCTAssertEqual(property, properties?["cta"] as? String)
}
And writing the tests just doing:
func testTrackManagePledgeOptionClicked_CancelPledgeSelected() {
self.assertManagePledgeOptionClickedProperties(of: .cancelPledge, property: "cancel_pledge")
}
self.menuOptionSelectedSignal.filter { $0 == .changePaymentMethod }.mapConst(.changePaymentMethod), | ||
self.menuOptionSelectedSignal.filter { $0 == .chooseAnotherReward }.mapConst(.chooseAnotherReward), | ||
self.menuOptionSelectedSignal.filter { $0 == .contactCreator }.mapConst(.contactCreator), | ||
self.menuOptionSelectedSignal.filter { $0 == .cancelPledge }.mapConst(.cancelPledge) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it's simpler if you create a function that takes a ManagePledgeAlertAction and returns a Koala.ManagePledgeMenuCTAType, using a switch statement.
This would simplify a lot this code by just using:
let managePledgeMenuType: Signal<Koala.ManagePledgeMenuCTAType, Never> = self.menuOptionSelectedSignal
.map(managePledgeMenuCTAType(for:))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing the feedback! ⭐️
📲 What
Improvements in monitoring user behavior for v1 checkout. This tracks the menu options tapped event in the manage pledge screen.
🤔 Why
We track the menu option event with the
Manage Pledge Option Clicked
. This event includes a property calledcta
which value would be the menu option tapped.✅ Acceptance criteria
Update pledge
tappedcta
value should be"update_pledge"
Change payment method
tappedcta
value should be"change_payment_method"
Choose another reward
tappedcta
value should be"choose_another_reward"
Contact creator
tappedcta
value should be"contact_creator"
Cancel pledge
tappedcta
value should be"cancel_pledge"