feat(reader-data): add engagement fields#4594
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds new engagement-related Reader Data fields and wires them into both the Reader Activation JS library and content-gate metering (JS + PHP) so engagement metrics can be persisted and synced for readers.
Changes:
- Initialize/store
first_visit_dateandlast_activeon the client during Reader Activation init. - Derive and store
articles_readandfavorite_categoriesfromarticle_viewactivity history. - Increment
paywall_hitsfor anonymous readers in JS metering and for logged-in readers in PHP metering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/reader-activation/index.js | Wires engagement setup into Reader Activation initialization. |
| src/reader-activation/engagement.js | Adds client-side initialization for first_visit_date and last_active. |
| src/reader-activation/article-view.js | Adds derived aggregates articles_read and favorite_categories from article_view activities. |
| src/content-gate/metering.js | Increments paywall_hits for anonymous metering blocks. |
| includes/content-gate/class-metering.php | Increments paywall_hits for logged-in metering blocks server-side. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dbe011d to
e51b001
Compare
e51b001 to
847b434
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const detectSeen = () => { | ||
| const delta = ( gate?.getBoundingClientRect().top || 0 ) - window.innerHeight / 2; | ||
| if ( delta < 0 ) { | ||
| handleSeen( gate, ! seen ); |
There was a problem hiding this comment.
Just noting I initially removed the typeof window.gtag check below to ensure this only triggers once, but it seems this check is to avoid a race condition and ensure the listener isn't removed when gtag hasn't been added to window before this runs. So instead, I am adding a seen flag to determine when to update paywall hits.
| const topCategories = Object.entries( catCounts ) | ||
| .sort( ( a, b ) => b[ 1 ] - a[ 1 ] ) | ||
| .slice( 0, 5 ) | ||
| .map( ( [ id ] ) => Number( id ) ); |
There was a problem hiding this comment.
The spec calls for a list of category strings, but since we only have ids here, I am storing ids which can be converted to string representation in the backend. But let me know if we prefer to store as strings here instead.
src/reader-activation/engagement.js
Outdated
| // first_visit_date — Date of the reader's very first visit to the site, regardless of whether or not they registered. | ||
| if ( ! ras.store.get( 'first_visit_date' ) ) { | ||
| ras.store.set( 'first_visit_date', Date.now() ); | ||
| } |
There was a problem hiding this comment.
The store sync strategy will overwrite this on every new authenticated session. The fresh session will generate a new first_visit_date and upon authentication, the new value will replace the one in the db:
newspack-plugin/src/reader-activation/store.js
Lines 263 to 266 in bb4813c
I don't think we ever got a consensus on whether the stored value takes precedence over the new value from the reader session. There's a case for both...
There was a problem hiding this comment.
Good catch @miguelpeixe!
I added a commit to ensure we always defer to the oldest date for first_visit_date here: 4a7f352
I suspect first_visit_date might be the only key which requires the stored value take precedence as the other fields should be updated with the latest activity right? I'm happy to revisit this if this isn't the case though.
There was a problem hiding this comment.
I think this works well for this case. I'm working on a session hydration feature in #4618 that will require some tweaks to support this new requirement. I can adapt once this is merged.
| .sort( ( a, b ) => b[ 1 ] - a[ 1 ] ) | ||
| .slice( 0, 5 ) | ||
| .map( ( [ id ] ) => Number( id ) ); | ||
| ras.store.set( 'favorite_categories', topCategories ); |
There was a problem hiding this comment.
Can we replace the existing favorite_categories matching function on newspack-popups to use this, so we have a single source of truth?
There was a problem hiding this comment.
Sure thing! I'll work on this in this PR: Automattic/newspack-popups#1542
There was a problem hiding this comment.
Looks like we're losing part of the matching logic, to have at least 2 categories or 2 posts within the same category.
Do you think we can move that from the matching function to the list generation itself? In reality, the reader shouldn't really have a favorite category if they just read a single article that happens to match a category.
There was a problem hiding this comment.
Good call. Added a filter for this in 68296d0
miguelpeixe
left a comment
There was a problem hiding this comment.
Thank you for the revisions!
|
Hey @chickenn00dle, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
All Submissions:
Changes proposed in this Pull Request:
Adds five new reader data fields to the Reader Data library for tracking reader engagement behavior:
first_visit_date— Timestamp of the reader's first visit (set once, never overwritten)last_active— Timestamp of the reader's most recent page load (updated every page load)articles_read— Count of unique articles viewed (derived fromarticle_viewactivities)favorite_categories— Top 5 category IDs by view frequency (derived fromarticle_viewactivity data)paywall_hits— Count of paywall encountersAll fields are client-authoritative and server-persisted — computed in JS, stored in localStorage, and synced to server via the existing store sync mechanism for logged-in users.
Closes https://linear.app/a8c/issue/NPPD-1387/create-new-fields-on-reader-data-library .
How to test the changes in this Pull Request:
np_reader_1_np_reader_1_first_visit_dateandnp_reader_1_last_activeappear with timestamp valuesfirst_visit_dateis unchanged butlast_activehas a newer timestampnp_reader_1_articles_readappears with value1articles_readis now2np_reader_1_favorite_categories— should contain an array of category IDsnp_reader_1_paywall_hitsincrements/newspack/v1/reader-data)Other information: