-
Notifications
You must be signed in to change notification settings - Fork 121
[Woo POS] Modularization: refactor WaitingTimeTracker to remove Analytics dependency and move it to WooFoundation
#16169
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
[Woo POS] Modularization: refactor WaitingTimeTracker to remove Analytics dependency and move it to WooFoundation
#16169
Conversation
…t it can be moved to WooFoundation. `AppStartupWaitingTimeTracker` is now a wrapper of `WaitingTimeTracker`.
Generated by 🚫 Danger |
|
|
WaitingTimeTracker to remove Analytics dependency and move it to WooFoundation
| case .analyticsHub: | ||
| return WooAnalyticsEvent(statName: .analyticsHubWaitingTimeLoaded, properties: [Keys.waitingTime: elapsedTime]) | ||
| case .appStartup: | ||
| return WooAnalyticsEvent(statName: .applicationOpenedWaitingTimeLoaded, properties: [Keys.waitingTime: elapsedTime]) |
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.
It's likely not related to this PR, but if you open the Woo app, and after some time change the site, the elapsed time is logged from the app launch up until the site change.
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.
I tested outside this PR, and confirmed the same behavior. It seems to be from the stats and onboarding tasks are always reloaded every app launch, and the completion of these reloads is required for logging the event:
woocommerce-ios/WooCommerce/Classes/Analytics/AppStartupWaitingTimeTracker.swift
Lines 40 to 43 in 83cfd68
| // If all actions completed without any errors, send the analytics event. | |
| if startupActionsPending.isEmpty { | |
| super.end() | |
| } |
I'm guessing this event was for performance measurement (Eagle worked on it) while the stats/onboarding were always reloaded (it was the case before), then the app changed to not always reload the stats/onboarding tasks. AppStartupWaitingTimeTracker also doesn't get reset when switching stores, so the loading time could become inaccurately longer as the starting time was from app launch. But since this is pre-existing behavior, perhaps the team working on it could revisit this performance metric.

Part of WOOMOB-935
Description
Refactors
WaitingTimeTrackerto remove itsAnalyticsdependency so that it can be moved to WooFoundation for future reuse across the app and Point of Sale (POS) module. The refactoredWaitingTimeTrackeris now a pure utility class that tracks time intervals and generates analytics events, whileAppStartupWaitingTimeTrackerserves as a wrapper that handles analytics tracking.Key Changes:
WaitingTimeTrackerfromWooCommerce/Classes/Analytics/toModules/Sources/WooFoundation/Utilities/Analyticsdependency fromWaitingTimeTrackerWaitingTimeTrackernow returnsWooAnalyticsEventobjects instead of directly tracking eventsAppStartupWaitingTimeTrackerwrapsWaitingTimeTrackerand handles analytics service interactionSteps to reproduce
App Startup Waiting Time Tracking:
application_opened_waiting_time_loaded'swaiting_timeDashboard Performance Tracking:
dashboard_main_stats_waiting_time_loaded'swaiting_timedashboard_top_performers_waiting_time_loaded'swaiting_timeAnalytics Hub Tracking:
analytics_hub_waiting_time_loaded'swaiting_timeOrder Details Tracking:
order_detail_waiting_time_loaded'swaiting_timePoint of Sale Tracking:
pos_loaded'smilliseconds_time_elapsed_in_splash_screenTesting information
RELEASE-NOTES.txtif necessary.