From 9c18a427fa3a338ebf9089b8d949c33a31053b41 Mon Sep 17 00:00:00 2001 From: Jacob Sewell Date: Thu, 3 Feb 2022 15:27:55 -0600 Subject: [PATCH] Avoid `get_notes` call in `CouponPageMoved` (https://github.com/woocommerce/woocommerce-admin/pull/8202) * First draft of CouponPageMoved::has_(unactioned|dismissed)_note() method changes to avoid get_notes(). * Add static function get_note_by_name( $note_name ) to Automattic\WooCommerce\Admin\Notes\Notes class. * Use Notes::get_note_by_name() in Notes::get_note_status(). * Use new Notes::get_note_by_name() in CouponPageMoved::has_unactioned_note(). * Use new Notes::get_note_by_name() in CouponPageMoved::has_dismissed_note(). * Add changelog for 7986/8202. --- ...ix-7986-avoid-get_notes-in-CouponPageMoved | 4 +++ .../src/Notes/CouponPageMoved.php | 27 +++++++++---------- plugins/woocommerce-admin/src/Notes/Notes.php | 27 +++++++++++++++---- 3 files changed, 38 insertions(+), 20 deletions(-) create mode 100644 plugins/woocommerce-admin/changelogs/fix-7986-avoid-get_notes-in-CouponPageMoved diff --git a/plugins/woocommerce-admin/changelogs/fix-7986-avoid-get_notes-in-CouponPageMoved b/plugins/woocommerce-admin/changelogs/fix-7986-avoid-get_notes-in-CouponPageMoved new file mode 100644 index 00000000000..b43515f737c --- /dev/null +++ b/plugins/woocommerce-admin/changelogs/fix-7986-avoid-get_notes-in-CouponPageMoved @@ -0,0 +1,4 @@ +Significance: minor +Type: Performance + +Avoid expensive get_notes() queries in CouponPageMoved admin_init actions by using new Notes::get_note_by_name() helper method. #8202 diff --git a/plugins/woocommerce-admin/src/Notes/CouponPageMoved.php b/plugins/woocommerce-admin/src/Notes/CouponPageMoved.php index dcabd72f0a9..fc7e9cfd1fa 100644 --- a/plugins/woocommerce-admin/src/Notes/CouponPageMoved.php +++ b/plugins/woocommerce-admin/src/Notes/CouponPageMoved.php @@ -94,15 +94,13 @@ class CouponPageMoved { * @return bool */ protected static function has_unactioned_note() { - $notes = self::get_data_store()->get_notes( - [ - 'name' => [ self::NOTE_NAME ], - 'status' => [ 'unactioned' ], - 'is_deleted' => false, - ] - ); + $note = Notes::get_note_by_name( self::NOTE_NAME ); - return ! empty( $notes ); + if ( ! $note ) { + return false; + } + + return $note->get_status() === 'unactioned'; } /** @@ -111,14 +109,13 @@ class CouponPageMoved { * @return bool */ protected static function has_dismissed_note() { - $notes = self::get_data_store()->get_notes( - [ - 'name' => [ self::NOTE_NAME ], - 'is_deleted' => true, - ] - ); + $note = Notes::get_note_by_name( self::NOTE_NAME ); - return ! empty( $notes ); + if ( ! $note ) { + return false; + } + + return ! $note->get_is_deleted(); } /** diff --git a/plugins/woocommerce-admin/src/Notes/Notes.php b/plugins/woocommerce-admin/src/Notes/Notes.php index 1de4b243812..9fbe943580d 100644 --- a/plugins/woocommerce-admin/src/Notes/Notes.php +++ b/plugins/woocommerce-admin/src/Notes/Notes.php @@ -82,6 +82,26 @@ class Notes { return false; } + /** + * Get admin note using its name. + * + * This is a shortcut for the common pattern of looking up note ids by name and then passing the first id to get_note(). + * It will behave unpredictably when more than one note with the given name exists. + * + * @param string $note_name Note name. + * @return Note|bool + **/ + public static function get_note_by_name( $note_name ) { + $data_store = self::load_data_store(); + $note_ids = $data_store->get_notes_with_name( $note_name ); + + if ( empty( $note_ids ) ) { + return false; + } + + return self::get_note( $note_ids[0] ); + } + /** * Get the total number of notes * @@ -296,15 +316,12 @@ class Notes { * @return string|bool The note status. */ public static function get_note_status( $note_name ) { - $data_store = self::load_data_store(); - $note_ids = $data_store->get_notes_with_name( $note_name ); + $note = self::get_note_by_name( $note_name ); - if ( empty( $note_ids ) ) { + if ( ! $note ) { return false; } - $note = self::get_note( $note_ids[0] ); - return $note->get_status(); }