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: Stats showing stale unique visitors count when views are updated #24356

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

pmusolino
Copy link
Contributor

Fixes #24114

While I was not able to reproduce the issue mentioned here #24114 and while it seems it's not a specific issue related to the app because of this Automattic/jetpack#42176 I noticed that the login for updating stats with mostRecentChartData was too restrictive. Even when new API data was available, the existing code would sometimes keep using old cached data due to specific date matching requirements.

So, I revised the conditional logic that now prioritizes using the newest data by simplifying the conditions and always choosing the most recent data available. In this way, we ensure that both views and unique visitors metrics update simultaneously when new data arrives from the API.

Testing

Verified that both views and unique visitors data update synchronously when new stats data is fetched. Please, also check that this change does not introduce any regression, like reintroducing this issue #19688

Regression Notes

  1. Potential unintended areas of impact

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

  3. 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)

…centChartData` and updating `currentEntryIndex`
@pmusolino pmusolino requested review from crazytonyli and kean March 28, 2025 13:59
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number26844
VersionPR #24356
Bundle IDorg.wordpress.alpha
Commitb00c570
Installation URL13hldqp5smfcg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number26844
VersionPR #24356
Bundle IDcom.jetpack.alpha
Commitb00c570
Installation URL66qs22mhnnbs8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Copy link
Contributor

@kean kean left a comment

Choose a reason for hiding this comment

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

It seems to work OK, but after reviewing this file for a good 30+ minutes, I can't tell that I understand what's going on in it.

The most problematic aspect is that mostRecentChartData and other state gets mutated inside of a method you would expect to be stateless: func overviewTableRows(). When and how often does it get called? It's really hard to tell:

Screenshot 2025-03-31 at 1 14 20 PM

@pmusolino
Copy link
Contributor Author

It seems to work OK, but after reviewing this file for a good 30+ minutes, I can't tell that I understand what's going on in it.

The most problematic aspect is that mostRecentChartData and other state gets mutated inside of a method you would expect to be stateless: func overviewTableRows(). When and how often does it get called? It's really hard to tell:

I understand your feelings, and I feel the same way. It's all so outdated that working on it and debugging something have become unnecessarily complex, without considering the use of WordPressFlux. I think it's worth considering how radical it could be to throw out the entire stats screen to completely rewrite it in SwiftUI in the future.

@pmusolino pmusolino added this pull request to the merge queue Mar 31, 2025
Merged via the queue into trunk with commit 8cc7912 Mar 31, 2025
26 checks passed
@pmusolino pmusolino deleted the issue/stats-clicks-lagging-behind-views branch March 31, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stats: clicks lagging behind views
4 participants