WordPress-iOS icon indicating copy to clipboard operation
WordPress-iOS copied to clipboard

Stats Revamp: Fix Period Store State when navigating between 1st and 2nd levels

Open sla8c opened this issue 2 years ago • 2 comments

This PR adds functionality so that when a user navigates to the main Insights Screen the PeriodStore will be set to .day granularity and a fresh call will be made for its data state.

Fixes #18983

This is quickest win solution for now. What is occurring is:

  • PeriodStore is a singleton
  • When tapping any of the Period stats such as Day / Week / Month / Year, the singleton PeriodStore internally holds the state of the interval selected such as Month or Year
  • When navigating back to the main insights screen, the ViewsVisitors chart reads from PeriodStore again however PeriodStore data is set for 'Year' or 'Month' ie whatever was selected

I tried another approach where I created a 2nd instance of PeriodStore to be used exclusively by the top level insight screen but this didn't work because the Flux events for PeriodStore are still the same so both PeriodStore still receive the same Flux events and are left in the same state.

In the future depending on what decisions are taken with Period Stats (Days / Weeks / Months) screens this could be further optimised and an enhanced Stats DataStore that accommodates mixed period scenarios could be worked on.

To test:

  1. enable both of the new stats feature flags statsNewAppearance and statsNewInsights via AppConfiguration.showsStatsRevampV2
  2. From top level Stats Insights screen take note of the Views / Visitors count
  3. Tap on one of the Period stats tabs such as Weeks, Months, Years.
  4. Navigate back to the insights tab. The Views / Visitors count should be the same as in step 2
  5. Switch sites and continue to test this for other sites

Regression Notes

  1. Potential unintended areas of impact2. Stats Insights

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

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

PR submission checklist:

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

sla8c avatar Jul 07 '22 11:07 sla8c

You can test the changes in WordPress from this Pull Request by:

  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19003-01872d0 on your iPhone
If you need access to App Center, please ask a maintainer to add you.

wpmobilebot avatar Jul 07 '22 12:07 wpmobilebot

You can test the changes in Jetpack from this Pull Request by:

  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19003-01872d0 on your iPhone
If you need access to App Center, please ask a maintainer to add you.

wpmobilebot avatar Jul 07 '22 12:07 wpmobilebot

I've removed myself as a reviewer now that @staskus is reviewing this. Please let me know though if I can help out with this or other PR reviews.

guarani avatar Sep 26 '22 19:09 guarani