Skip to content

Stats Traffic: Avoid initializing Stats Traffic before displaying it #22722

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

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Feb 28, 2024

Fixes #22717

I noticed stats_period_accessed event called when opening the Insights tab, both with and without the feature flag enabled. It's caused by a combination of 2 reasons:

  • StatsTrafficDatePickerViewModel tracks stats_period_accessed event when StatsTrafficDatePickerView is initialized
  • trafficTableViewControlleris initialized whenever SiteStatsDashboardViewController loads since jetpackBanner is set

Solution

I'm choosing the easiest solution to avoid initializing trafficTableViewController when SiteStatsDashboardViewController loads by setting jetpackBanner only when

To test:

Analytics

  1. Enable Stats Traffic feature flag
  2. Open Stats
  3. Select Insights tab
  4. Leave Stats
  5. Return to Stats
  6. Confirm Insights tab is opened
  7. Confirm stats_insights_accessed is tracked and stats_period_accessed not tracked
  8. Select Traffic tab
  9. Confirm stats_period_accessed tracked

Regression - Jetpack banner works

The easiest way to test is to manually enable JetpackBrandingVisibility or just watch the video to confirm that the banner works when needed:

Simulator.Screen.Recording.-.iPhone.15.-.2024-02-28.at.17.40.23.mp4

Regression Notes

  1. Potential unintended areas of impact

Breaking Jetpack banner

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

Manual testing

  1. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@staskus staskus requested a review from guarani February 28, 2024 15:43
@staskus staskus added this to the 24.4 milestone Feb 28, 2024
@wpmobilebot
Copy link
Contributor

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22722-e6d69b7
Version24.3
Bundle IDorg.wordpress.alpha
Commite6d69b7
App Center BuildWPiOS - One-Offs #9011
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22722-e6d69b7
Version24.3
Bundle IDcom.jetpack.alpha
Commite6d69b7
App Center Buildjetpack-installable-builds #8048
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Thanks for catching and fixing this, @staskus! I like the fix because it simplifies the code (I hadn't seen earlier that trafficTableViewController was getting initialised through the call to configureTrafficTableViewController()).

@staskus staskus merged commit 628e618 into trunk Feb 29, 2024
@staskus staskus deleted the bug/22717-stats-traffic-period-accessed-event-called-when-opening-insights-tab branch February 29, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stats Traffic: stats_period_accessed event called when opening Insights tab
3 participants