Merge remote-tracking branch 'origin' into Add/test-tools

.github/workflows/lint.yml

@@ -34,7 +34,7 @@ jobs:
 
       - uses: technote-space/get-diff-action@v6 # repo is archived.
         with:
-          SUFFIX_FILTER: .php
+          PATTERNS: ./**/*.php
 
       - name: Get Composer cache directory
         id: composer-cache
@@ -68,5 +68,5 @@ jobs:
           GITHUB_TOKEN: "${{ github.token }}"
 
       - name: Detecting PHP Code Standards Violations
-        run: vendor/bin/phpcs --standard=phpcs.xml -s ${{ env.GIT_DIFF }} --report=checkstyle | cs2pr
-        if: "!! env.GIT_DIFF"
+        run: vendor/bin/phpcs --standard=phpcs.xml -s ${{ env.GIT_DIFF_FILTERED }} --report=checkstyle | cs2pr
+        if: "!! env.GIT_DIFF_FILTERED"

.github/workflows/unit-tests-and-coverage-report.yml

@@ -15,7 +15,7 @@ on:
 
 jobs:
 
-  codecoverage-main:
+  unit-tests:
     runs-on: ubuntu-latest
 
     services:

composer.json

@@ -71,7 +71,7 @@
       "vendor/bin/phpcbf . --standard=phpcs.xml"
     ],
     "cs-changes": [
-      "updated_files=$( git status | grep '\\(new file\\|modified\\):\\s.*.php$' | cut -c14- | awk '{ printf(\"%s \", $0) }' ); echo \"\\nChecking\"$(git status | grep '\\(new file\\|modified\\):\\s.*.php$' | tail -n+2 | wc -l)\" files\"; phpcbf $(echo $updated_files); phpcs $(echo $updated_files);"
+      "updated_files=$(echo $(git diff --name-only `git merge-base origin/main HEAD` | grep php)); if [ -n \"$updated_files\" ]; then phpcbf $(echo $updated_files); phpcs $(echo $updated_files); else echo \"No modified .php files for PHPCS.\"; fi;"
     ],
     "lint": [
       "vendor/bin/phpcs . --standard=phpcs.xml -s"

includes/HiiveConnection.php

@@ -78,24 +78,32 @@ public function rest_api_init(): void {
 	 *
 	 * Hiive will first attempt to verify using the REST API, and fallback to this AJAX endpoint on error.
 	 *
+	 * Token is generated in {@see self::connect()} using {@see md5()}.
+	 *
 	 * @hooked wp_ajax_nopriv_nfd-hiive-verify
 	 *
 	 * @return never
 	 */
 	public function ajax_verify() {
-		$valid  = $this->verify_token( $_REQUEST['token'] );
-		$status = ( $valid ) ? 200 : 400;
+		// PHPCS: Ignore the nonce verification here – the token _is_ a nonce.
+		// @phpcs:ignore WordPress.Security.NonceVerification.Recommended
+		$token = $_REQUEST['token'];
+
+		$is_valid = $this->verify_token( $token );
+		$status   = ( $is_valid ) ? 200 : 400;
 
 		$data = array(
-			'token' => $_REQUEST['token'],
-			'valid' => $valid,
+			'token' => $token,
+			'valid' => $is_valid,
 		);
 		\wp_send_json( $data, $status );
 	}
 
 	/**
 	 * Confirm whether verification token is valid
 	 *
+	 * Token is generated in {@see self::connect()} using {@see md5()}.
+	 *
 	 * @param string $token Token to verify
 	 */
 	public function verify_token( string $token ): bool {
@@ -260,7 +268,6 @@ public function send_event( Event $event ) {
 		$hiive_response = $this->hiive_request( 'sites/v1/events', $payload );
 
 		if ( is_wp_error( $hiive_response ) ) {
-			// TODO: enqueue failed event for later. Should this function call go via EventManager?
 			return $hiive_response;
 		}
 

includes/Listeners/Plugin.php

@@ -21,11 +21,15 @@ class Plugin extends Listener {
 	/**
 	 * Constructor
 	 *
-	 * @param EventManager $manager Event manager instance
+	 * @param EventManager  $manager Event manager instance
+	 * @param ?PluginHelper $plugin_helper Class used to fetch plugin data.
 	 */
-	public function __construct( EventManager $manager ) {
+	public function __construct(
+		EventManager $manager,
+		?PluginHelper $plugin_helper = null
+	) {
 		parent::__construct( $manager );
-		$this->plugin_helper = new PluginHelper();
+		$this->plugin_helper = $plugin_helper ?? new PluginHelper();
 	}
 
 	/**

phpcs.xml

@@ -7,4 +7,7 @@
 
     <file>includes</file>
     <file>upgrades</file>
+
+    <!-- Show progress and sniff codes in all reports -->
+    <arg value="ps"/>
 </ruleset>

tests/phpunit/includes/Listeners/PluginTest.php

@@ -4,6 +4,7 @@
 
 use Mockery;
 use NewfoldLabs\WP\Module\Data\EventManager;
+use NewfoldLabs\WP\Module\Data\Helpers\Plugin as Plugin_Helper;
 use WP_Mock;
 
 /**
@@ -17,8 +18,6 @@ class PluginTest extends \WP_Mock\Tools\TestCase {
 	public function tearDown(): void {
 		parent::tearDown();
 
-		\Patchwork\restoreAll();
-
 		unset( $_SERVER['REMOTE_ADDR'] );
 		unset( $_SERVER['REQUEST_URI'] );
 	}
@@ -33,16 +32,18 @@ public function tearDown(): void {
 	public function upgrader_process_complete_data_provider(): array {
 		return array(
 			array(
-				'plugins'           => array(
+				'plugins'                => array(
 					'bluehost-wordpress-plugin/bluehost-wordpress-plugin.php',
 				),
-				'expect_push_times' => 1,
+				'expected_count_collect' => 1,
+				'expect_push_times'      => 1,
 			),
 			array(
-				'plugins'           => array(
+				'plugins'                => array(
 					'',
 				),
-				'expect_push_times' => 0,
+				'expected_count_collect' => 0,
+				'expect_push_times'      => 0,
 			),
 		);
 	}
@@ -63,7 +64,7 @@ public function upgrader_process_complete_data_provider(): array {
 	 * @param array $plugins          The plugins value sent to the `upgrader_process_complete` action.
 	 * @param int   $expect_push_times The number of times the `push` method should be called. I.e. 0 when there is no plugin.
 	 */
-	public function test_upgrader_process_complete_fired( array $plugins, int $expect_push_times ): void {
+	public function test_upgrader_process_complete_fired( array $plugins, int $expected_count_collect, int $expect_push_times ): void {
 
 		/**
 		 * It is difficult to mock the `Plugin_Upgrader` class, so we will just pass `null` for now.
@@ -79,10 +80,8 @@ public function test_upgrader_process_complete_fired( array $plugins, int $expec
 			'plugins' => $plugins,
 		);
 
-		$event_manager = Mockery::mock( EventManager::class );
-		$event_manager->expects( 'push' )->times( $expect_push_times );
-
-		$sut = new Plugin( $event_manager );
+		$event_manager_mock = Mockery::mock( EventManager::class );
+		$event_manager_mock->expects( 'push' )->times( $expect_push_times );
 
 		/**
 		 * This will only be called if the plugin is not empty, meaning we don't test with the current problematic
@@ -100,13 +99,13 @@ public function test_upgrader_process_complete_fired( array $plugins, int $expec
 			'auto_updates' => true,
 		);
 
-		\Patchwork\redefine(
-			array( \NewfoldLabs\WP\Module\Data\Helpers\Plugin::class, 'collect' ),
-			function () use ( $plugin_collected ) {
-					return $plugin_collected;
-			}
-		);
+		$plugin_helper_mock = Mockery::mock( Plugin_Helper::class )->makePartial();
+		$plugin_helper_mock->shouldReceive( 'collect' )
+						->times( $expected_count_collect )
+						->with( 'bluehost-wordpress-plugin/bluehost-wordpress-plugin.php' )
+						->andReturn( $plugin_collected );
 
+		$sut = new Plugin( $event_manager_mock, $plugin_helper_mock );
 		/**
 		 * The Event constructor calls a lot of WordPress functions to determine the environment.
 		 *