fix(ga4): ensure gate_post_id and newspack_popup_id are passed to activities #3956

dkoo · 2025-04-29T18:44:02Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Fixes issues with reader_registered and reader_logged_in activities losing the gate_post_id and newspack_popup_id params in certain cases.

Also removes nonce verification for the Reader Registration block form, which seems to be resulting in failed registration attempts in some cases where whole-page HTML is served from a cache.

How to test the changes in this Pull Request:

Follow testing instructions from #3895, preferably using a site that has edge caching enabled. Ensure that:

After a successful registration by any means from inside a content gate or a Campaigns prompt, the GA events for np_reader_registration should have gate_post_id or newspack_popup_id params attached.
Events for np_reader_logged_in should have gate_post_id params if triggered from a #signin_modal link inside a content gate, whether using password or OTP.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

…ivities

includes/reader-activation/class-reader-activation.php

miguelpeixe · 2025-05-02T19:39:03Z

src/blocks/reader-registration/index.php

 function process_form() {
 	// No need to process form values if Reader Activation is disabled.
-	if ( ! Reader_Activation::is_enabled() ) {
+	if ( ! Reader_Activation::is_enabled() || \is_user_logged_in() ) {


Will we ever have this running when the reader is logged in?

It's not doing much, removed in c26838d

miguelpeixe · 2025-05-02T19:40:22Z

src/reader-activation-auth/auth-form.js

 				}

 				if ( 'otp' === action ) {
+					const code = { code: body.get( 'otp_code' ) };


Maybe change this variable to something else, like data or payload?

Changed this approach in c26838d, so it's no longer needed

miguelpeixe · 2025-05-02T19:42:39Z

includes/data-events/class-memberships.php

+		if ( ! empty( $gate_post_id ) && ( isset( $metadata['registration_method'] ) || isset( $metadata['login_method'] ) ) ) {
+			$metadata['gate_post_id'] = $gate_post_id;
+			if ( isset( $metadata['registration_method'] ) ) {
+				$metadata['registration_method'] = $metadata['registration_method'] . '-content-gate';
+			}
+			if ( isset( $metadata['login_method'] ) ) {
+				$metadata['login_method'] = $metadata['login_method'] . '-content-gate';
+			}


I know this has already been merged in this format, but I think we can get more out of this if we use different parameters here instead of concatenating strings. WDYT?

Something like:
login_method: otp
login_trigger: content-gate

I agree that I'm not a fan of the string concatenation altering the values of registration_method and login_method. I might argue that adding the -popup or -content-gate suffix to the method is now no longer needed since we're always passing newspack_popup_id and gate_post_id params when the action originates from those triggers, WDYT @kmwilkerson?

~~1bf9122~~ c26838d and Automattic/newspack-popups#1431 propose removing these suffixes to avoid the string concatenation.

Maybe you forgot to push 1bf9122 or is it in c26838d?

Sorry, I meant c26838d

miguelpeixe · 2025-05-02T19:46:14Z

src/reader-activation/index.js

 * @return {Promise} Promise.
 */
-export function authenticateOTP( code ) {
+export function authenticateOTP( { code, memberships_content_gate } ) {


I'm not a fan of this approach... Outside analytics, this metadata is not relevant to the authenticateOTP() function. If the analytics metadata grows, it can look pretty bad.

Can we do this in a more extensible way? Perhaps leveraging the store or cookies to pass this information along to the backend?

I'll think of a better way to do this

c26838d avoids passing the gate post ID to the backend entirely by just looking for the presence of the memberships_content_gate data in the form data. Come to think of it, since all of the activities and the GA events they trigger are now on the front-end, there's really no reason to send this info the back-end at all, but I didn't want to refactor too much in this changeset. Let me know if you'd prefer to just rip off that band-aid instead.

Awesome! Confirmed the gate ID is present on the OTP auth flow:

Co-authored-by: Miguel Peixe <[email protected]>

miguelpeixe

Thank you for the revisions!

Code looks good and working as per testing instructions.

I tested one other thing that didn't behave as I expected and I'm not sure if it's intentional. Signing in via a prompt doesn't include the popup ID in the reader_logged_in event like it does for gates. I'm ready to approve if that's expected.

dkoo · 2025-05-07T17:59:30Z

I tested one other thing that didn't behave as I expected and I'm not sure if it's intentional. Signing in via a prompt doesn't include the popup ID in the reader_logged_in event like it does for gates. I'm ready to approve if that's expected.

I wasn't sure what to do for capturing logins via prompts. In almost every case a login from a prompt will come from the Registration block's "Sign into an existing account" button, which will simply open the auth modal. So there's no way to tell the difference between a login from clicking this button vs. logging in via the auth modal directly. So for now, this is expected, but I'm open to handling this in some other way (perhaps by adding a hidden input to the auth modal form if it's opened by clicking the button? But this seems like overengineering to me).

miguelpeixe

I agree!

I think we faced this before and at the time I also considered expanding the modal hashes to also hold additional parameters for such cases but didn't pursue...

# [6.5.0-alpha.3](v6.5.0-alpha.2...v6.5.0-alpha.3) (2025-05-07) ### Bug Fixes * **ga4:** ensure gate_post_id and newspack_popup_id are passed to activities ([#3956](#3956)) ([fadaa4d](fadaa4d))

matticbot · 2025-05-07T19:10:49Z

🎉 This PR is included in version 6.5.0-alpha.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [6.6.0-alpha.1](v6.5.0...v6.6.0-alpha.1) (2025-05-08) ### Bug Fixes * add white background to Newspack admin screens ([#3909](#3909)) ([4c8409e](4c8409e)) * allow guest contributors to have empty archives ([#3902](#3902)) ([af8bed9](af8bed9)) * **analytics:** migrate back-end GA4 events to front-end ([#3895](#3895)) ([7e18628](7e18628)) * **auth-form:** check user before is_user_reader() call ([#3944](#3944)) ([1cbb760](1cbb760)) * avoid warning for network event ([#3942](#3942)) ([60d0aa3](60d0aa3)) * **bylines:** avoid rerendering of editable area ([#3896](#3896)) ([9fdf659](9fdf659)) * **bylines:** custom edited state for modal ([#3919](#3919)) ([2a1a800](2a1a800)) * **bylines:** get CAP data from the plugin's store ([#3906](#3906)) ([0745113](0745113)) * **corrections:** Limit CPT to posts only ([#3927](#3927)) ([1d49c39](1d49c39)) * **corrections:** Use Corrections Archive template by default ([#3929](#3929)) ([c4c1925](c4c1925)) * **email-change:** ensure success and error messages persist after redirect ([#3913](#3913)) ([caf82a5](caf82a5)) * **email-change:** improve messaging for in-progress or expired change requests ([#3886](#3886)) ([02d2ea0](02d2ea0)) * **email-change:** typo in variable name ([#3941](#3941)) ([941dc21](941dc21)) * **ga4:** ensure gate_post_id and newspack_popup_id are passed to activities ([#3956](#3956)) ([fadaa4d](fadaa4d)) * **google-login:** handle google oauth disabled state in the editor ([#3946](#3946)) ([0f9f7c6](0f9f7c6)) * hide campaigns menu when the Newspack Campaigns plugin is not activated ([#3922](#3922)) ([adcc21f](adcc21f)) * make front end not dependent on coauthors plus ([#3905](#3905)) ([dc96e5b](dc96e5b)) * **newsletters-wizard:** remove advertisers menu item ([#3948](#3948)) ([2fddbd6](2fddbd6)) * **otp-input:** enhance OTP input handling for clipboard ([#3898](#3898)) ([a56328b](a56328b)) * reinstate the Add New Newsletter link in the admin menu ([#3926](#3926)) ([1c8c7d7](1c8c7d7)) * remove extra space above empty help text ([#3904](#3904)) ([6a4fe4f](6a4fe4f)) * remove import and route for Suppression ([#3894](#3894)) ([a2034e7](a2034e7)) * **setup-wizard:** prevent Yoast redirect ([#3940](#3940)) ([3461b5f](3461b5f)) ### Features * **auth:** OTP autocomplete integration ([#3888](#3888)) ([5a1d7a8](5a1d7a8)) * **bylines:** get post byline authors ([#3911](#3911)) ([17942a2](17942a2)) * **bylines:** useAuthorTokens() hook with user entity validation ([#3907](#3907)) ([3d1feee](3d1feee)) * **email-change:** allow old email to cancel email change only, not confirm ([#3912](#3912)) ([92413e3](92413e3)) * **ga4:** np_newsletter_subscribed event upon front-end signup ([#3938](#3938)) ([2295277](2295277)) * **ras-newsletter:** Newsletters list progressive disclosure ([#3887](#3887)) ([53f4eed](53f4eed)) * **reader-auth:** send WP login reminder email to non-reader accounts ([#3796](#3796)) ([d1af25e](d1af25e)), closes [#3823](#3823) * **scss:** export colors as module for JS consumption ([#3910](#3910)) ([81da8f0](81da8f0)) * **segments:** add filter for device segment ([#3880](#3880)) ([d465726](d465726)) * update colors and add all design system variables ([#3882](#3882)) ([6ee8e30](6ee8e30))

matticbot · 2025-05-08T17:10:49Z

🎉 This PR is included in version 6.6.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [6.6.0](v6.5.1...v6.6.0) (2025-05-14) ### Bug Fixes * add white background to Newspack admin screens ([#3909](#3909)) ([4c8409e](4c8409e)) * allow guest contributors to have empty archives ([#3902](#3902)) ([af8bed9](af8bed9)) * **analytics:** migrate back-end GA4 events to front-end ([#3895](#3895)) ([7e18628](7e18628)) * **auth-form:** check user before is_user_reader() call ([#3944](#3944)) ([1cbb760](1cbb760)) * avoid warning for network event ([#3942](#3942)) ([60d0aa3](60d0aa3)) * **bylines:** avoid rerendering of editable area ([#3896](#3896)) ([9fdf659](9fdf659)) * **bylines:** custom edited state for modal ([#3919](#3919)) ([2a1a800](2a1a800)) * **bylines:** get CAP data from the plugin's store ([#3906](#3906)) ([0745113](0745113)) * **corrections:** Limit CPT to posts only ([#3927](#3927)) ([1d49c39](1d49c39)) * **corrections:** Use Corrections Archive template by default ([#3929](#3929)) ([c4c1925](c4c1925)) * **email-change:** ensure success and error messages persist after redirect ([#3913](#3913)) ([caf82a5](caf82a5)) * **email-change:** improve messaging for in-progress or expired change requests ([#3886](#3886)) ([02d2ea0](02d2ea0)) * **email-change:** typo in variable name ([#3941](#3941)) ([941dc21](941dc21)) * **ga4:** ensure gate_post_id and newspack_popup_id are passed to activities ([#3956](#3956)) ([fadaa4d](fadaa4d)) * **google-login:** handle google oauth disabled state in the editor ([#3946](#3946)) ([0f9f7c6](0f9f7c6)) * hide campaigns menu when the Newspack Campaigns plugin is not activated ([#3922](#3922)) ([adcc21f](adcc21f)) * make front end not dependent on coauthors plus ([#3905](#3905)) ([dc96e5b](dc96e5b)) * **newsletters-wizard:** remove advertisers menu item ([#3948](#3948)) ([2fddbd6](2fddbd6)) * **otp-input:** enhance OTP input handling for clipboard ([#3898](#3898)) ([a56328b](a56328b)) * reinstate the Add New Newsletter link in the admin menu ([#3926](#3926)) ([1c8c7d7](1c8c7d7)) * remove extra space above empty help text ([#3904](#3904)) ([6a4fe4f](6a4fe4f)) * remove import and route for Suppression ([#3894](#3894)) ([a2034e7](a2034e7)) * **setup-wizard:** prevent Yoast redirect ([#3940](#3940)) ([3461b5f](3461b5f)) ### Features * **auth:** OTP autocomplete integration ([#3888](#3888)) ([5a1d7a8](5a1d7a8)) * **bylines:** get post byline authors ([#3911](#3911)) ([17942a2](17942a2)) * **bylines:** useAuthorTokens() hook with user entity validation ([#3907](#3907)) ([3d1feee](3d1feee)) * **email-change:** allow old email to cancel email change only, not confirm ([#3912](#3912)) ([92413e3](92413e3)) * **email-change:** remove env constant requirement ([#3943](#3943)) ([f04e58f](f04e58f)) * **ga4:** np_newsletter_subscribed event upon front-end signup ([#3938](#3938)) ([2295277](2295277)) * **ras-newsletter:** Newsletters list progressive disclosure ([#3887](#3887)) ([53f4eed](53f4eed)) * **reader-auth:** send WP login reminder email to non-reader accounts ([#3796](#3796)) ([d1af25e](d1af25e)), closes [#3823](#3823) * **scss:** export colors as module for JS consumption ([#3910](#3910)) ([81da8f0](81da8f0)) * **segments:** add filter for device segment ([#3880](#3880)) ([d465726](d465726)) * update colors and add all design system variables ([#3882](#3882)) ([6ee8e30](6ee8e30))

matticbot · 2025-05-14T17:29:05Z

🎉 This PR is included in version 6.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

dkoo added 2 commits April 29, 2025 12:27

fix(ga4): ensure gate_post_id and newspack_popup_id are passed to act…

067682c

…ivities

fix(ga4): re-add newspack_registration_before_form_fields action

482dcc6

dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Apr 29, 2025

dkoo self-assigned this Apr 29, 2025

dkoo requested a review from a team as a code owner April 29, 2025 18:44

miguelpeixe requested changes May 2, 2025

View reviewed changes

dkoo and others added 2 commits May 2, 2025 14:13

Update includes/reader-activation/class-reader-activation.php

6939aac

Co-authored-by: Miguel Peixe <[email protected]>

fix: feedback from PR

c26838d

dkoo mentioned this pull request May 2, 2025

fix: remove "-popup" suffix from method params, no longer needed Automattic/newspack-popups#1431

Merged

6 tasks

miguelpeixe requested changes May 6, 2025

View reviewed changes

dkoo requested a review from miguelpeixe May 7, 2025 17:59

miguelpeixe approved these changes May 7, 2025

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels May 7, 2025

dkoo merged commit fadaa4d into alpha May 7, 2025
16 checks passed

dkoo deleted the fix/ga4-gate-popup-ids branch May 7, 2025 19:05

matticbot added the released on @alpha label May 7, 2025

matticbot added the released label May 14, 2025

dkoo mentioned this pull request Nov 12, 2025

fix: content gate metadata methods and params #4306

Merged

6 tasks

fix(ga4): ensure gate_post_id and newspack_popup_id are passed to activities #3956

fix(ga4): ensure gate_post_id and newspack_popup_id are passed to activities #3956

Uh oh!

Conversation

dkoo commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkoo May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkoo May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miguelpeixe May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkoo May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miguelpeixe left a comment

Choose a reason for hiding this comment

Uh oh!

dkoo commented May 7, 2025

Uh oh!

miguelpeixe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matticbot commented May 7, 2025

Uh oh!

matticbot commented May 8, 2025

Uh oh!

matticbot commented May 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dkoo commented Apr 29, 2025 •

edited

Loading

dkoo May 2, 2025 •

edited

Loading

dkoo May 2, 2025 •

edited

Loading

miguelpeixe May 6, 2025 •

edited

Loading

dkoo May 2, 2025 •

edited

Loading