Allow opting into analytics #101

kienstra · 2021-12-09T22:07:29Z

Changes

Copies BMO's analytics work from Genesis Blocks into GCB
Port the Settings page PHP from Pro to free
Not in scope: using analytics, like sending an event

Testing instructions

composer i && npm i && npm run dev
/wp-admin > Custom Blocks > Settings
Check 'Enable analytics':
Click 'Save Changes'
Create a new post
In the DevTools, enter window.GcbAnalytics
Expected: It's defined:

It looks like WP is still on node version 14.

Though it only has free plugin logic, wiht an action for the Pro plugin (or anything else) to run.

Thanks to BMO for: https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/AssetManager.php#L71

kienstra · 2021-12-15T17:10:29Z

php/Blocks/Loader.php

+		if ( Settings::ANALYTICS_OPTED_IN_VALUE === get_option( Settings::ANALYTICS_OPTION_NAME ) ) {
+			wp_enqueue_script(
+				self::ANALYTICS_SCRIPT_SLUG,
+				'https://www.googletagmanager.com/gtag/js?id=UA-17364082-14',


The UA id should probably be unique to GCB

Should be able to get that from the GA dashboard

OK nice, could you Slack me a link to that?

https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/AssetManager.php#L79

https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/js/gaClient.js

… GCB

kienstra · 2021-12-15T18:18:23Z

.nvmrc

@@ -1 +1 @@
-lts/*
+14


Node LTS used to be 14, but now it's 16. It seems that Gutenberg still uses 14

interesting! I haven't heard of this lts before.

kienstra · 2021-12-15T18:20:18Z

js/src/common/classes/index.js

@@ -0,0 +1 @@
+export { default as GAClient } from './GAClient';


This is silly for one file, but it follows the pattern of exporting files in all of the other directories

BMO-tech · 2021-12-16T17:13:41Z

js/src/common/classes/GAClient.js

+		};
+
+		// @ts-ignore
+		this.config = window.genesisAnalyticsConfig || {};


Does this value need to be window.gcbAnalyticsConfig ?

Uf, you're right.

Fixed in b45e3a3

BMO-tech · 2021-12-16T17:29:46Z

js/src/block-editor/index.js

 registerBlocks( genesisCustomBlocks, gcbBlocks, Edit );
+
+domReady( () => {


domReady is interesting. Where you coming across any issues that needed this?

That's a good point, I didn't notice that it was needed.

Moving it out of domReady() now…

Moved out in 87d1f45

BMO-tech · 2021-12-16T17:30:06Z

js/src/block-editor/index.js

+
+domReady( () => {
+	// @ts-ignore
+	window.dataLayer = window.dataLayer || [];


What was you logic for moving window.dataLayer our of GAClient ?

Not a functional reason, I just like class files to have only the class, and no logic other than that.

Maybe from too much time in PHP 🤣

But if you'd prefer, I could move it back.

Actually, I'm going to move it back.

Moved it back in 9d8bd80

BMO-tech · 2021-12-16T17:32:15Z

js/src/common/helpers/debounce.js

+ *
+ * @return {() => void} A debounced function.
+ */
+const debounce = ( func, wait ) => {


It doesn't follow the same standard as the rest of this plugin, but it is possible to name the export instead of using default.

export const debounce = () => { ... }

That will prevent the need for export default debounce at the bottom, and it can be included with

include { debounce } from './debounce';

Yeah, this plugin's setup is a bit outdated, with the index.js file having all of the exports.

Ideally, I'd modernize the whole js/src directory.

But it might be strange to not have this debounce in index.js, while all of the other helper functions are in it.

BMO-tech · 2021-12-16T17:34:19Z

php/Blocks/Loader.php

@@ -158,6 +166,22 @@ public function editor_assets() {
 			]
 		);

+		if ( Settings::ANALYTICS_OPTED_IN_VALUE === get_option( Settings::ANALYTICS_OPTION_NAME ) ) {


Do you want to factor in any historical opt-in events like we just merged into GB?

Hm, good question.

That might be something for product, but I think for this scope, this might be enough.

There is no genesisAnalyticsConfig in this plugin.

It's an essential part of the class, so it's strange to not have it in that file.

johnstonphilip · 2021-12-20T17:53:18Z

js/src/common/classes/GAClient.js

+	 * @param {boolean | number | string} enable The value to be set.
+	 */
+	enableAnalytics( enable ) {
+		enable = !! +enable;


I'm not sure I'm familiar with this syntax. What is happening on this line?

Ah, I stole that from BMO.

I didn't know about + either.

It's first casting enable to a Number with +enable. For example, converting '0' to 0.

Then, it casts it to a Boolean with !!.

I think this prevents an enable value of '0' from being true.

Like:

if ( '0' ) { console.log( 'This is true' ); }

Of course! I just found out about +

johnstonphilip · 2021-12-20T18:15:48Z

Looks all good here!

kienstra · 2021-12-20T18:26:42Z

Hi @johnstonphilip,
Thanks a lot for reviewing and testing this!

BMO-tech

Great work!

kienstra · 2021-12-21T20:22:55Z

@BMO-tech, thanks so much for your testing and review.

kienstra added 4 commits December 9, 2021 16:04

Mainly copy-paste the Settings page PHP to the free plugin

6b8e8b9

Correct 2 package tags to be for free, not pro

659f77f

Change node version to 14.18.1, see if that makes builds pass

0b75e3d

Change .nvmrc to 14, as lts is now 16

96f2872

It looks like WP is still on node version 14.

kienstra changed the title ~~Allow opting in to analytics~~ Allow opting into analytics Dec 9, 2021

kienstra added 5 commits December 13, 2021 22:22

Move the settings registration to the top of Admin class

6c9b39c

Change the settings group name to align with what it does

80db35c

Add a Settings.php view, copied from the Pro plugin

ae67220

Though it only has free plugin logic, wiht an action for the Pro plugin (or anything else) to run.

Enqueue the analytics script if opted-in, props BMO

9a204a5

Thanks to BMO for: https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/AssetManager.php#L71

Add a test for having opted into analytics

16e8841

kienstra commented Dec 15, 2021

View reviewed changes

kienstra added 7 commits December 15, 2021 11:11

Bump copyright dates that I should have bumped earlier

2fd92f4

Copy BMO's wp_localize_script() call

ce0791a

https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/AssetManager.php#L79

Copy BMO's GAClient into GCB

7a6a566

https://github.com/studiopress/genesis-blocks/blob/81b39c336123748bda7e4b2724cf9dbcea13c48b/lib/Analytics/js/gaClient.js

Instantiate GAClient in the GCB editor and the block editor

1848697

Remove the GB Analytics ID, add Todo to replace with a unique one for…

8e4d34f

… GCB

Change window.GenesisAnalytics to not conflict with Genesis Blocks

459f339

Move the analytics bootstrapping inside domReady()

c5eb1d3

kienstra commented Dec 15, 2021

View reviewed changes

kienstra added 2 commits December 15, 2021 12:21

Acknowledge BMO's work in debounce()

4c6b41b

Change some copypasta: this was copied from a Subscription page

0b2f869

kienstra marked this pull request as ready for review December 16, 2021 16:20

kienstra added 2 commits December 16, 2021 10:22

Acknowledge BMO's work in the GAClient

7c00b23

Make comment format more like GCB's comments

40cd3de

kienstra requested review from BMO-tech, dreamwhisper and johnstonphilip December 16, 2021 16:36

BMO-tech reviewed Dec 16, 2021

View reviewed changes

Change genesisAnalyticsConfig to gcbAnalyticsConfig

b45e3a3

There is no genesisAnalyticsConfig in this plugin.

kienstra added 2 commits December 16, 2021 12:20

Change genesisAnalyticsConfig to gcbAnalyticsConfig

87d1f45

There is no genesisAnalyticsConfig in this plugin.

Move window.dataLayer assignment back into the class file

9d8bd80

It's an essential part of the class, so it's strange to not have it in that file.

johnstonphilip reviewed Dec 20, 2021

View reviewed changes

johnstonphilip self-requested a review December 20, 2021 18:16

johnstonphilip approved these changes Dec 20, 2021

View reviewed changes

dreamwhisper approved these changes Dec 20, 2021

View reviewed changes

BMO-tech approved these changes Dec 21, 2021

View reviewed changes

kienstra merged commit eb77207 into develop Jan 7, 2022

kienstra deleted the add/analytics-opt-in branch January 7, 2022 17:36

kienstra mentioned this pull request Mar 22, 2022

Don't instantiate unused analytics client #116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow opting into analytics #101

Allow opting into analytics #101

kienstra commented Dec 9, 2021 •

edited

Loading

kienstra Dec 15, 2021

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021

kienstra Dec 15, 2021

johnstonphilip Dec 20, 2021

kienstra Dec 15, 2021

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021

kienstra Dec 16, 2021 •

edited

Loading

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021

kienstra Dec 16, 2021

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021

kienstra Dec 16, 2021

kienstra Dec 16, 2021

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021 •

edited

Loading

BMO-tech Dec 16, 2021

kienstra Dec 16, 2021

johnstonphilip Dec 20, 2021

kienstra Dec 20, 2021 •

edited

Loading

johnstonphilip Dec 20, 2021

kienstra Dec 20, 2021

johnstonphilip commented Dec 20, 2021

kienstra commented Dec 20, 2021

BMO-tech left a comment

kienstra commented Dec 21, 2021

		@@ -0,0 +1 @@
		export { default as GAClient } from './GAClient';

		registerBlocks( genesisCustomBlocks, gcbBlocks, Edit );

		domReady( () => {

		@@ -1 +1 @@
		lts/*
		14

Allow opting into analytics #101

Allow opting into analytics #101

Conversation

kienstra commented Dec 9, 2021 • edited Loading

Changes

Testing instructions

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Dec 16, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Dec 16, 2021 • 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

kienstra Dec 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstonphilip commented Dec 20, 2021

kienstra commented Dec 20, 2021

BMO-tech left a comment

Choose a reason for hiding this comment

kienstra commented Dec 21, 2021

kienstra commented Dec 9, 2021 •

edited

Loading

kienstra Dec 16, 2021 •

edited

Loading

kienstra Dec 16, 2021 •

edited

Loading

kienstra Dec 20, 2021 •

edited

Loading