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

WaitingTimeTracker improvements #15082

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Feb 6, 2025

Description

This PR is a side-quest from #15047 to improve WaitingTimeTracker's readability. In that PR we reused the existing WaitingTimeTracker to capture POS events in milliseconds without realizing the actual elapsed time was being captured in seconds. With these changes we extend the class to allow both, and be more explicit about it.

Changes:

  • Updates ...InMillis property in the interface to avoid using it incorrectly by mistake, by being explicit about the time unit.
  • Updates internals to use seconds (by default) or milliseconds by choice
  • Adds additional tests

Testing information

  • Pre-existing WooAnalyticsEvent.WaitingTime.Scenario are still tracked in seconds with the key waiting_time. For example these two run as soon as we load the app:
🔵 Tracked dashboard_main_stats_waiting_time_loaded, properties: [store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, is_wpcom_store: false, blog_id: -1, plan: , site_url: https://indiemelon.mystagingwebsite.com, waiting_time: 2.065809965133667, was_ecommerce_trial: false]
🔵 Tracked dashboard_top_performers_waiting_time_loaded, properties: [store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, plan: , site_url: https://indiemelon.mystagingwebsite.com, was_ecommerce_trial: false, is_wpcom_store: false, blog_id: -1, waiting_time: 7.46925497055054]
  • In a POS eligible-store, load POS and observe the loading event is tracked in milliseconds with the key milliseconds_time_elapsed_in_splash_screen:
🔵 Tracked pos_pos_loaded, properties: [is_wpcom_store: false, blog_id: -1, milliseconds_time_elapsed_in_splash_screen: 2152.8499126434326, site_url: https://indiemelon.mystagingwebsite.com, was_ecommerce_trial: false, plan: , store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f]

Tested in simulator iPad Air 11 iOS 17.5


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

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@iamgabrielma iamgabrielma added type: enhancement A request for an enhancement. category: tracks Related to analytics, including Tracks Events. labels Feb 6, 2025
@iamgabrielma iamgabrielma added this to the 21.8 milestone Feb 6, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review February 6, 2025 14:22
@dangermattic
Copy link
Collaborator

dangermattic commented Feb 6, 2025

1 Warning
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

@iamgabrielma iamgabrielma requested review from jaclync and ThomazFB and removed request for ThomazFB February 6, 2025 14:23
@iamgabrielma
Copy link
Contributor Author

Only one review is needed. cc: @ThomazFB in case I missed something here 👀

@wpmobilebot
Copy link
Collaborator

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr15082-6de1ef0
Version21.6
Bundle IDcom.automattic.alpha.woocommerce
Commit6de1ef0
App Center BuildWooCommerce - Prototype Builds #12844
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

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: enhancement A request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants