From 8292ff1cca0336b6c9fffbf202e652aa15c6bb2f Mon Sep 17 00:00:00 2001 From: Ron Rennick Date: Thu, 14 Dec 2023 08:55:34 -0400 Subject: [PATCH 1/6] Add InternalInjection sniff exceptions for blocks --- phpcs.xml | 4 ++++ plugins/woocommerce/changelog/add-blocks-injection-rule | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 plugins/woocommerce/changelog/add-blocks-injection-rule diff --git a/phpcs.xml b/phpcs.xml index fd355d33463..c359cb47c24 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -113,12 +113,16 @@ src/Internal/Admin/ src/Admin/ + src/Blocks/ + src/StoreApi/ src/Internal/Admin/ src/Admin/ + src/Blocks/ + src/StoreApi/ diff --git a/plugins/woocommerce/changelog/add-blocks-injection-rule b/plugins/woocommerce/changelog/add-blocks-injection-rule new file mode 100644 index 00000000000..df513a6a407 --- /dev/null +++ b/plugins/woocommerce/changelog/add-blocks-injection-rule @@ -0,0 +1,4 @@ +Significance: minor +Type: update + +Add InternalInjection sniff exceptions for blocks From 743b26d4981e51cc64a5a10cd286a5a9e886739f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 14 Dec 2023 14:27:42 +0100 Subject: [PATCH 2/6] Performance: Log the performance metrics to codevitals --- .github/workflows/metrics.yml | 8 +++ tools/compare-perf/log-to-codevitals.js | 86 +++++++++++++++++++++++++ tools/compare-perf/package.json | 3 +- 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 tools/compare-perf/log-to-codevitals.js diff --git a/.github/workflows/metrics.yml b/.github/workflows/metrics.yml index da5de8e5312..4e3db893dd1 100644 --- a/.github/workflows/metrics.yml +++ b/.github/workflows/metrics.yml @@ -51,3 +51,11 @@ jobs: with: name: performance-results path: ${{ env.WP_ARTIFACTS_PATH }}/*.performance-results*.json + + - name: Publish performance results + if: github.event_name == 'pull_request' + env: + CODEVITALS_PROJECT_TOKEN: ${{ secrets.CODEVITALS_PROJECT_TOKEN }} + run: | + COMMITTED_AT=$(git show -s $GITHUB_SHA --format="%cI") + cd tools/compare-perf && pnpm run log $CODEVITALS_PROJECT_TOKEN trunk $GITHUB_SHA 2f6ca66e00b3666b2567877ae67cbfb5b6ce171a $COMMITTED_AT diff --git a/tools/compare-perf/log-to-codevitals.js b/tools/compare-perf/log-to-codevitals.js new file mode 100644 index 00000000000..bf772f7d384 --- /dev/null +++ b/tools/compare-perf/log-to-codevitals.js @@ -0,0 +1,86 @@ +#!/usr/bin/env node +/* eslint-disable no-console */ +const fs = require( 'fs' ); +const path = require( 'path' ); +const https = require( 'https' ); +const [ token, branch, hash, baseHash, timestamp ] = process.argv.slice( 2 ); + +const resultsFiles = [ + { + file: 'editor.performance-results.json', + metricsPrefix: 'editor', + }, +]; + +const performanceResults = resultsFiles.map( ( { file } ) => + JSON.parse( + fs.readFileSync( + path.join( process.env.WP_ARTIFACTS_PATH, file ), + 'utf8' + ) + ) +); + +const data = new TextEncoder().encode( + JSON.stringify( { + branch, + hash, + baseHash, + timestamp, + metrics: resultsFiles.reduce( ( result, { metricsPrefix }, index ) => { + return { + ...result, + ...Object.fromEntries( + Object.entries( + performanceResults[ index ][ hash ] ?? {} + ).map( ( [ key, value ] ) => [ + metricsPrefix + key, + value, + ] ) + ), + }; + }, {} ), + baseMetrics: resultsFiles.reduce( + ( result, { metricsPrefix }, index ) => { + return { + ...result, + ...Object.fromEntries( + Object.entries( + performanceResults[ index ][ baseHash ] ?? {} + ).map( ( [ key, value ] ) => [ + metricsPrefix + key, + value, + ] ) + ), + }; + }, + {} + ), + } ) +); + +const options = { + hostname: 'codevitals.run', + port: 443, + path: '/api/log?token=' + token, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': data.length, + }, +}; + +const req = https.request( options, ( res ) => { + console.log( `statusCode: ${ res.statusCode }` ); + + res.on( 'data', ( d ) => { + process.stdout.write( d ); + } ); +} ); + +req.on( 'error', ( error ) => { + console.error( error ); +} ); + +req.write( data ); +req.end(); diff --git a/tools/compare-perf/package.json b/tools/compare-perf/package.json index 77c193c56ff..36ae6fe2746 100644 --- a/tools/compare-perf/package.json +++ b/tools/compare-perf/package.json @@ -7,7 +7,8 @@ "license": "GPLv2", "repository": "woocommerce/woocommerce", "scripts": { - "compare": "node index.js" + "compare": "node index.js", + "log": "node log-to-codevitals.js" }, "dependencies": { "@wordpress/env": "^8.13.0", From 28e876e7cd40b58dcb7ec21d1feace63764c1163 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 14 Dec 2023 14:56:17 +0100 Subject: [PATCH 3/6] Fix URL --- tools/compare-perf/log-to-codevitals.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/compare-perf/log-to-codevitals.js b/tools/compare-perf/log-to-codevitals.js index bf772f7d384..cc6823c2f9a 100644 --- a/tools/compare-perf/log-to-codevitals.js +++ b/tools/compare-perf/log-to-codevitals.js @@ -60,7 +60,7 @@ const data = new TextEncoder().encode( ); const options = { - hostname: 'codevitals.run', + hostname: 'www.codevitals.run', port: 443, path: '/api/log?token=' + token, method: 'POST', From 8c09ae38172262dfebdfd362a98c2526c15e0645 Mon Sep 17 00:00:00 2001 From: Ron Rennick Date: Thu, 14 Dec 2023 10:07:41 -0400 Subject: [PATCH 4/6] add exclusion for short array syntax --- phpcs.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpcs.xml b/phpcs.xml index c359cb47c24..7324030f0aa 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -107,6 +107,8 @@ src/Internal/Admin/ src/Admin/ + src/Blocks/ + src/StoreApi/ From 6f902e3dbb71d869f43f3528a35f3184067c764b Mon Sep 17 00:00:00 2001 From: Leif Singer Date: Thu, 14 Dec 2023 15:24:09 +0100 Subject: [PATCH 5/6] Delete trashed orders after `EMPTY_TRASH_DAYS` as defined by WordPress (HPOS) (#41949) * Add a test that fails if trashed orders are never deleted under HPOS * Delete trashed orders after `EMPTY_TRASH_DAYS` as defined by WordPress * add the changelog file * appease the linter * return early if HPOS is not authorative * attempt to precede WordPress itself (which uses priority 10) to increase the probability of orders and their posts being handled solely by us * appease the linter --- plugins/woocommerce/changelog/fix-41249 | 4 ++ .../DataStores/Orders/DataSynchronizer.php | 44 +++++++++++++++ .../Orders/DataSynchronizerTests.php | 54 ++++++++++++++++++- 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 plugins/woocommerce/changelog/fix-41249 diff --git a/plugins/woocommerce/changelog/fix-41249 b/plugins/woocommerce/changelog/fix-41249 new file mode 100644 index 00000000000..762c0a25ed5 --- /dev/null +++ b/plugins/woocommerce/changelog/fix-41249 @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Delete trashed orders after `EMPTY_TRASH_DAYS` as defined by WordPress (HPOS) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php index 5d6a383b636..235f4787816 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php @@ -80,6 +80,13 @@ class DataSynchronizer implements BatchProcessorInterface { */ private $error_logger; + /** + * The instance of the LegacyProxy object to use. + * + * @var LegacyProxy + */ + private $legacy_proxy; + /** * The order cache controller. * @@ -103,6 +110,7 @@ class DataSynchronizer implements BatchProcessorInterface { self::add_action( 'woocommerce_refund_created', array( $this, 'handle_updated_order' ), 100 ); self::add_action( 'woocommerce_update_order', array( $this, 'handle_updated_order' ), 100 ); self::add_action( 'wp_scheduled_auto_draft_delete', array( $this, 'delete_auto_draft_orders' ), 9 ); + self::add_action( 'wp_scheduled_delete', array( $this, 'delete_trashed_orders' ), 9 ); self::add_filter( 'updated_option', array( $this, 'process_updated_option' ), 999, 3 ); self::add_filter( 'added_option', array( $this, 'process_added_option' ), 999, 2 ); self::add_filter( 'deleted_option', array( $this, 'process_deleted_option' ), 999 ); @@ -136,6 +144,7 @@ class DataSynchronizer implements BatchProcessorInterface { $this->data_store = $data_store; $this->database_util = $database_util; $this->posts_to_cot_migrator = $posts_to_cot_migrator; + $this->legacy_proxy = $legacy_proxy; $this->error_logger = $legacy_proxy->call_function( 'wc_get_logger' ); $this->order_cache_controller = $order_cache_controller; $this->batch_processing_controller = $batch_processing_controller; @@ -966,6 +975,41 @@ ORDER BY orders.id ASC do_action( 'woocommerce_scheduled_auto_draft_delete' ); } + /** + * Handles deletion of trashed orders after `EMPTY_TRASH_DAYS` as defined by WordPress. + * + * @since 8.5.0 + * + * @return void + */ + private function delete_trashed_orders() { + if ( ! $this->custom_orders_table_is_authoritative() ) { + return; + } + + $delete_timestamp = $this->legacy_proxy->call_function( 'time' ) - ( DAY_IN_SECONDS * EMPTY_TRASH_DAYS ); + $args = array( + 'status' => 'trash', + 'limit' => self::ORDERS_SYNC_BATCH_SIZE, + 'date_modified' => '<' . $delete_timestamp, + ); + + $orders = wc_get_orders( $args ); + if ( ! $orders || ! is_array( $orders ) ) { + return; + } + + foreach ( $orders as $order ) { + if ( $order->get_status() !== 'trash' ) { + continue; + } + if ( $order->get_date_modified()->getTimestamp() >= $delete_timestamp ) { + continue; + } + $order->delete( true ); + } + } + /** * Handle the 'woocommerce_feature_description_tip' filter. * diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php index d64ae7ae3b3..52a4eeab8c6 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -25,13 +25,18 @@ class DataSynchronizerTests extends HposTestCase { */ public function setUp(): void { parent::setUp(); + + $this->reset_legacy_proxy_mocks(); + $container = wc_get_container(); + $container->reset_all_resolved(); + // Remove the Test Suite’s use of temporary tables https://wordpress.stackexchange.com/a/220308. remove_filter( 'query', array( $this, '_create_temporary_tables' ) ); remove_filter( 'query', array( $this, '_drop_temporary_tables' ) ); OrderHelper::delete_order_custom_tables(); // We need this since non-temporary tables won't drop automatically. OrderHelper::create_order_custom_table_if_not_exist(); OrderHelper::toggle_cot_feature_and_usage( false ); - $this->sut = wc_get_container()->get( DataSynchronizer::class ); + $this->sut = $container->get( DataSynchronizer::class ); } /** @@ -530,6 +535,53 @@ class DataSynchronizerTests extends HposTestCase { $this->assertNotContains( $order1->get_id(), $orders ); } + /** + * Test that trashed orders are deleted after the time set in `EMPTY_TRASH_DAYS`. + */ + public function test_trashed_order_deletion(): void { + $this->toggle_cot_authoritative( true ); + $this->disable_cot_sync(); + + $order = new WC_Order(); + $order->save(); + + // Ensure the placeholder post is there. + $placeholder = get_post( $order->get_id() ); + $this->assertEquals( $order->get_id(), $placeholder->ID ); + + // Trashed orders should be deleted by the collection mechanism. + $order->get_data_store()->delete( $order ); + $this->assertEquals( $order->get_status(), 'trash' ); + $order->save(); + + // Run scheduled deletion. + do_action( 'wp_scheduled_delete' ); // phpcs:ignore WooCommerce.Commenting.CommentHooks.HookCommentWrongStyle + + // Refresh order and ensure it's *not* gone. + $order = wc_get_order( $order->get_id() ); + $this->assertNotNull( $order ); + + // Time-travel into the future so that the time required to delete a trashed order has passed. + $this->register_legacy_proxy_function_mocks( + array( + 'time' => function() { + return time() + DAY_IN_SECONDS * EMPTY_TRASH_DAYS + 1; + }, + ) + ); + + // Run scheduled deletion. + do_action( 'wp_scheduled_delete' ); // phpcs:ignore WooCommerce.Commenting.CommentHooks.HookCommentWrongStyle + + // Ensure the placeholder post is gone. + $placeholder = get_post( $order->get_id() ); + $this->assertNull( $placeholder ); + + // Refresh order and ensure it's gone. + $order = wc_get_order( $order->get_id() ); + $this->assertFalse( $order ); + } + /** * @testDox When HPOS is enabled, the custom orders table is created. */ From e1b7c42256af492773fa3e26f584e3945b065ab6 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 14 Dec 2023 15:33:42 +0100 Subject: [PATCH 6/6] try sending the right hashes --- .github/workflows/metrics.yml | 4 ++-- tools/compare-perf/log-to-codevitals.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/metrics.yml b/.github/workflows/metrics.yml index 4e3db893dd1..61d5f3efede 100644 --- a/.github/workflows/metrics.yml +++ b/.github/workflows/metrics.yml @@ -53,9 +53,9 @@ jobs: path: ${{ env.WP_ARTIFACTS_PATH }}/*.performance-results*.json - name: Publish performance results - if: github.event_name == 'pull_request' + if: github.event_name == 'push' env: CODEVITALS_PROJECT_TOKEN: ${{ secrets.CODEVITALS_PROJECT_TOKEN }} run: | COMMITTED_AT=$(git show -s $GITHUB_SHA --format="%cI") - cd tools/compare-perf && pnpm run log $CODEVITALS_PROJECT_TOKEN trunk $GITHUB_SHA 2f6ca66e00b3666b2567877ae67cbfb5b6ce171a $COMMITTED_AT + cd tools/compare-perf && pnpm run log $CODEVITALS_PROJECT_TOKEN trunk $GITHUB_SHA 19f3d0884617d7ecdcf37664f648a51e2987cada $COMMITTED_AT diff --git a/tools/compare-perf/log-to-codevitals.js b/tools/compare-perf/log-to-codevitals.js index cc6823c2f9a..3b4e31a6a1e 100644 --- a/tools/compare-perf/log-to-codevitals.js +++ b/tools/compare-perf/log-to-codevitals.js @@ -8,7 +8,7 @@ const [ token, branch, hash, baseHash, timestamp ] = process.argv.slice( 2 ); const resultsFiles = [ { file: 'editor.performance-results.json', - metricsPrefix: 'editor', + metricsPrefix: 'editor-', }, ];