Skip to content

Card fields analytics#1817

Merged
buzzamus merged 9 commits into
card-fields-featurefrom
card-fields-analytics
Jun 11, 2026
Merged

Card fields analytics#1817
buzzamus merged 9 commits into
card-fields-featurefrom
card-fields-analytics

Conversation

@buzzamus

@buzzamus buzzamus commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary of changes

  • Added 2 new analytic events for tracking CardFields when they render on screen and when the tokenize request is started. Verified the events triggered and carry same session ID throughout

AI Usage

Which AI Agent Was Used?

  • Copilot
  • Claude
  • Other (Type Name Here)

How was AI used?
N/A: I coded this like our ancient forefathers did:

Estimated AI Code Contribution

  • less than 30%
  • 30 - 60%
  • 60 - 100%

Checklist

  • Added a changelog entry
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

List GitHub usernames for everyone who contributed to this pull request.

@buzzamus buzzamus requested a review from a team as a code owner June 8, 2026 18:37
func tokenize() {
guard isFormValid else { return }

sendAnalyticsEvent(UIComponentsAnalytics.cardFieldsSelected)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: could we callapiClient.sendAnalyticsEvent() directly here instead of going through the helper 👀


// MARK: - Private Properties

var apiClient: BTAPIClient

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this need to be private 👀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah yep! Good call thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually sorry it is exposed for testing like the current pattern for the other clients. I moved and added docstring to follow existing pattern

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Your comment and Jax's below sort of intersected, so ended up changing this up

final class CardFieldsViewModelTests: XCTestCase {

private var mockAPIClient = MockAPIClient(authorization: "development_testing_tokenization_key_sandbox")
private var viewModel = CardFieldsViewModel(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: it looks like we're initializing the same on line 19

Suggested change
private var viewModel = CardFieldsViewModel(
private var viewModel = CardFieldsViewModel

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ope yep fixed!

Comment on lines +53 to +55
func sendAnalyticsEvent(_ event: String) {
apiClient.sendAnalyticsEvent(event)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we add a docstring to this? It took me a second to wrap my head around what was going on. Alternatively we could consider constructing the apiClient in CardFields and passing it as a param to the view model? That way we aren't constructing it twice which on fast connections could mean 2 config calls before it's in the cache. Then we can send both events in the same place. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep passing the client in from CardFields works! I briefly thought about going down that road early on, but I think it makes much more sense than having this weird phantom 2nd apiClient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated again to remove the apiClient directly from the viewModel. I had to create a submitAction to house the analytics and tokenize call to use it in the onvalid closure check

@agedd agedd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀

@buzzamus buzzamus merged commit 972ad96 into card-fields-feature Jun 11, 2026
14 checks passed
@buzzamus buzzamus deleted the card-fields-analytics branch June 11, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants