Skip to content

Fix new-stats Views card bottom row via dedicated stats/visits call#23098

Open
adalpari wants to merge 6 commits into
trunkfrom
fix/cmm-2152-newstats-visitors-dedicated-call
Open

Fix new-stats Views card bottom row via dedicated stats/visits call#23098
adalpari wants to merge 6 commits into
trunkfrom
fix/cmm-2152-newstats-visitors-dedicated-call

Conversation

@adalpari

@adalpari adalpari commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Fixes CMM-2152.

NOTE: CI will fail until we merge the changes in WordPress-rs and target it.

Problem

On the new stats screen, the Views card bottom row (Views, Visitors, Likes, Comments, Posts) was derived by summing the chart's per-bucket data points. For Visitors this is wrong: uniques are counted once per bucket, so summing daily buckets over a long range over-counts them, and the single-day case could misreport the total.

Fix

Fill the bottom row with a dedicated stats/visits call tuned per range, so visitor uniques de-duplicate server-side per bucket before summing.

  • Unit selection: day for ≤31 days, month up to ~2 years (731d), year beyond. A single day uses unit=day&quantity=1.
  • Always a dedicated call: the bottom row is fetched from its own stats/visits calls (current + previous) for every period, independently of the chart, restricted to the bottom-row stat_fields.
  • Independent loading: the chart (2 calls) and the bottom row (2 calls) load from separate requests and render on their own; either region can fail without affecting the other.
  • Hide on failure: if a bottom-row call errors or throws, the row is hidden. A successful-but-empty response still shows a legitimate all-zero row.

Views/Likes/Comments/Posts are additive, so their totals are unchanged; only Visitors is affected.

Known tradeoffs / follow-ups

  • Visitor de-dup granularity (follow-up PR): de-duplication happens per bucket on the server, so summing multiple buckets still over-counts uniques across buckets. This PR fixes the single-day/0 case and reduces over-counting via coarser units on long ranges, but does not fully de-duplicate visitors for multi-bucket ranges (e.g. Last 7 Days sums 7 daily buckets). Fine-grained per-range visitor de-duplication is intentionally deferred to a separate PR.
  • Dedicated calls even when the unit matches the chart (intentional): for periods where the bottom-row unit/quantity equal the chart's (Last 7/30 Days, Last 6/12 Months), the dedicated call duplicates the chart's request. This is deliberate — it keeps the chart and bottom-row fetches fully independent so one can load or fail without blocking or corrupting the other, rather than coupling them by reusing chart aggregates.

Dependency

Requires wordpress-rs with start_date/stat_fields on StatsVisitsParams. gradle/libs.versions.toml is bumped to 1434-e7e17e7168b28116f3c7975ac8e20757259589cb (wordpress-rs PR #1434).

Testing

  1. Open the new stats screen -> Views Card
  • Verify it works as expected.
    Note: there could be some incongruences with "Today's card" or between the chart and the bottom row for one day selections. I'll work on polishing that and be sure it's behaving like the web in a different PR as this one is big enough.

🤖 Generated with Claude Code

The Views card bottom row (Views, Visitors, Likes, Comments, Posts) was
derived by summing the chart's per-bucket data points. For visitors this
over-counts on long ranges (uniques counted once per bucket are summed)
and could misreport the single-day total.

Instead fill the bottom row with a dedicated stats/visits call tuned per
range so visitor uniques de-duplicate server-side per bucket before
summing:
- Unit selection: day for <=31 days, month up to ~2 years, year beyond;
  single day uses unit=day quantity=1.
- The dedicated call fires only when its unit/quantity differ from the
  chart's; otherwise the chart aggregates are reused to avoid redundant
  network requests. Falls back to chart aggregates if the call fails.

Requires wordpress-rs with start_date/stat_fields support on
StatsVisitsParams (bumped to 1434-e7e17e7).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@adalpari adalpari requested a review from a team as a code owner July 14, 2026 09:44
@adalpari adalpari requested review from nbradbury and removed request for a team July 14, 2026 09:44
@dangermattic

dangermattic commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator
1 Error
🚫 This PR includes a PR version of wordpress-rs (1434-e7e17e7168b28116f3c7975ac8e20757259589cb). Please merge the corresponding wordpress-rs PR and update to a trunk version before merging.
2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@adalpari adalpari removed the request for review from nbradbury July 14, 2026 09:46
@wpmobilebot

Copy link
Copy Markdown
Contributor

Project dependencies changes

list
! Upgraded Dependencies
net.java.dev.jna:jna:5.19.1, (changed from 5.19.0)
rs.wordpress.api:android:1434-e7e17e7168b28116f3c7975ac8e20757259589cb, (changed from 0.5.0)
rs.wordpress.api:kotlin:1434-e7e17e7168b28116f3c7975ac8e20757259589cb, (changed from 0.5.0)
tree
 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:0.5.0
-|         +--- com.squareup.okhttp3:okhttp:5.4.0 (*)
-|         +--- com.squareup.okhttp3:okhttp-tls:5.4.0
-|         |    +--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
-|         |    +--- com.squareup.okio:okio:3.17.0 (*)
-|         |    \--- com.squareup.okhttp3:okhttp:5.4.0 (*)
-|         +--- net.java.dev.jna:jna:5.19.0
-|         +--- rs.wordpress.api:kotlin:0.5.0
-|         |    +--- com.squareup.okhttp3:okhttp:5.4.0 (*)
-|         |    +--- com.squareup.okhttp3:okhttp-tls:5.4.0 (*)
-|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 -> 1.11.0 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
-|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
+|    \--- rs.wordpress.api:android:1434-e7e17e7168b28116f3c7975ac8e20757259589cb
+|         +--- com.squareup.okhttp3:okhttp:5.4.0 (*)
+|         +--- com.squareup.okhttp3:okhttp-tls:5.4.0
+|         |    +--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
+|         |    +--- com.squareup.okio:okio:3.17.0 (*)
+|         |    \--- com.squareup.okhttp3:okhttp:5.4.0 (*)
+|         +--- net.java.dev.jna:jna:5.19.1
+|         +--- rs.wordpress.api:kotlin:1434-e7e17e7168b28116f3c7975ac8e20757259589cb
+|         |    +--- com.squareup.okhttp3:okhttp:5.4.0 (*)
+|         |    +--- com.squareup.okhttp3:okhttp-tls:5.4.0 (*)
+|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 -> 1.11.0 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
-+--- rs.wordpress.api:android:0.5.0 (*)
++--- rs.wordpress.api:android:1434-e7e17e7168b28116f3c7975ac8e20757259589cb (*)
 \--- com.automattic:encryptedlogging:1.1.1
-     \--- net.java.dev.jna:jna:5.17.0 -> 5.19.0
+     \--- net.java.dev.jna:jna:5.17.0 -> 5.19.1

@wpmobilebot

wpmobilebot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr23098-934349e
Build Number1498
Application IDcom.jetpack.android.prealpha
Commit934349e
Installation URL4t001hd5n2mag
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr23098-934349e
Build Number1498
Application IDorg.wordpress.android.prealpha
Commit934349e
Installation URL57q5fik3q8lg8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

adalpari and others added 3 commits July 14, 2026 12:34
Address code-review findings on the new-stats Views card bottom row:

- Always fire the dedicated bottom-stats call for every period instead of
  reusing the chart aggregates when the coarser unit matched the chart's.
- Hide the bottom row when the dedicated call errors, throws, or returns an
  empty response, rather than falling back to chart aggregates or showing
  zeros. The bottom fetch now swallows exceptions (rethrowing cancellation)
  so it can't cancel the sibling chart fetches or fail the whole card.
- Fix the >2-year (YEAR unit) previous-window skew: the previous window now
  mirrors the current window's exact day span for all units instead of being
  padded out to full calendar years, keeping the change comparison symmetric.

Bottom aggregates are now nullable end-to-end (repository -> ViewModel ->
UI state), and the card omits BottomStatsRow when they're absent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Split the Views card fetch so the chart (2 calls) and the bottom-stats
row (2 dedicated calls) load from separate, independent requests instead
of one blocking fetch of all four:

- The chart no longer waits on the bottom-row calls, and each region
  updates on its own as it returns (own placeholder/spinner).
- Either region can fail without hiding the other: a failed chart shows
  a compact retry while the bottom row stays; a failed bottom call hides
  only the row.
- A successful-but-empty bottom response now shows a zero-valued row
  instead of hiding it (only errors hide the row).
- Bottom totals use a dateless BottomStatsAggregates, dropping the
  unused date plumbing; the current-window logic is unified via
  currentPeriodWindow().

Reworks ViewsStatsCardUiState into Content(chart, bottomStats) with
independent ChartUiState and BottomStatsUiState, and updates tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@adalpari adalpari requested a review from nbradbury July 14, 2026 11:59
@nbradbury

Copy link
Copy Markdown
Contributor

@adalpari Claude found some issues, and at least the first one sounds like it needs to be addressed.

review-pr23098-newstats-visitors-2026-07-14.pdf

adalpari and others added 2 commits July 14, 2026 15:47
Keep the card dimmed until both the chart and the dedicated bottom-stats
call finish, so a drill-down no longer un-dims the old period's totals as
if they belonged to the new period. Also treat a period as loaded only
when both regions succeed, so a transient bottom-stats failure retries on
next visibility instead of hiding the row permanently.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Delete the private rememberShimmerBrush() copy in ViewsStatsCard and use
the shared util helper, which honors LocalShimmerBrush so the Views card
shimmer syncs with the other stats cards instead of spinning up its own
infinite transitions.

Extract unitAndQuantityFor() and previousWindowMirror() in StatsRepository
so calculateBottomStatsRange and calculateCustomPeriodDates share the
day/month threshold, the MONTHS.between + 1 quantity, and the exact
day-span previous-window mirror. The bottom-row-only YEAR tier is gated
behind an allowYear flag.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@adalpari

Copy link
Copy Markdown
Contributor Author

@adalpari Claude found some issues, and at least the first one sounds like it needs to be addressed.

review-pr23098-newstats-visitors-2026-07-14.pdf

Yup, some of them are expected, and I'm fixing that in a subsequent PR because Claude was driving me crazy. I left a note in the PR description for that, but it seems like Claude kept going around it.

  1. I'm improving this in a new PR in parallel to the mentioned problem
  2. Fixed
  3. I don't think it's applicable
  4. Will go away in the new PR changes
  5. Fixed

I've also done some of the suggested cleanups

Thanks!

@nbradbury nbradbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Feel free to :shipit: when the wp-rs dependency is updated.

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.

4 participants