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

Fix application_store_snapshot event not showing up in Tracks #10575

Merged

Conversation

jaclync
Copy link
Contributor

@jaclync jaclync commented Aug 30, 2023

Part of #10574

Why

Even though we implemented the Tracks event application_store_snapshot in #10484, we didn't see any events in Tracks after a day of release.

After a deeper look, I found a silent validation error on the event properties because the property name can only contain alpha characters and underscores while the payment gateway properties have -: Custom properties dictionary keys must contain alpha characters and underscores only.

Validation errors like this aren’t logged to the console at all, and were missed during development/review. I'm also looking into the validation error logging issue in Automattic/Automattic-Tracks-iOS#262.

How

The event properties are fixed by replacing - with _ in payment gateway event properties in the snapshot event.

Testing instructions

  • Reinstall the app, or log in to a store whose snapshot hasn't been tracked
  • Log in and connect to a store --> shortly after entering the site, an event is logged in the console 🔵 Tracked application_store_snapshot AND it should also get sent in the Tracks POST request after a bit. You can check the Tracks POST request body in the Menu tab > Settings > Launch Wormholy Debug, and search for tracks/record POST requests and find application_store_snapshot in the request body

  • @jaclync make sure the event shows up in Tracks

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync changed the base branch from trunk to release/15.1 August 30, 2023 04:07
@jaclync
Copy link
Contributor Author

jaclync commented Aug 30, 2023

Hi @spencertransier 👋 just a heads up that it'd be great if this PR can be merged to the release 15.1 branch when it's ready for the beta build.

@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr10575-00ff80f on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@jaclync jaclync marked this pull request as ready for review August 30, 2023 06:27
@jaclync jaclync requested review from Ecarrion and ThomazFB August 30, 2023 06:28
@jaclync jaclync added this to the 15.1 ❄️ milestone Aug 30, 2023
@jaclync jaclync added type: bug A confirmed bug. category: tracks Related to analytics, including Tracks Events. labels Aug 30, 2023
@ThomazFB ThomazFB self-assigned this Aug 30, 2023
Copy link
Contributor

@ThomazFB ThomazFB left a comment

Choose a reason for hiding this comment

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

Looks good!

@spencertransier spencertransier merged commit eceeec6 into release/15.1 Aug 30, 2023
@spencertransier spencertransier deleted the issue/10574-fix-snapshot-event-properties branch August 30, 2023 23:36
@spencertransier
Copy link
Contributor

@jaclync This fix is in the 15.1.0.2 build that'll be available in TestFlight shortly.

@jaclync
Copy link
Contributor Author

jaclync commented Aug 31, 2023

Thanks for the review @ThomazFB & release updates @spencertransier! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tracks Related to analytics, including Tracks Events. type: bug A confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants