fix: execute tracking in an earlier hook

includes/class-newspack-newsletters-ads.php

@@ -710,6 +710,10 @@ public static function mark_ad_inserted( $newsletter_id, $ad_id ) {
 		if ( empty( self::$inserted_ads[ $newsletter_id ] ) ) {
 			self::$inserted_ads[ $newsletter_id ] = [];
 		}
+		// Avoid duplicate.
+		if ( in_array( $ad_id, self::$inserted_ads[ $newsletter_id ], true ) ) {
+			return;
+		}
 		self::$inserted_ads[ $newsletter_id ][] = $ad_id;
 		update_post_meta( $newsletter_id, 'inserted_ads', self::$inserted_ads[ $newsletter_id ] );
 	}

includes/tracking/class-click.php

@@ -19,12 +19,15 @@ final class Click {
 	public static function init() {
 		\add_action( 'init', [ __CLASS__, 'rewrite_rule' ] );
 		\add_filter( 'query_vars', [ __CLASS__, 'query_vars' ] );
+		\add_action( 'init', [ __CLASS__, 'handle_click' ], 2, 0 ); // Run on priority 2 to allow Data Events and ActionScheduler to initialize first.
 		\add_action( 'template_redirect', [ __CLASS__, 'handle_click' ] );
 		\add_filter( 'newspack_newsletters_process_link', [ __CLASS__, 'process_link' ], 10, 3 );
 	}
 
 	/**
 	 * Add rewrite rule for tracking url.
+	 *
+	 * Backwards compatibility for old tracking URLs.
 	 */
 	public static function rewrite_rule() {
 		\add_rewrite_rule( 'np-newsletters-click', 'index.php?' . self::QUERY_VAR . '=1', 'top' );
@@ -51,10 +54,12 @@ public static function query_vars( $vars = [] ) {
 	/**
 	 * Get tracking URL.
 	 *
+	 * Formerly 'home_url( 'np-newsletters-click' );'
+	 *
 	 * @return string
 	 */
 	public static function get_tracking_url() {
-		return \home_url( 'np-newsletters-click' );
+		return \add_query_arg( [ self::QUERY_VAR => 1 ], \home_url() );
 	}
 
 	/**
@@ -129,7 +134,7 @@ public static function track_click( $newsletter_id, $email_address, $url ) {
 	 * Handle proxied URL click and redirect to destination.
 	 */
 	public static function handle_click() {
-		if ( ! \get_query_var( self::QUERY_VAR ) ) {
+		if ( ! \get_query_var( self::QUERY_VAR ) && ! isset( $_GET[ self::QUERY_VAR ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
 			return;
 		}
 

includes/tracking/class-data-events.php

@@ -15,7 +15,7 @@ final class Data_Events {
 	 * Initialize hooks.
 	 */
 	public static function init() {
-		add_action( 'init', [ __CLASS__, 'register_listeners' ] );
+		add_action( 'plugins_loaded', [ __CLASS__, 'register_listeners' ] );
 	}
 
 	/**

includes/tracking/class-pixel.php

@@ -28,6 +28,7 @@ public static function init() {
 		\add_action( 'init', [ __CLASS__, 'rewrite_rule' ] );
 		\add_filter( 'redirect_canonical', [ __CLASS__, 'redirect_canonical' ], 10, 2 );
 		\add_filter( 'query_vars', [ __CLASS__, 'query_vars' ] );
+		\add_action( 'init', [ __CLASS__, 'render' ], 2, 0 ); // Run on priority 2 to allow Data Events and ActionScheduler to initialize first.
 		\add_action( 'template_redirect', [ __CLASS__, 'render' ] );
 	}
 
@@ -52,6 +53,8 @@ public static function add_tracking_pixel( $post ) {
 
 	/**
 	 * Add rewrite rule for tracking pixel.
+	 *
+	 * Backwards compatibility for old tracking URLs.
 	 */
 	public static function rewrite_rule() {
 		\add_rewrite_rule( 'np-newsletters.gif', 'index.php?' . self::QUERY_VAR . '=1', 'top' );
@@ -103,11 +106,12 @@ public static function get_pixel_url( $post_id ) {
 		}
 		$url = \add_query_arg(
 			[
-				'id'  => $post_id,
-				'tid' => $tracking_id,
-				'em'  => Utils::get_email_address_tag(),
+				self::QUERY_VAR => 1,
+				'id'            => $post_id,
+				'tid'           => $tracking_id,
+				'em'            => Utils::get_email_address_tag(),
 			],
-			\home_url( 'np-newsletters.gif' )
+			\home_url()
 		);
 		return $url;
 	}
@@ -149,7 +153,7 @@ public static function track_seen( $newsletter_id, $tracking_id, $email_address
 	 * Render the tracking pixel.
 	 */
 	public static function render() {
-		if ( ! \get_query_var( self::QUERY_VAR ) ) {
+		if ( ! \get_query_var( self::QUERY_VAR ) && ! isset( $_GET[ self::QUERY_VAR ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
 			return;
 		}
 

tests/test-tracking.php

@@ -21,10 +21,10 @@ public function test_tracking_pixel() {
 		ob_start();
 		do_action( 'newspack_newsletters_editor_mjml_body', $post );
 		$mjml_body = ob_get_clean();
-		$this->assertMatchesRegularExpression( '/np-newsletters\.gif\?id=' . $post_id . '/', $mjml_body );
+		$this->assertMatchesRegularExpression( '/\?np_newsletters_pixel=1&id=' . $post_id . '/', $mjml_body );
 
 		// Fetch the tracking pixel URL from body.
-		$pattern = '/src="([^"]*np-newsletters\.gif[^"]*)"/i';
+		$pattern = '/src="([^"]*np_newsletters_pixel[^"]*)"/i';
 		$matches = [];
 		preg_match( $pattern, $mjml_body, $matches );
 		$pixel_url  = html_entity_decode( $matches[1] );