From 0ae4f8e484662a848c2c766d0ab487d08f771a4d Mon Sep 17 00:00:00 2001 From: Jacob Sewell Date: Mon, 11 Apr 2022 21:08:26 -0500 Subject: [PATCH 1/4] Add context param to Notes\DataStore::get_notes() and associated methods/filters. --- .../woocommerce/src/Admin/Notes/DataStore.php | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/plugins/woocommerce/src/Admin/Notes/DataStore.php b/plugins/woocommerce/src/Admin/Notes/DataStore.php index 0a7abdf954b..ea1949979b8 100644 --- a/plugins/woocommerce/src/Admin/Notes/DataStore.php +++ b/plugins/woocommerce/src/Admin/Notes/DataStore.php @@ -11,6 +11,9 @@ defined( 'ABSPATH' ) || exit; * WC Admin Note Data Store (Custom Tables) */ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Interface { + // Extensions should define their own contexts and use them to avoid applying woocommerce_note_where_clauses when not needed. + const WC_ADMIN_NOTE_OPER_GLOBAL = 'global'; + /** * Method to create a new note in the database. * @@ -324,9 +327,10 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * Return an ordered list of notes. * * @param array $args Query arguments. + * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. * @return array An array of objects containing a note id. */ - public function get_notes( $args = array() ) { + public function get_notes( $args = array(), $context = self::WC_ADMIN_NOTE_OPER_GLOBAL ) { global $wpdb; $defaults = array( @@ -338,7 +342,7 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter $args = wp_parse_args( $args, $defaults ); $offset = $args['per_page'] * ( $args['page'] - 1 ); - $where_clauses = $this->get_notes_where_clauses( $args ); + $where_clauses = $this->get_notes_where_clauses( $args, $context ); $query = $wpdb->prepare( // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.PreparedSQL.InterpolatedNotPrepared @@ -378,16 +382,18 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * * @param string $type Comma separated list of note types. * @param string $status Comma separated list of statuses. + * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. * @return array An array of objects containing a note id. */ - public function get_notes_count( $type = array(), $status = array() ) { + public function get_notes_count( $type = array(), $status = array(), $context = self::WC_ADMIN_NOTE_OPER_GLOBAL ) { global $wpdb; $where_clauses = $this->get_notes_where_clauses( array( 'type' => $type, 'status' => $status, - ) + ), + $context ); if ( ! empty( $where_clauses ) ) { @@ -426,9 +432,10 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * * @uses args_to_where_clauses * @param array $args Array of args to pass. + * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. * @return string Where clauses for the query. */ - public function get_notes_where_clauses( $args = array() ) { + public function get_notes_where_clauses( $args = array(), $context = self::WC_ADMIN_NOTE_OPER_GLOBAL ) { $where_clauses = $this->args_to_where_clauses( $args ); /** @@ -438,8 +445,9 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * * @param string $where_clauses The generated WHERE clause. * @param array $args The original arguments for the request. + * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. */ - return apply_filters( 'woocommerce_note_where_clauses', $where_clauses, $args ); + return apply_filters( 'woocommerce_note_where_clauses', $where_clauses, $args, $context ); } /** From e83631983a6339f0ae28036b680a20d33f4ad027 Mon Sep 17 00:00:00 2001 From: Jacob Sewell Date: Mon, 11 Apr 2022 21:09:04 -0500 Subject: [PATCH 2/4] Add unit tests for new context param in Notes\DataStore::get_notes(). --- .../notes/class-wc-tests-notes-data-store.php | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/notes/class-wc-tests-notes-data-store.php b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/notes/class-wc-tests-notes-data-store.php index c36be43720d..b4445dab708 100644 --- a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/notes/class-wc-tests-notes-data-store.php +++ b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/notes/class-wc-tests-notes-data-store.php @@ -7,6 +7,7 @@ use \Automattic\WooCommerce\Admin\Notes\Notes; use \Automattic\WooCommerce\Admin\Notes\Note; +use \Automattic\WooCommerce\Admin\Notes\DataStore; /** * Class WC_Admin_Tests_Notes_Data_Store @@ -344,6 +345,92 @@ class WC_Admin_Tests_Notes_Data_Store extends WC_Unit_Test_Case { $this->assertEquals( $lookup_note_zero->get_id(), $get_note_zero->get_id() ); } + /** + * Test that get_notes properly handles the $context parameter + */ + public function test_get_notes_context_param() { + $data_store = WC_Data_Store::load( 'admin-note' ); + + // Create two Notes: one in context and one out of it. + $test_context = self::class; + $global_context = DataStore::WC_ADMIN_NOTE_OPER_GLOBAL; + $context_name = 'PHP_UNIT_IN_CONTEXT_TEST_NOTE'; + $out_of_context_name = 'PHP_UNIT_OUT_OF_CONTEXT_TEST_NOTE'; + + foreach ( array( $context_name, $out_of_context_name ) as $note_name ) { + $note = new Note(); + $note->set_name( $note_name ); + $note->set_title( 'PHPUNIT_CONTEXT_TEST_NOTE' ); + $note->set_content( 'PHPUNIT_CONTEXT_TEST_NOTE_CONTENT' ); + $note->set_type( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ); + $note->set_source( 'PHPUNIT_TEST' ); + $note->set_is_snoozable( false ); + $note->set_layout( 'plain' ); + $note->set_image( '' ); + $note->add_action( + 'PHPUNIT_TEST_ACTION_SLUG', + 'PHPUNIT_TEST_ACTION_LABEL', + '?s=PHPUNIT_TEST_ACTION_URL' + ); + $note->set_is_deleted( false ); + $note->save(); + } + + // Add filter for 'woocommerce_note_where_clauses' that applies only in context. + $context_filter_hit_count = 0; + $context_filter_callback = function( $where_clauses, $args, $context ) use ( $test_context, $global_context, $context_name, &$context_filter_hit_count ) { + if ( $context === $test_context ) { + $context_filter_hit_count++; + $where_clauses .= ' AND name = "' . $context_name . '"'; + } + return $where_clauses; + }; + add_filter( 'woocommerce_note_where_clauses', $context_filter_callback, 10, 3 ); + + // Add filter for 'woocommerce_note_where_clauses' that applies in any context. + $no_context_filter_hit_count = 0; + $global_context_received = null; + $no_context_filter_callback = function( $where_clauses, $args, $context ) use ( $test_context, &$global_context_received, &$no_context_filter_hit_count ) { + // Record the context we get passed in. + if ( $test_context !== $context ) { + $global_context_received = $context; + } + + // Record that we're here. + $no_context_filter_hit_count++; + $where_clauses .= ' AND source = "PHPUNIT_TEST"'; + return $where_clauses; + }; + add_filter( 'woocommerce_note_where_clauses', $no_context_filter_callback, 10, 3 ); + + // Get note counts. + $no_context_note_count = $data_store->get_notes_count( array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ), array() ); + $test_context_note_count = $data_store->get_notes_count( array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ), array(), $test_context ); + + // The no context filter should have been hit twice, the context filter once. + $this->assertEquals( 2, $no_context_filter_hit_count ); + $this->assertEquals( 1, $context_filter_hit_count ); + + // There should be only one note that satisfies the context filter. + $this->assertEquals( 1, $test_context_note_count ); + + // There should be more than one note that satisfies the no-context filter. + $this->assertGreaterThan( 1, $no_context_note_count ); + + // Same tests with get_notes(). + $no_context_get_notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ) ) ); + $test_context_get_notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ) ), $test_context ); + + // There should only be one note in the context result set. + $this->assertEquals( 1, count( $test_context_get_notes ) ); + + // There should be more than one note in the no-context result set. + $this->assertGreaterThan( 1, count( $no_context_get_notes ) ); + + // When not explicitly passed, context should default to WC_ADMIN_NOTE_OPER_GLOBAL. + $this->assertEquals( $global_context, $global_context_received ); + } + /** * Delete notes created by this class's tests. */ From e1db7ab0f2f1a54de7169c382f5dac88ab989960 Mon Sep 17 00:00:00 2001 From: Jacob Sewell Date: Mon, 11 Apr 2022 21:17:59 -0500 Subject: [PATCH 3/4] Changelog for 32573/32574. --- .../woocommerce/changelog/add-32573-context-arg-for-get-notes | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 plugins/woocommerce/changelog/add-32573-context-arg-for-get-notes diff --git a/plugins/woocommerce/changelog/add-32573-context-arg-for-get-notes b/plugins/woocommerce/changelog/add-32573-context-arg-for-get-notes new file mode 100644 index 00000000000..d576cf03ea3 --- /dev/null +++ b/plugins/woocommerce/changelog/add-32573-context-arg-for-get-notes @@ -0,0 +1,4 @@ +Significance: minor +Type: performance + +Add a context param with a default value of global to Admin\Notes\DataStore::get_notes(), get_notes_count(), and get_notes_where_clauses(). #32574 From 4b9c44ebb37e1b53519f048156ddb7dff53e8d27 Mon Sep 17 00:00:00 2001 From: Jacob Sewell Date: Tue, 12 Apr 2022 15:46:02 -0500 Subject: [PATCH 4/4] Docblock linter fixes. --- plugins/woocommerce/src/Admin/Notes/DataStore.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/woocommerce/src/Admin/Notes/DataStore.php b/plugins/woocommerce/src/Admin/Notes/DataStore.php index ea1949979b8..49b14e500b3 100644 --- a/plugins/woocommerce/src/Admin/Notes/DataStore.php +++ b/plugins/woocommerce/src/Admin/Notes/DataStore.php @@ -326,7 +326,7 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter /** * Return an ordered list of notes. * - * @param array $args Query arguments. + * @param array $args Query arguments. * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. * @return array An array of objects containing a note id. */ @@ -431,7 +431,7 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * Applies woocommerce_note_where_clauses filter. * * @uses args_to_where_clauses - * @param array $args Array of args to pass. + * @param array $args Array of args to pass. * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. * @return string Where clauses for the query. */