Moving hook initialisation functions out of constructors (#7355)

changelog/dev-7262-remove-hooks-from-constructors

@@ -0,0 +1,4 @@
+Significance: minor
+Type: dev
+
+Refactor to move hook initialisation out of constructors.

dev/phpcs/ruleset.xml

@@ -15,11 +15,6 @@
 	<exclude-pattern>*/includes/woopay-user/*</exclude-pattern>
 	<exclude-pattern>*/includes/class-wc-payments-order-success-page.php</exclude-pattern>
 
-	<!-- https://github.com/Automattic/woocommerce-payments/issues/7262 -->
-	<exclude-pattern>*/includes/class-wc-payments-account.php</exclude-pattern>
-	<exclude-pattern>*/includes/class-wc-payments-incentives-service.php</exclude-pattern>
-	<exclude-pattern>*/includes/class-wc-payments-onboarding-service.php</exclude-pattern>
-
 	<!-- https://github.com/Automattic/woocommerce-payments/issues/7263 -->
 	<exclude-pattern>*/includes/class-wc-payments-apple-pay-registration.php</exclude-pattern>
 	<exclude-pattern>*/includes/class-wc-payments-express-checkout-button-display-handler.php</exclude-pattern>

includes/class-wc-payments-account.php

@@ -82,24 +82,32 @@ public function __construct(
 		$this->database_cache           = $database_cache;
 		$this->action_scheduler_service = $action_scheduler_service;
 		$this->session_service          = $session_service;
+	}
 
+	/**
+	 * Initialise class hooks.
+	 *
+	 * @return void
+	 */
+	public function init_hooks() {
+		// Add admin init hooks.
 		add_action( 'admin_init', [ $this, 'maybe_handle_onboarding' ] );
 		add_action( 'admin_init', [ $this, 'maybe_redirect_to_onboarding' ], 11 ); // Run this after the WC setup wizard and onboarding redirection logic.
 		add_action( 'admin_init', [ $this, 'maybe_redirect_to_wcpay_connect' ], 12 ); // Run this after the redirect to onboarding logic.
+		add_action( 'admin_init', [ $this, 'maybe_redirect_to_capital_offer' ] );
+		add_action( 'admin_init', [ $this, 'maybe_redirect_to_server_link' ] );
+		add_action( 'admin_init', [ $this, 'maybe_activate_woopay' ] );
+
+		// Add handlers for inbox notes and reminders.
 		add_action( 'woocommerce_payments_account_refreshed', [ $this, 'handle_instant_deposits_inbox_note' ] );
 		add_action( 'woocommerce_payments_account_refreshed', [ $this, 'handle_loan_approved_inbox_note' ] );
 		add_action( self::INSTANT_DEPOSITS_REMINDER_ACTION, [ $this, 'handle_instant_deposits_inbox_reminder' ] );
+
+		// Add all other hooks.
 		add_filter( 'allowed_redirect_hosts', [ $this, 'allowed_redirect_hosts' ] );
 		add_action( 'jetpack_site_registered', [ $this, 'clear_cache' ] );
 		add_action( 'updated_option', [ $this, 'possibly_update_wcpay_account_locale' ], 10, 3 );
 		add_action( 'woocommerce_woocommerce_payments_updated', [ $this, 'clear_cache' ] );
-
-		// Add capital offer redirection.
-		add_action( 'admin_init', [ $this, 'maybe_redirect_to_capital_offer' ] );
-
-		// Add server links handler.
-		add_action( 'admin_init', [ $this, 'maybe_redirect_to_server_link' ] );
-		add_action( 'admin_init', [ $this, 'maybe_activate_woopay' ] );
 	}
 
 	/**
@@ -1199,7 +1207,7 @@ private function init_stripe_onboarding( $wcpay_connect_from, $additional_args =
 		// Clear persisted onboarding flow state.
 		WC_Payments_Onboarding_Service::clear_onboarding_flow_state();
 
-		$return_url   = $this->get_onboarding_return_url( $wcpay_connect_from );
+		$return_url = $this->get_onboarding_return_url( $wcpay_connect_from );
 		if ( ! empty( $additional_args ) ) {
 			$return_url = add_query_arg( $additional_args, $return_url );
 		}
@@ -1839,7 +1847,7 @@ private function get_onboarding_user_data(): array {
 				'accept_language'  => isset( $_SERVER['HTTP_ACCEPT_LANGUAGE'] ) ? wc_clean( wp_unslash( $_SERVER['HTTP_ACCEPT_LANGUAGE'] ) ) : '',
 				'content_language' => empty( get_user_locale() ) ? 'en-US' : str_replace( '_', '-', get_user_locale() ),
 			],
-			'referer'         => isset( $_SERVER['HTTP_REFERER'] ) ? sanitize_url( wp_unslash( $_SERVER['HTTP_REFERER'] ) ) : '',
+			'referer'         => isset( $_SERVER['HTTP_REFERER'] ) ? esc_url_raw( wp_unslash( $_SERVER['HTTP_REFERER'] ) ) : '',
 		];
 	}
 }

includes/class-wc-payments-incentives-service.php

@@ -30,7 +30,14 @@ class WC_Payments_Incentives_Service {
 	 */
 	public function __construct( Database_Cache $database_cache ) {
 		$this->database_cache = $database_cache;
+	}
 
+	/**
+	 * Initialise class hooks.
+	 *
+	 * @return void
+	 */
+	public function init_hooks() {
 		add_action( 'admin_menu', [ $this, 'add_payments_menu_badge' ] );
 		add_filter( 'woocommerce_admin_allowed_promo_notes', [ $this, 'allowed_promo_notes' ] );
 		add_filter( 'woocommerce_admin_woopayments_onboarding_task_badge', [ $this, 'onboarding_task_badge' ] );

includes/class-wc-payments-onboarding-service.php

@@ -43,7 +43,14 @@ class WC_Payments_Onboarding_Service {
 	public function __construct( WC_Payments_API_Client $payments_api_client, Database_Cache $database_cache ) {
 		$this->payments_api_client = $payments_api_client;
 		$this->database_cache      = $database_cache;
+	}
 
+	/**
+	 * Initialise class hooks.
+	 *
+	 * @return void
+	 */
+	public function init_hooks() {
 		add_filter( 'wcpay_dev_mode', [ $this, 'maybe_enable_dev_mode' ], 100 );
 		add_filter( 'admin_body_class', [ $this, 'add_admin_body_classes' ] );
 	}

includes/class-wc-payments.php

@@ -498,7 +498,11 @@ public static function init() {
 
 		( new WooPay_Scheduler( self::$api_client ) )->init();
 
+		// Initialise hooks.
+		self::$account->init_hooks();
 		self::$fraud_service->init_hooks();
+		self::$onboarding_service->init_hooks();
+		self::$incentives_service->init_hooks();
 
 		self::$legacy_card_gateway = new CC_Payment_Gateway( self::$api_client, self::$account, self::$customer_service, self::$token_service, self::$action_scheduler_service, self::$failed_transaction_rate_limiter, self::$order_service, self::$duplicate_payment_prevention_service, self::$localization_service );
 

tests/unit/test-class-wc-payments-account-capital.php

@@ -77,9 +77,10 @@ public function set_up() {
 
 		// Mock WC_Payments_Account without redirect_to to prevent headers already sent error.
 		$this->wcpay_account = $this->getMockBuilder( WC_Payments_Account::class )
-			->setMethods( [ 'redirect_to' ] )
+			->setMethods( [ 'redirect_to', 'init_hooks' ] )
 			->setConstructorArgs( [ $this->mock_api_client, $this->mock_database_cache, $this->mock_action_scheduler_service, $this->mock_session_service ] )
 			->getMock();
+		$this->wcpay_account->init_hooks();
 	}
 
 	public function tear_down() {
@@ -95,8 +96,10 @@ public function tear_down() {
 
 	public function test_maybe_redirect_to_capital_offer_will_run() {
 		$wcpay_account = $this->getMockBuilder( WC_Payments_Account::class )
+			->setMethodsExcept( [ 'maybe_redirect_to_capital_offer', 'init_hooks' ] )
 			->setConstructorArgs( [ $this->mock_api_client, $this->mock_database_cache, $this->mock_action_scheduler_service, $this->mock_session_service ] )
 			->getMock();
+		$wcpay_account->init_hooks();
 
 		$this->assertNotFalse(
 			has_action( 'admin_init', [ $wcpay_account, 'maybe_redirect_to_capital_offer' ] )

tests/unit/test-class-wc-payments-account-link.php

@@ -78,6 +78,8 @@ public function set_up() {
 			->setMethods( [ 'redirect_to' ] )
 			->setConstructorArgs( [ $this->mock_api_client, $this->mock_database_cache, $this->mock_action_scheduler_service, $this->mock_session_service ] )
 			->getMock();
+
+		$this->wcpay_account->init_hooks();
 	}
 
 	public function tear_down() {

tests/unit/test-class-wc-payments-account.php

@@ -79,6 +79,7 @@ public function set_up() {
 		$this->mock_session_service          = $this->createMock( WC_Payments_Session_Service::class );
 
 		$this->wcpay_account = new WC_Payments_Account( $this->mock_api_client, $this->mock_database_cache, $this->mock_action_scheduler_service, $this->mock_session_service );
+		$this->wcpay_account->init_hooks();
 	}
 
 	public function tear_down() {
@@ -87,17 +88,20 @@ public function tear_down() {
 		parent::tear_down();
 	}
 
-	/**
-	 * @param bool $can_manage_woocommerce
-	 *
-	 * @return Closure
-	 */
-	private function create_can_manage_woocommerce_cap_override( bool $can_manage_woocommerce ) {
-		return function ( $allcaps ) use ( $can_manage_woocommerce ) {
-			$allcaps['manage_woocommerce'] = $can_manage_woocommerce;
-
-			return $allcaps;
-		};
+	public function test_filters_registered_properly() {
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_handle_onboarding' ] ), 'maybe_handle_onboarding action does not exist.' );
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_redirect_to_onboarding' ] ), 'maybe_redirect_to_onboarding action does not exist.' );
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_redirect_to_wcpay_connect' ] ), 'maybe_redirect_to_wcpay_connect action does not exist.' );
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_redirect_to_capital_offer' ] ), 'maybe_redirect_to_capital_offer action does not exist.' );
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_redirect_to_server_link' ] ), 'maybe_redirect_to_server_link action does not exist.' );
+		$this->assertNotFalse( has_action( 'admin_init', [ $this->wcpay_account, 'maybe_activate_woopay' ] ), 'maybe_activate_woopay action does not exist.' );
+		$this->assertNotFalse( has_action( 'woocommerce_payments_account_refreshed', [ $this->wcpay_account, 'handle_instant_deposits_inbox_note' ] ), 'handle_instant_deposits_inbox_note action does not exist.' );
+		$this->assertNotFalse( has_action( 'woocommerce_payments_account_refreshed', [ $this->wcpay_account, 'handle_loan_approved_inbox_note' ] ), 'handle_loan_approved_inbox_note action does not exist.' );
+		$this->assertNotFalse( has_action( 'wcpay_instant_deposit_reminder', [ $this->wcpay_account, 'handle_instant_deposits_inbox_reminder' ] ), 'handle_instant_deposits_inbox_reminder action does not exist.' );
+		$this->assertNotFalse( has_filter( 'allowed_redirect_hosts', [ $this->wcpay_account, 'allowed_redirect_hosts' ] ), 'allowed_redirect_hooks filter does not exist.' );
+		$this->assertNotFalse( has_action( 'jetpack_site_registered', [ $this->wcpay_account, 'clear_cache' ] ), 'jetpack_site_registered action does not exist.' );
+		$this->assertNotFalse( has_action( 'updated_option', [ $this->wcpay_account, 'possibly_update_wcpay_account_locale' ] ), 'updated_option action does not exist.' );
+		$this->assertNotFalse( has_action( 'woocommerce_woocommerce_payments_updated', [ $this->wcpay_account, 'clear_cache' ] ), 'woocommerce_woocommerce_payments_updated action does not exist.' );
 	}
 
 	public function test_maybe_redirect_to_onboarding_stripe_disconnected_redirects() {
@@ -1200,6 +1204,19 @@ function ( $key, $generator, $validator ) {
 			);
 	}
 
+	/**
+	 * @param bool $can_manage_woocommerce
+	 *
+	 * @return Closure
+	 */
+	private function create_can_manage_woocommerce_cap_override( bool $can_manage_woocommerce ) {
+		return function ( $allcaps ) use ( $can_manage_woocommerce ) {
+			$allcaps['manage_woocommerce'] = $can_manage_woocommerce;
+
+			return $allcaps;
+		};
+	}
+
 	/**
 	 * Cache account details.
 	 *

tests/unit/test-class-wc-payments-incentives-service.php

@@ -34,6 +34,7 @@ public function set_up() {
 
 		$this->mock_database_cache = $this->createMock( Database_Cache::class );
 		$this->incentives_service  = new WC_Payments_Incentives_Service( $this->mock_database_cache );
+		$this->incentives_service->init_hooks();
 
 		global $menu;
 		// phpcs:ignore: WordPress.WP.GlobalVariablesOverride.Prohibited
@@ -49,27 +50,6 @@ public function tear_down() {
 		$menu = null; // phpcs:ignore: WordPress.WP.GlobalVariablesOverride.Prohibited
 	}
 
-	/**
-	 * Mocked incentive data.
-	 *
-	 * @var array
-	 */
-	private $mock_incentive_data = [
-		'incentive'    => [
-			'id'          => 'incentive_id',
-			'type'        => 'connect_page',
-			'description' => 'incentive_description',
-			'tc_url'      => 'incentive_tc_url',
-		],
-		// This is the hash of the test store context:
-		// 'country' => 'US',
-		// 'locale' => 'en_US',
-		// 'has_orders' => false,
-		// 'has_payments' => false,
-		// 'has_wcpay' => false.
-		'context_hash' => '6d37bc19d822af681f896b21065134c7',
-	];
-
 	public function test_filters_registered_properly() {
 		$this->assertNotFalse( has_action( 'admin_menu', [ $this->incentives_service, 'add_payments_menu_badge' ] ) );
 		$this->assertNotFalse( has_filter( 'woocommerce_admin_allowed_promo_notes', [ $this->incentives_service, 'allowed_promo_notes' ] ) );
@@ -267,4 +247,25 @@ function() use ( $response ) {
 			}
 		);
 	}
+
+	/**
+	 * Mocked incentive data.
+	 *
+	 * @var array
+	 */
+	private $mock_incentive_data = [
+		'incentive'    => [
+			'id'          => 'incentive_id',
+			'type'        => 'connect_page',
+			'description' => 'incentive_description',
+			'tc_url'      => 'incentive_tc_url',
+		],
+		// This is the hash of the test store context:
+		// 'country' => 'US',
+		// 'locale' => 'en_US',
+		// 'has_orders' => false,
+		// 'has_payments' => false,
+		// 'has_wcpay' => false.
+		'context_hash' => '6d37bc19d822af681f896b21065134c7',
+	];
 }

tests/unit/test-class-wc-payments-onboarding-service.php

@@ -132,6 +132,7 @@ public function set_up() {
 		$this->mock_database_cache = $this->createMock( Database_Cache::class );
 
 		$this->onboarding_service = new WC_Payments_Onboarding_Service( $this->mock_api_client, $this->mock_database_cache );
+		$this->onboarding_service->init_hooks();
 	}
 
 	public function test_filters_registered_properly() {