From 038b60fd312e4de910529036a7bface0bd59f406 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Fri, 15 Nov 2024 15:03:59 -0800 Subject: [PATCH 01/11] Add default paths so it runs without parameters --- phpcs.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpcs.xml b/phpcs.xml index e1bf8e02..b0747441 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -3,4 +3,8 @@ + + + ./includes + ./upgrades From 67f252a538c985e6436a0612572d91354713e60a Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Fri, 15 Nov 2024 15:05:35 -0800 Subject: [PATCH 02/11] Phive install `brianhenryie/php-diff-test` --- .phive/phars.xml | 4 ++++ tools/php-diff-test | 1 + 2 files changed, 5 insertions(+) create mode 100644 .phive/phars.xml create mode 120000 tools/php-diff-test diff --git a/.phive/phars.xml b/.phive/phars.xml new file mode 100644 index 00000000..6869f4f1 --- /dev/null +++ b/.phive/phars.xml @@ -0,0 +1,4 @@ + + + + diff --git a/tools/php-diff-test b/tools/php-diff-test new file mode 120000 index 00000000..7b0621bc --- /dev/null +++ b/tools/php-diff-test @@ -0,0 +1 @@ +/Users/brian.henry/.phive/phars/brianhenryie/php-diff-test-0.7.1.phar \ No newline at end of file From 0f89d2eb3ca46cb0c014d4949158c0f144c305ab Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Fri, 15 Nov 2024 15:44:21 -0800 Subject: [PATCH 03/11] Revert "Phive install `brianhenryie/php-diff-test`" This reverts commit 67f252a538c985e6436a0612572d91354713e60a. --- .phive/phars.xml | 4 ---- tools/php-diff-test | 1 - 2 files changed, 5 deletions(-) delete mode 100644 .phive/phars.xml delete mode 120000 tools/php-diff-test diff --git a/.phive/phars.xml b/.phive/phars.xml deleted file mode 100644 index 6869f4f1..00000000 --- a/.phive/phars.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/tools/php-diff-test b/tools/php-diff-test deleted file mode 120000 index 7b0621bc..00000000 --- a/tools/php-diff-test +++ /dev/null @@ -1 +0,0 @@ -/Users/brian.henry/.phive/phars/brianhenryie/php-diff-test-0.7.1.phar \ No newline at end of file From 1af8eb2c76f897fbb8f870a81eecb380b97b38b2 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 09:29:08 -0800 Subject: [PATCH 04/11] Allow injecting `Helpers/Plugin` --- includes/Listeners/Plugin.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/includes/Listeners/Plugin.php b/includes/Listeners/Plugin.php index 82f7e370..1bbc4c41 100644 --- a/includes/Listeners/Plugin.php +++ b/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(); } /** From 3626318c312dc29f881d1d9a5e77cc7d41a2f691 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 09:53:50 -0800 Subject: [PATCH 05/11] Use Mockery instead of Patchwork. --- .../phpunit/includes/Listeners/PluginTest.php | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/phpunit/includes/Listeners/PluginTest.php b/tests/phpunit/includes/Listeners/PluginTest.php index 95f5599f..cabe175e 100644 --- a/tests/phpunit/includes/Listeners/PluginTest.php +++ b/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. * From d10b59b4dd22ddc67a8a3b659776cb234862423f Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:19:38 -0800 Subject: [PATCH 06/11] Update deprecated diff filter config --- .github/workflows/lint.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a9f738e8..63b695ca 100644 --- a/.github/workflows/lint.yml +++ b/.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" From 6436c2b10cd2228fa79abb6a1a2f147f98688f24 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:32:45 -0800 Subject: [PATCH 07/11] Use diff in `compose cs-changes` script --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 1b573dcc..84255c0f 100644 --- a/composer.json +++ b/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" From dda58c884f57f1fee47106a9c035c09baa96d4cc Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:32:59 -0800 Subject: [PATCH 08/11] Show progress and sniff codes in all reports --- phpcs.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpcs.xml b/phpcs.xml index 1c6f8e23..8d5883cc 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -7,4 +7,7 @@ includes upgrades + + + From 85c4630d7a53993c69f5d402d9eea3db7db273a8 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:33:20 -0800 Subject: [PATCH 09/11] =?UTF-8?q?Remove=20TODO=20=E2=80=93=20covered=20by?= =?UTF-8?q?=20a=20broad=20Jira=20ticket?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- includes/HiiveConnection.php | 1 - 1 file changed, 1 deletion(-) diff --git a/includes/HiiveConnection.php b/includes/HiiveConnection.php index 576f5248..4d6e0d42 100644 --- a/includes/HiiveConnection.php +++ b/includes/HiiveConnection.php @@ -260,7 +260,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; } From 0db708466ec373f13a7051b88727e96ff3567ab3 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:33:37 -0800 Subject: [PATCH 10/11] `phpcs:ignore` the verify token, which is a nonce --- includes/HiiveConnection.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/includes/HiiveConnection.php b/includes/HiiveConnection.php index 4d6e0d42..abb2ecb0 100644 --- a/includes/HiiveConnection.php +++ b/includes/HiiveConnection.php @@ -78,17 +78,23 @@ 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 ); } @@ -96,6 +102,8 @@ public function ajax_verify() { /** * 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 { From 5ec6f299f16a1fc23689c452fd7cbe64dffc0521 Mon Sep 17 00:00:00 2001 From: Brian Henry Date: Mon, 18 Nov 2024 12:35:07 -0800 Subject: [PATCH 11/11] Rename `codecoverage-main` to `unit-tests` --- .github/workflows/unit-tests-and-coverage-report.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests-and-coverage-report.yml b/.github/workflows/unit-tests-and-coverage-report.yml index ef254c01..66493ff4 100644 --- a/.github/workflows/unit-tests-and-coverage-report.yml +++ b/.github/workflows/unit-tests-and-coverage-report.yml @@ -15,7 +15,7 @@ on: jobs: - codecoverage-main: + unit-tests: runs-on: ubuntu-latest services: