Skip to content

CMM-2197: Make Posts & Pages items in new stats clickable - #23171

Open
nbradbury wants to merge 8 commits into
trunkfrom
issue/CMM-2197-new-stats-clickable-posts
Open

CMM-2197: Make Posts & Pages items in new stats clickable#23171
nbradbury wants to merge 8 commits into
trunkfrom
issue/CMM-2197-new-stats-clickable-posts

Conversation

@nbradbury

@nbradbury nbradbury commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes CMM-2197

In the new stats, items in the Posts & Pages card aren't tappable, unlike old stats. This PR adds the previous behavior to the new stats.

Note that we use the old per-post stats screen here. We'll want to add a new screen for this which matches the new UI, but at the moment there's no per-post stats endpoint in wordpress-rs so we stick with the old screen. This isn't ideal, but it's better than not having the feature at all.

Testing instructions

  1. Open the Traffic tab in new stats
  2. Tap a post in the Posts & Pages card
  • The post's detail stats screen opens with the correct title and data
  1. Go back, tap "Show all" on Posts & Pages, then tap a post there
  • The same detail screen opens

Note: This is currently failing CI but not due to changes in this PR. I'll rerun checks once that gets resolved.

Carries url/postType through from the wordpress-rs top-posts response and
opens the existing StatsDetailActivity when a post row is tapped on the
card or the Show All screen, matching old stats behavior (including the
STATS_POSTS_AND_PAGES_ITEM_TAPPED analytics event).
@nbradbury nbradbury added the Stats label Aug 3, 2026
@dangermattic

dangermattic commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator
1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Aug 3, 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
Versionpr23171-5bfab63
Build Number1498
Application IDorg.wordpress.android.prealpha
Commit5bfab63
Installation URL2a5lsrpl8vdlo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Aug 3, 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
Versionpr23171-5bfab63
Build Number1498
Application IDcom.jetpack.android.prealpha
Commit5bfab63
Installation URL244eti9l9u3qg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.09677% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.84%. Comparing base (6d9ff29) to head (6e1a3db).
⚠️ Report is 1 commits behind head on trunk.

Files with missing lines Patch % Lines
.../wordpress/android/ui/newstats/NewStatsActivity.kt 0.00% 12 Missing ⚠️
...java/org/wordpress/android/ui/ActivityNavigator.kt 23.07% 10 Missing ⚠️
...s/android/ui/newstats/mostviewed/MostViewedCard.kt 0.00% 5 Missing ⚠️
...newstats/mostviewed/MostViewedCommonComposables.kt 0.00% 5 Missing ⚠️
...ui/newstats/mostviewed/MostViewedDetailActivity.kt 0.00% 4 Missing ⚠️
...roid/ui/newstats/datasource/StatsDataSourceImpl.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #23171      +/-   ##
==========================================
- Coverage   37.84%   37.84%   -0.01%     
==========================================
  Files        2345     2345              
  Lines      127530   127573      +43     
  Branches    17716    17720       +4     
==========================================
+ Hits        48258    48274      +16     
- Misses      75313    75340      +27     
  Partials     3959     3959              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nbradbury
nbradbury marked this pull request as ready for review August 3, 2026 18:51
@nbradbury
nbradbury requested review from jkmassel and oguzkocer August 3, 2026 18:52
Collapses the duplicated post-tap navigation in NewStatsActivity and
MostViewedDetailActivity into one ActivityNavigator method, and moves the
legacy statsDetailItemType mapping out of the new-stats UI-state layer to
the navigation boundary.
ActivityNavigator has a @singleton @Inject constructor, so Hilt constructs
it directly. The unscoped @provides also shadowed the class-level @singleton,
handing out a new instance per injection site.
@nbradbury
nbradbury requested a review from dcalhoun August 4, 2026 15:50
@wpmobilebot

wpmobilebot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

@oguzkocer oguzkocer 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.

Worked as expected. I left a couple notes, but nothing blocking the merge. :shipit:

* No-ops when no site is selected.
*/
fun openPostDetailStats(context: Context, postId: Long, postType: String?, postTitle: String, postUrl: String?) {
val site = selectedSiteRepository.getSelectedSite() ?: return

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.

I don't feel strongly about this, but it looks like we are showing a toast when a site is missing in many other cases, so maybe worth adding here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call - I'll add this.

*/
fun openPostDetailStats(context: Context, postId: Long, postType: String?, postTitle: String, postUrl: String?) {
val site = selectedSiteRepository.getSelectedSite() ?: return
AnalyticsTracker.track(AnalyticsTracker.Stat.STATS_POSTS_AND_PAGES_ITEM_TAPPED)

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.

It looks like we were doing this slightly differently with trackGranular here. Not sure if that's available to us, but thought I'd mention.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, the new stats periods (Today / Last 7 Days / Last 30 Days / 6–12 Months / custom) don’t map cleanly onto the old StatsGranularity taxonomy, so the trackGranular property would mislabel events.

Addresses review feedback: matches the openMediaInNewStack pattern instead
of silently ignoring the tap.
@nbradbury
nbradbury enabled auto-merge (squash) August 4, 2026 20:33
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.

4 participants