-
Notifications
You must be signed in to change notification settings - Fork 58
feat(reader-data): add engagement fields #4594
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
Changes from all commits
b3297a6
c3db9f4
ab17b6a
847b434
21c456e
4a7f352
68296d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,5 +36,25 @@ export default function setupArticleViewsAggregates( ras ) { | |
| } | ||
| per_month[ month ][ data.post_id ] = true; | ||
| ras.store.set( 'article_view_per_month', per_month ); | ||
|
|
||
| // articles_read — A cumulative count of articles the reader has read. | ||
| const uniqueViews = ras.getUniqueActivitiesBy( 'article_view', 'post_id' ); | ||
| ras.store.set( 'articles_read', uniqueViews.length ); | ||
|
|
||
| // favorite_categories — A list of the reader's most-engaged content categories, ordered by frequency. | ||
| const allActivities = ras.getActivities( 'article_view' ); | ||
| const catCounts = {}; | ||
| for ( const activity of allActivities ) { | ||
| const cats = activity.data?.categories || []; | ||
| for ( const cat of cats ) { | ||
| catCounts[ cat ] = ( catCounts[ cat ] || 0 ) + 1; | ||
| } | ||
| } | ||
| const topCategories = Object.entries( catCounts ) | ||
| .filter( ( [ , count ] ) => count >= 2 ) | ||
| .sort( ( a, b ) => b[ 1 ] - a[ 1 ] ) | ||
| .slice( 0, 5 ) | ||
| .map( ( [ id ] ) => Number( id ) ); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| ras.store.set( 'favorite_categories', topCategories ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we replace the existing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure thing! I'll work on this in this PR: Automattic/newspack-popups#1542
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. Added a filter for this in 68296d0 |
||
| } ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import setupArticleViewsAggregates from './article-view'; | ||
| import { createMockRAS } from './mocks/ras'; | ||
|
|
||
| describe( 'setupArticleViewsAggregates', () => { | ||
| let mock; | ||
|
|
||
| beforeEach( () => { | ||
| mock = createMockRAS(); | ||
| setupArticleViewsAggregates( mock.ras ); | ||
| } ); | ||
|
|
||
| afterEach( () => { | ||
| mock.reset(); | ||
| } ); | ||
|
|
||
| function simulateArticleView( data, timestamp = Date.now() ) { | ||
chickenn00dle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| mock.trigger( 'activity', { action: 'article_view', data, timestamp } ); | ||
| } | ||
|
|
||
| it( 'should register an activity listener', () => { | ||
| expect( mock.ras.on ).toHaveBeenCalledWith( 'activity', expect.any( Function ) ); | ||
| } ); | ||
|
|
||
| it( 'should ignore non-article_view actions', () => { | ||
| mock.trigger( 'activity', { action: 'other_action', data: {}, timestamp: Date.now() } ); | ||
| expect( mock.ras.store.set ).not.toHaveBeenCalled(); | ||
| } ); | ||
|
|
||
| describe( 'articles_read', () => { | ||
| it( 'should set articles_read to count of unique post IDs', () => { | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [] } ); | ||
| mock.addActivity( 'article_view', { post_id: 2, categories: [] } ); | ||
| simulateArticleView( { post_id: 2, categories: [] } ); | ||
| expect( mock.storeData.articles_read ).toBe( 2 ); | ||
| } ); | ||
|
|
||
| it( 'should not increment for duplicate post IDs', () => { | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [] } ); | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [] } ); | ||
| simulateArticleView( { post_id: 1, categories: [] } ); | ||
| expect( mock.storeData.articles_read ).toBe( 1 ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'favorite_categories', () => { | ||
| it( 'should contain category IDs sorted by frequency', () => { | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [ 10, 20 ] } ); | ||
| mock.addActivity( 'article_view', { post_id: 2, categories: [ 10 ] } ); | ||
| mock.addActivity( 'article_view', { post_id: 3, categories: [ 20, 30 ] } ); | ||
| simulateArticleView( { post_id: 3, categories: [ 20, 30 ] } ); | ||
| // 10 appears 2x, 20 appears 2x, 30 appears 1x (excluded — needs >= 2). | ||
| expect( mock.storeData.favorite_categories ).toEqual( [ 10, 20 ] ); | ||
| } ); | ||
|
|
||
| it( 'should exclude categories with only 1 view', () => { | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [ 10 ] } ); | ||
| simulateArticleView( { post_id: 2, categories: [ 20 ] } ); | ||
| // Each category has only 1 view. | ||
| expect( mock.storeData.favorite_categories ).toEqual( [] ); | ||
| } ); | ||
|
|
||
| it( 'should limit to top 5 categories', () => { | ||
| // Each category needs at least 2 views to be included. | ||
| mock.addActivity( 'article_view', { post_id: 1, categories: [ 1, 2, 3, 4, 5, 6, 7 ] } ); | ||
| mock.addActivity( 'article_view', { post_id: 2, categories: [ 1, 2, 3, 4, 5, 6, 7 ] } ); | ||
| simulateArticleView( { post_id: 3, categories: [ 1, 2, 3, 4, 5, 6, 7 ] } ); | ||
| expect( mock.storeData.favorite_categories ).toHaveLength( 5 ); | ||
| } ); | ||
|
|
||
| it( 'should handle articles with no categories', () => { | ||
| mock.addActivity( 'article_view', { post_id: 1 } ); | ||
| simulateArticleView( { post_id: 1 } ); | ||
| expect( mock.storeData.favorite_categories ).toEqual( [] ); | ||
| } ); | ||
| } ); | ||
| } ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| /* globals newspack_reader_data */ | ||
|
|
||
| /** | ||
| * Set up general reader engagement fields. | ||
| * | ||
| * @param {Object} ras Reader Activation object. | ||
| */ | ||
| export default function setupEngagement( ras ) { | ||
| // first_visit_date — preserve the oldest known value (server or client). | ||
| const serverFirstVisit = newspack_reader_data?.items?.first_visit_date; | ||
| const serverValue = serverFirstVisit ? JSON.parse( serverFirstVisit ) : null; | ||
| const clientValue = ras.store.get( 'first_visit_date' ); | ||
| const candidates = [ serverValue, clientValue ].filter( Boolean ); | ||
| const firstVisit = candidates.length ? Math.min( ...candidates ) : Date.now(); | ||
| ras.store.set( 'first_visit_date', firstVisit ); | ||
|
|
||
| // last_active — Date reader was last seen on site. | ||
| ras.store.set( 'last_active', Date.now() ); | ||
| } | ||
chickenn00dle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import setupEngagement from './engagement'; | ||
| import { createMockRAS } from './mocks/ras'; | ||
|
|
||
| describe( 'setupEngagement', () => { | ||
| let mock; | ||
|
|
||
| beforeEach( () => { | ||
| mock = createMockRAS(); | ||
| window.newspack_reader_data = {}; | ||
| } ); | ||
|
|
||
| afterEach( () => { | ||
| mock.reset(); | ||
| delete window.newspack_reader_data; | ||
| } ); | ||
|
|
||
| it( 'should set first_visit_date on first call', () => { | ||
| setupEngagement( mock.ras ); | ||
| expect( mock.ras.store.set ).toHaveBeenCalledWith( 'first_visit_date', expect.any( Number ) ); | ||
| } ); | ||
|
|
||
| it( 'should preserve existing client first_visit_date when no server value', () => { | ||
| mock.storeData.first_visit_date = 1000; | ||
| setupEngagement( mock.ras ); | ||
| expect( mock.storeData.first_visit_date ).toBe( 1000 ); | ||
| } ); | ||
|
|
||
| it( 'should prefer older server value over newer client value', () => { | ||
| const oldServerValue = 1000; | ||
| const newClientValue = 9999; | ||
| mock.storeData.first_visit_date = newClientValue; | ||
| window.newspack_reader_data = { | ||
| items: { first_visit_date: JSON.stringify( oldServerValue ) }, | ||
| }; | ||
| setupEngagement( mock.ras ); | ||
| expect( mock.storeData.first_visit_date ).toBe( oldServerValue ); | ||
| } ); | ||
|
|
||
| it( 'should prefer older client value over newer server value', () => { | ||
| const oldClientValue = 1000; | ||
| const newServerValue = 9999; | ||
| mock.storeData.first_visit_date = oldClientValue; | ||
| window.newspack_reader_data = { | ||
| items: { first_visit_date: JSON.stringify( newServerValue ) }, | ||
| }; | ||
| setupEngagement( mock.ras ); | ||
| expect( mock.storeData.first_visit_date ).toBe( oldClientValue ); | ||
| } ); | ||
|
|
||
| it( 'should always set last_active', () => { | ||
| setupEngagement( mock.ras ); | ||
| expect( mock.ras.store.set ).toHaveBeenCalledWith( 'last_active', expect.any( Number ) ); | ||
| } ); | ||
|
|
||
| it( 'should set last_active to a recent timestamp', () => { | ||
| const before = Date.now(); | ||
| setupEngagement( mock.ras ); | ||
| const after = Date.now(); | ||
| expect( mock.storeData.last_active ).toBeGreaterThanOrEqual( before ); | ||
| expect( mock.storeData.last_active ).toBeLessThanOrEqual( after ); | ||
| } ); | ||
| } ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /** | ||
| * Creates a mock RAS (Reader Activation System) object for testing. | ||
| * | ||
| * @return {Object} Mock RAS with store, event handlers, and activity helpers. | ||
| */ | ||
| export function createMockRAS() { | ||
| const storeData = {}; | ||
| const activities = []; | ||
| const handlers = {}; | ||
|
|
||
| const ras = { | ||
| store: { | ||
| get: jest.fn( key => storeData[ key ] ?? null ), | ||
| set: jest.fn( ( key, value ) => { | ||
| storeData[ key ] = value; | ||
| } ), | ||
| }, | ||
| on: jest.fn( ( event, callback ) => { | ||
| handlers[ event ] = callback; | ||
| } ), | ||
| getActivities: jest.fn( () => activities ), | ||
| getUniqueActivitiesBy: jest.fn( () => { | ||
| const seen = {}; | ||
| return activities.filter( a => { | ||
| if ( seen[ a.data.post_id ] ) { | ||
| return false; | ||
| } | ||
| seen[ a.data.post_id ] = true; | ||
chickenn00dle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return true; | ||
| } ); | ||
| } ), | ||
| }; | ||
|
|
||
| return { | ||
| ras, | ||
| /** | ||
| * Get the current store data. | ||
| */ | ||
| storeData, | ||
| /** | ||
| * Add an activity to the internal activities array. | ||
| * | ||
| * @param {string} action Activity action name. | ||
| * @param {Object} data Activity data. | ||
| * @param {number} timestamp Optional timestamp. | ||
| */ | ||
| addActivity( action, data, timestamp = Date.now() ) { | ||
| activities.push( { action, data, timestamp } ); | ||
| }, | ||
| /** | ||
| * Trigger a registered event handler. | ||
| * | ||
| * @param {string} event Event name. | ||
| * @param {Object} detail Event detail payload. | ||
| */ | ||
| trigger( event, detail ) { | ||
| if ( handlers[ event ] ) { | ||
| handlers[ event ]( { detail } ); | ||
| } | ||
| }, | ||
| /** | ||
| * Reset all state between tests. | ||
| */ | ||
| reset() { | ||
| for ( const key in storeData ) { | ||
| delete storeData[ key ]; | ||
| } | ||
chickenn00dle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for ( const event in handlers ) { | ||
| delete handlers[ event ]; | ||
| } | ||
| activities.length = 0; | ||
| jest.clearAllMocks(); | ||
| }, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
seenflag to determine when to update paywall hits.