Skip to content

Fix activity kit stats: switch to get_total_post_views() for per-kit view counts - #3591

Open
Piyopiyo-Kitsune wants to merge 3 commits into
trunkfrom
fix/activity-kit-stats-views-api
Open

Fix activity kit stats: switch to get_total_post_views() for per-kit view counts#3591
Piyopiyo-Kitsune wants to merge 3 commits into
trunkfrom
fix/activity-kit-stats-views-api

Conversation

@Piyopiyo-Kitsune

Copy link
Copy Markdown
Collaborator

Problem

The Activity Kit Stats page shows 0 views for all kits despite Jetpack being connected and the site receiving thousands of daily page views (confirmed via Jetpack Stats).

Root cause

get_jetpack_post_views() was calling WPCOM_Stats::get_top_posts() — which returns the top 1,000 most-viewed posts on the entire site, ranked by accumulated all-time view count. Activity kits went live on Aug 11 and have only 1–2 days of traffic. With years of established courses, lessons, and learning pathways outranking them, the kits don't appear in the top 1,000 and the function silently returns 0 for every kit.

This was confirmed by checking the Jetpack Stats admin page, which shows the site receiving ~5,000–8,000 views per day but only lists long-standing content (Beginner WordPress User, Introduction to WordPress, etc.) as top posts — no activity kits in sight.

Fix

Switch to WPCOM_Stats::get_total_post_views(), which hits the stats/views/posts endpoint and fetches view counts for specific post IDs directly, regardless of site-wide ranking. The kit post IDs are already known from the get_posts() query, so they are passed as a post_ids comma-separated parameter.

Also corrects the response key: the views/posts API uses uppercase 'ID' for the post identifier; top-posts used lowercase 'id'.

Testing

  1. Visit the Activity Kit Stats page
  2. Views should now show non-zero counts for kits that have been visited

🤖 Generated with Claude Code

Copilot AI 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.

Pull request overview

This PR addresses incorrect “0 views” reporting on the Activity Kit Stats REST endpoint by switching Jetpack Stats fetching to a per-post-ID views API, ensuring newly published kits are included even when they aren’t in the site-wide “top posts” list.

Changes:

  • Update the stats REST handler to fetch view counts by explicit kit post IDs via WPCOM_Stats::get_total_post_views().
  • Adjust parsing to match the views/posts response shape (including using uppercase ID).
  • Add an “Activity Library” item to the theme’s site navigation menus.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
wp-content/themes/pub/wporg-learn-2024/functions.php Adds an “Activity Library” entry to the Learn navigation menu.
wp-content/plugins/wporg-learn/inc/activity-kit-rest.php Switches Jetpack view retrieval to a per-post-ID endpoint and updates response parsing for accurate per-kit view counts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread wp-content/themes/pub/wporg-learn-2024/functions.php
…_posts()

The stats page was showing 0 views for all kits despite Jetpack being
connected and the site receiving thousands of daily page views.

Root cause: get_top_posts() returns the top N most-viewed posts on the
entire site, ranked by accumulated all-time views. With max=1000 and years
of established content (courses, lessons, learning pathways) outranking them,
newly published activity kits never appear in that list and silently return 0.

Fix: Switch to get_total_post_views() which calls the stats/views/posts
endpoint and fetches view counts for specific post IDs directly, regardless
of site-wide ranking. The kit post IDs are already known from the get_posts()
query, so they are passed as a comma-separated post_ids parameter.

Also update the response key parsing: the views/posts API returns uppercase
'ID' for the post identifier, whereas top-posts used lowercase 'id'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Piyopiyo-Kitsune
Piyopiyo-Kitsune force-pushed the fix/activity-kit-stats-views-api branch from bd0ddb6 to e9e1ee0 Compare August 12, 2026 01:52
@obenland

Copy link
Copy Markdown
Member

Code review findings

The core change is correct — stats/views/posts with the uppercase ID response key is the right endpoint for per-post view counts. But the endpoint's server-side validation breaks two of the four ranges, including the default one, so the "0 views" bug survives in its most common configuration.

1. Blocker: 90d and all ranges still return 0 views (activity-kit-rest.php:187)

The WPCOM stats/views/posts endpoint rejects num > 30 with a 422 (Number of days must be between 1 and 30. — confirmed in the wpcom server source; the public docs also say "Maximum of 30"). This PR sends num=90 for 90d and num=36 for all — and all is the REST route's default. A non-200 response means get_total_post_views() returns a WP_Error, get_jetpack_post_views() returns array(), and every kit shows 0 views on the initial stats page load. Only 7d and 30d actually work.

Serving the long ranges requires summing chunked ≤30-day windows, or a different endpoint for those ranges.

2. period is silently dropped (activity-kit-rest.php:186)

views/posts only declares post_ids, num (days), date, and offset — undeclared args are discarded by the WPCOM JSON API framework, so the 'all' => period='month', num=36 branch encodes an intent the API can never honor. The block now visually mirrors the still-meaningful one in get_jetpack_download_clicks() while meaning something different, which invites a bad "dedupe" refactor later. The views-side range mapping should be rewritten in this endpoint's own terms (days only) with period removed.

3. Unbounded kit query vs. the 100-ID cap on post_ids (activity-kit-rest.php:185)

post_ids accepts at most 100 IDs, but the kit query uses posts_per_page => -1. Once more than 100 activity kits are published, kits beyond the cap silently report 0 views (or the whole call errors and all report 0). Chunk with array_chunk( $kit_ids, 100 ) and merge the maps — or when the kit filter param is set, pass only that kit's ID.


Checked and cleared: the docblock updates, wp_list_pluck( $kits, 'ID' ) on WP_Post[], the uppercase 'ID' response key, and get_total_post_views() availability in the jetpack-stats package.

🤖 Generated with Claude Code

Address three issues from obenland's code review:

1. Blocker – remove num > 30: The views/posts API rejects any num > 30
   with a 422. The previous 'all' branch sent num=36 (months) and the
   '90d' branch sent num=90 — both always returned a WP_Error, so every
   kit showed 0 views on the default page load. Fix: cap each call at
   30 days and issue multiple windows for longer ranges (7d=1 call,
   30d=1 call, 90d/all=3 × 30-day calls offset by 0/30/60 days), then
   sum the results.

2. Remove silent period param: views/posts only accepts post_ids, num,
   date, and offset — any other arg is discarded by the WPCOM JSON API
   framework. Passing period='month' encoded an intent the API could
   never honor and mirrored the structure of get_jetpack_download_clicks()
   in a misleading way.

3. Chunk post_ids at 100: the endpoint accepts at most 100 IDs per call.
   With posts_per_page=-1 the library can grow past that limit and silently
   lose data. Use array_chunk(, 100) and merge the maps.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Piyopiyo-Kitsune

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough review — all three points addressed in f73fd26.

1. Blocker — 90d/all returning 0 views: Replaced the single call with a multi-window approach. Each window is ≤ 30 days; for 90d and all, three consecutive 30-day windows (offset 0, 30, 60 days) are issued and their results summed. 7d and 30d remain a single call each.

2. period silently dropped: Removed entirely. get_total_post_views() receives only post_ids, num, and date — the three parameters the endpoint actually reads.

3. 100-ID cap on post_ids: Added array_chunk( $kit_ids, 100 ) around the outer loop; results are merged into a single map. The inner window loop runs per chunk, so all combinations of chunk × window are covered.

The all range label in rangeLabel() in index.js was already updated to 'All time (max 90 days)' in a previous commit, so the UI correctly communicates the cap to admins.

🤖 Written with Claude Code

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +207 to +224
case '90d':
case 'all':
default:
$windows = array(
array(
'num' => 30,
'offset' => 0,
),
array(
'num' => 30,
'offset' => 30,
),
array(
'num' => 30,
'offset' => 60,
),
);
break;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — addressed in d2a9882.

A note on the "prior 36-month behavior": the old period=month, num=36 approach was already broken before this PR. The WPCOM JSON API framework discards period as an undeclared parameter, and num=36 triggers a 422 (Number of days must be between 1 and 30), so get_total_post_views() was returning WP_Error for both 90d and all — 0 views was always shown regardless.

The fix gives all its own 6-window case (offsets 0/30/60/90/120/150 days ≈ 180 days / ~6 months), so it now returns meaningfully more data than 90d (3 windows / 90 days). The JS rangeLabel() label is updated to "All time (max ~6 months)" to match.

Extending to 1 year (12 windows) is possible but means 12 sequential API calls per chunk, which adds noticeable latency to an admin page — 6 windows is a pragmatic ceiling for now and can be raised later if demand warrants it.

🤖 Written with Claude Code

Previously both '90d' and 'all' fell through to the same 3×30-day case,
so selecting 'All time' returned identical data to 'Last 90 days'. Give
'all' its own 6-window case (offsets 0/30/60/90/120/150 days ≈ 6 months)
so the two ranges return meaningfully different data.

Also update the JS rangeLabel() comment and label from 'All time (max 90
days)' to 'All time (max ~6 months)' to keep the UI in sync.

Extending further (e.g. 1 year = 12 windows) is possible but multiplies
sequential API calls by the same factor; 6 is a reasonable ceiling for an
admin-only dashboard with a small kit count.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants