Blog Stats Block: Include Visitors Data #35427

Aurorum · 2024-02-02T23:18:25Z

Proposed changes:

Adds the number of visitors to a site as something that can be displayed as part of the Blog Stats block.

Jetpack product discussion

See original issue (technically relates to the Legacy Widget, but I think it's reasonable for the block to supersede that).

Does this pull request change what data or activity we track or use?

We already use this data elsewhere, but it makes it visible in a new way.

Testing instructions:

Enable Beta blocks and add the "Blog Stats" widget to your site. You should now see two radios (an option dropdown felt very overkill for just two options, so I think a radio is most appropriate here). The new one should enable you to display your site visitors instead of views.

Note: we don't collect data of unique visitors for individual posts. You should be prevented from selecting the "Visitors" and "This individual post" combination.

cc @kangzj - sorry to give you something else to review :) for what it's worth, I think this is the only significant issue for the Legacy Widget in the backlog, so would be nice to fix it with this new block!

@jeherve - I'd appreciate your advice here regarding what you said in #11428 (comment):

I don't think that data is available in the WordPress.com REST API right now, so we'd have to extend the API first.

My understanding is that visitors in the API is just that, unique visitors. WordPress.com uses that term precisely from launch here and refer to it still here. As such, I believe that the comment in #11428 (comment) was mistaken, but just wanted to make sure.

github-actions · 2024-02-02T23:19:47Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

WordPress.com Simple releases happen daily.
WoA releases happen weekly.
Releases to self-hosted sites happen monthly. The next release is scheduled for March 5, 2024 (scheduled code freeze on March 4, 2024).

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

kangzj

It tests well and looks good to me! ❤️

kangzj · 2024-02-04T21:26:59Z

projects/plugins/jetpack/extensions/blocks/blog-stats/edit.js

@@ -34,6 +38,13 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) {
 		}
 	}, [ postId, isModuleActive ] );

+	// We don't collect visitor data for individual posts.
+	useEffect( () => {


kangzj · 2024-02-04T21:27:38Z

projects/plugins/jetpack/extensions/blocks/blog-stats/test/controls.js

@@ -19,10 +20,30 @@ describe( 'BlogStatsControls', () => {
 	} );

 	describe( 'Inspector settings', () => {
-		test( 'loads and displays settings', () => {
+		test( 'loads and displays views or visitors settings', () => {


kangzj · 2024-02-04T21:32:51Z

I noticed the build process is throwing an error complaining:

[plugins/jetpack 2:10.354] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "hit"

Aurorum · 2024-02-04T21:35:00Z

Err, it should still be there:

jetpack/projects/plugins/jetpack/extensions/blocks/blog-stats/edit.js

Lines 60 to 61 in 6e4ad75

    
           /* Translators: Number of views */ 
        
           const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack' );

Any idea on what the issue might be?

kangzj · 2024-02-04T21:39:26Z

projects/plugins/jetpack/extensions/blocks/blog-stats/blog-stats.php

 		}
 	}

-	$label = $attributes['label'] ? $attributes['label'] : esc_html(
+	$fallback_label = $attributes['statsData'] === 'visitors' ? esc_html(
+		_n( 'visitor', 'visitors', $stats, 'jetpack' )


Suggested change

_n( 'visitor', 'visitors', $stats, 'jetpack' )

/* Translators: Number of visitors */

_n( 'visitor', 'visitors', $stats, 'jetpack' )

Let's try this 🤔

Build is still failing 🤔

Do you have any insights please @jeherve @anomiex? Thanks

[plugins/jetpack 2:10.354] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "hit"

Looks a bit ugly, but I think it relates to where the comment goes: #28351 (comment)

Just added a commit that will maybe fix it? Not sure.

Was worth a try, but nope, issue seems to persist!

You should be able to work around that by adding a dummy third argument, like it's done here:

jetpack/projects/plugins/jetpack/extensions/blocks/map/styles.js

Lines 22 to 25 in 7693bbd

label:

provider === 'mapkit'

? __( 'Muted', 'jetpack', /* dummy arg to avoid bad minification */ 0 )

: __( 'Black & White', 'jetpack' ),

Thanks, done! Not sure how to check locally, but hopefully that fixes the tests.

Part of the confusion is that this isn't the code causing that error, it's the JS code in edit.js. I'll comment there with details.

You should be able to check locally with jetpack build --production plugins/jetpack, or in projects/plugins/jetpack/ run pnpm build-production-extensions.

…ts.php

jeherve · 2024-02-05T10:48:34Z

My understanding is that visitors in the API is just that, unique visitors.

Yup, this should be good! I don't believe this was previously available for single posts in the API, but from what you're saying I'm getting that this is now available? 🥳

Aurorum · 2024-02-05T12:29:25Z

I don't believe this was previously available for single posts in the API, but from what you're saying I'm getting that this is now available? 🥳

@jeherve I'm afraid that we still don't have visitor data for individual posts. I think that's because it was never collected as opposed to not being exposed in the API though.

We do have visitor data for the whole site though (which I think is what the original issue was about - the Blog Stats widget never allowed access to this!), so that's what this PR adds to the block. :) I've got around this by disabling the "Visitors" + "Individual post" combination.

anomiex · 2024-02-05T15:01:57Z

projects/plugins/jetpack/extensions/blocks/blog-stats/blog-stats.php

 		}
 	}

-	$label = $attributes['label'] ? $attributes['label'] : esc_html(
+	$fallback_label = $attributes['statsData'] === 'visitors' ? esc_html(
+		_n( 'visitor', 'visitors', $stats, 'jetpack' )


Part of the confusion is that this isn't the code causing that error, it's the JS code in edit.js. I'll comment there with details.

You should be able to check locally with jetpack build --production plugins/jetpack, or in projects/plugins/jetpack/ run pnpm build-production-extensions.

anomiex · 2024-02-05T15:17:59Z

projects/plugins/jetpack/extensions/blocks/blog-stats/edit.js

+	/* Translators: Number of visitors */
+	const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 );
+
+	/* Translators: Number of views */
+	const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );


The error

[plugins/jetpack 2:10.354] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "hit"

is actually coming from here. There's a detailed explanation of what's going on at https://www.npmjs.com/package/@automattic/i18n-check-webpack-plugin#lost-comments-due-to-expression-movement, but the TL;DR is that this code is getting transformed to look something like this instead:

/* Translators: Number of visitors */ const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 ), viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 ); /* Translators: Number of views */

(i.e. it deletes the first const and moves the second assignment into the first one, but leaves the comment behind where it was)

This seems to work

Suggested change

/* Translators: Number of visitors */

const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 );

/* Translators: Number of views */

const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );

const

/* Translators: Number of visitors */

visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 ),

/* Translators: Number of views */

viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );

Or this

Suggested change

/* Translators: Number of visitors */

const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 );

/* Translators: Number of views */

const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );

const visitorsPlaceholder =

/* Translators: Number of visitors */

_n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 );

const viewsPlaceholder =

/* Translators: Number of views */

_n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );

I don't know why 924d9be didn't work. Possibly I should remove the corresponding suggestion from the doc I linked if it doesn't work anymore.

I've moved the comment and added a parameter - hopefully that works! Thanks for taking a look. :)

[plugins/jetpack 2:10.961] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "visitor" [plugins/jetpack 2:10.961] [build-production-extensions] - Translators: Number of visitors [plugins/jetpack 2:10.961] [build-production-extensions] [plugins/jetpack 2:10.961] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "hit" [plugins/jetpack 2:10.961] [build-production-extensions] - Translators: Number of views [plugins/jetpack 2:10.961] [build-production-extensions]

🤔

Unfortunately f8a461d is basically a re-do of 924d9be which still doesn't work (see #35452 for what I know about that). Either of the suggestions I made above worked for me locally.

Sorry, it looks like I committed the wrong thing. 🤦 It works fine locally for me too - re-added your suggestion now!

kangzj

🎉

* Add changelog * Blog Stats block: Include visitors data * Remove unneeded class name * Update projects/plugins/jetpack/extensions/blocks/blog-stats/blog-stats.php * Try fix tests * Fix translator comment * Update translator string * Fix comments again --------- Co-authored-by: Jasper Kang <[email protected]>

Aurorum added 2 commits February 2, 2024 23:02

Add changelog

4f711d0

Blog Stats block: Include visitors data

ec9ffbd

github-actions bot added [Block] Blog Stats [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Tests] Includes Tests labels Feb 2, 2024

github-actions bot added the OSS Citizen This Pull Request was opened by an Open Source contributor. label Feb 2, 2024

Remove unneeded class name

6e4ad75

kangzj previously approved these changes Feb 4, 2024

View reviewed changes

kangzj enabled auto-merge (squash) February 4, 2024 21:28

kangzj disabled auto-merge February 4, 2024 21:29

kangzj reviewed Feb 4, 2024

View reviewed changes

Update projects/plugins/jetpack/extensions/blocks/blog-stats/blog-sta…

54d78c5

…ts.php

kangzj dismissed their stale review via 54d78c5 February 4, 2024 21:40

kangzj enabled auto-merge (squash) February 4, 2024 21:41

kangzj previously approved these changes Feb 4, 2024

View reviewed changes

Try fix tests

924d9be

auto-merge was automatically disabled February 4, 2024 21:49
Head branch was pushed to by a user without write access

Aurorum dismissed kangzj’s stale review via 924d9be February 4, 2024 21:49

Fix translator comment

2057abc

anomiex reviewed Feb 5, 2024

View reviewed changes

Aurorum added 3 commits February 5, 2024 15:52

Update translator string

f8a461d

Fix comments again

3dbaa4c

Merge branch 'trunk' into add/blog-stats-visitors

0bb778c

kangzj approved these changes Feb 6, 2024

View reviewed changes

kangzj merged commit 2cbe58f into Automattic:trunk Feb 6, 2024
52 checks passed

github-actions bot added this to the jetpack/13.2 milestone Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blog Stats Block: Include Visitors Data #35427

Blog Stats Block: Include Visitors Data #35427

Aurorum commented Feb 2, 2024

github-actions bot commented Feb 2, 2024 •

edited

Loading

kangzj left a comment

kangzj Feb 4, 2024

kangzj Feb 4, 2024

kangzj commented Feb 4, 2024

Aurorum commented Feb 4, 2024

kangzj Feb 4, 2024

kangzj Feb 4, 2024

Aurorum Feb 4, 2024

kangzj Feb 4, 2024 •

edited

Loading

Aurorum Feb 4, 2024

Aurorum Feb 4, 2024

jeherve Feb 5, 2024

Aurorum Feb 5, 2024

anomiex Feb 5, 2024

jeherve commented Feb 5, 2024

Aurorum commented Feb 5, 2024

anomiex Feb 5, 2024

anomiex Feb 5, 2024

Aurorum Feb 5, 2024

kangzj Feb 6, 2024

anomiex Feb 6, 2024

Aurorum Feb 6, 2024

kangzj left a comment

	_n( 'visitor', 'visitors', $stats, 'jetpack' )
	/* Translators: Number of visitors */
	_n( 'visitor', 'visitors', $stats, 'jetpack' )

	label:
	provider === 'mapkit'
	? __( 'Muted', 'jetpack', /* dummy arg to avoid bad minification */ 0 )
	: __( 'Black & White', 'jetpack' ),

Blog Stats Block: Include Visitors Data #35427

Blog Stats Block: Include Visitors Data #35427

Conversation

Aurorum commented Feb 2, 2024

Proposed changes:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Feb 2, 2024 • edited Loading

kangzj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangzj commented Feb 4, 2024

Aurorum commented Feb 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangzj Feb 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeherve commented Feb 5, 2024

Aurorum commented Feb 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangzj left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 2, 2024 •

edited

Loading

kangzj Feb 4, 2024 •

edited

Loading