From 4c177768159aa03d2560a7069a3dc7b07f7bfb8f Mon Sep 17 00:00:00 2001 From: Waclaw Jacek Date: Mon, 26 Apr 2021 23:57:39 +0200 Subject: [PATCH] Throw custom exception in NoteTraits if notes are disabled (https://github.com/woocommerce/woocommerce-admin/pull/6771) A custom exception `NotesUnavailableException` will be thrown on attempts to load the "admin-note" data store using the `Notes::load_data_store()` method introduced in this PR. All calls to `\WC_Data_Store::load( 'admin-note' )` were replaced with calls to the above method. --- plugins/woocommerce-admin/readme.txt | 1 + .../src/Composer/Package.php | 6 +- .../src/Notes/InstallJPAndWCSPlugins.php | 2 +- .../MerchantEmailNotifications.php | 2 +- .../src/Notes/NavigationFeedbackFollowUp.php | 2 +- .../src/Notes/NavigationNudge.php | 2 +- plugins/woocommerce-admin/src/Notes/Note.php | 2 +- .../src/Notes/NoteTraits.php | 16 ++- plugins/woocommerce-admin/src/Notes/Notes.php | 37 +++-- .../src/Notes/NotesUnavailableException.php | 15 +++ .../src/Notes/WooCommercePayments.php | 2 +- .../src/Notes/WooSubscriptionsNotes.php | 6 +- .../RemoteInboxNotifications/SpecRunner.php | 2 +- ...ests-learn-more-about-variable-product.php | 4 +- .../notes/class-wc-tests-marketing-notes.php | 4 +- .../notes/class-wc-tests-note-traits.php | 127 ++++++++++++++++++ .../tests/notes/class-wc-tests-notes.php | 35 +++++ 17 files changed, 238 insertions(+), 27 deletions(-) create mode 100644 plugins/woocommerce-admin/src/Notes/NotesUnavailableException.php create mode 100644 plugins/woocommerce-admin/tests/notes/class-wc-tests-note-traits.php create mode 100644 plugins/woocommerce-admin/tests/notes/class-wc-tests-notes.php diff --git a/plugins/woocommerce-admin/readme.txt b/plugins/woocommerce-admin/readme.txt index 64329be6853..ff25145a137 100644 --- a/plugins/woocommerce-admin/readme.txt +++ b/plugins/woocommerce-admin/readme.txt @@ -106,6 +106,7 @@ Release and roadmap notes are available on the [WooCommerce Developers Blog](htt - Fix: Parsing bad JSON string data from user WooCommerce meta. #6819 - Fix: Remove PayPal for India #6828 - Fix: Report filters expecting specific ordering. #6847 +- Fix: Throw exception if the data store cannot be loaded when trying to use notes. #6771 - Performance: Avoid updating customer info synchronously from the front end. #6765 - Tweak: Add settings_section event prop for CES #6762 - Tweak: Refactor payments to allow management of methods #6786 diff --git a/plugins/woocommerce-admin/src/Composer/Package.php b/plugins/woocommerce-admin/src/Composer/Package.php index 3a0f5b73571..843eebe358f 100644 --- a/plugins/woocommerce-admin/src/Composer/Package.php +++ b/plugins/woocommerce-admin/src/Composer/Package.php @@ -12,6 +12,8 @@ namespace Automattic\WooCommerce\Admin\Composer; defined( 'ABSPATH' ) || exit; use Automattic\WooCommerce\Admin\Notes\DeactivatePlugin; +use Automattic\WooCommerce\Admin\Notes\Notes; +use Automattic\WooCommerce\Admin\Notes\NotesUnavailableException; use Automattic\WooCommerce\Admin\FeaturePlugin; /** @@ -152,8 +154,8 @@ class Package { */ private static function is_notes_initialized() { try { - \WC_Data_Store::load( 'admin-note' ); - } catch ( \Exception $e ) { + Notes::load_data_store(); + } catch ( NotesUnavailableException $e ) { return false; } return true; diff --git a/plugins/woocommerce-admin/src/Notes/InstallJPAndWCSPlugins.php b/plugins/woocommerce-admin/src/Notes/InstallJPAndWCSPlugins.php index 2fb8f1fc21b..ed25910b215 100644 --- a/plugins/woocommerce-admin/src/Notes/InstallJPAndWCSPlugins.php +++ b/plugins/woocommerce-admin/src/Notes/InstallJPAndWCSPlugins.php @@ -76,7 +76,7 @@ class InstallJPAndWCSPlugins { } // Action any notes with a matching name. - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::NOTE_NAME ); foreach ( $note_ids as $note_id ) { diff --git a/plugins/woocommerce-admin/src/Notes/MerchantEmailNotifications/MerchantEmailNotifications.php b/plugins/woocommerce-admin/src/Notes/MerchantEmailNotifications/MerchantEmailNotifications.php index 2d75e353d46..e7c61207a43 100644 --- a/plugins/woocommerce-admin/src/Notes/MerchantEmailNotifications/MerchantEmailNotifications.php +++ b/plugins/woocommerce-admin/src/Notes/MerchantEmailNotifications/MerchantEmailNotifications.php @@ -69,7 +69,7 @@ class MerchantEmailNotifications { * Send all the notifications type `email`. */ public static function run() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_EMAIL ), diff --git a/plugins/woocommerce-admin/src/Notes/NavigationFeedbackFollowUp.php b/plugins/woocommerce-admin/src/Notes/NavigationFeedbackFollowUp.php index 976b29cbb3c..8d2cde05140 100644 --- a/plugins/woocommerce-admin/src/Notes/NavigationFeedbackFollowUp.php +++ b/plugins/woocommerce-admin/src/Notes/NavigationFeedbackFollowUp.php @@ -35,7 +35,7 @@ class NavigationFeedbackFollowUp { } // Check that the first note was created. - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( 'wc-admin-navigation-feedback' ); if ( empty( $note_ids ) ) { return; diff --git a/plugins/woocommerce-admin/src/Notes/NavigationNudge.php b/plugins/woocommerce-admin/src/Notes/NavigationNudge.php index 8ddb008e058..d1691cebb71 100644 --- a/plugins/woocommerce-admin/src/Notes/NavigationNudge.php +++ b/plugins/woocommerce-admin/src/Notes/NavigationNudge.php @@ -68,7 +68,7 @@ class NavigationNudge { return; } - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::NOTE_NAME ); if ( ! empty( $note_ids ) ) { diff --git a/plugins/woocommerce-admin/src/Notes/Note.php b/plugins/woocommerce-admin/src/Notes/Note.php index 689bf4df3e4..3ef0ead1c07 100644 --- a/plugins/woocommerce-admin/src/Notes/Note.php +++ b/plugins/woocommerce-admin/src/Notes/Note.php @@ -84,7 +84,7 @@ class Note extends \WC_Data { $this->set_object_read( true ); } - $this->data_store = \WC_Data_Store::load( 'admin-note' ); + $this->data_store = Notes::load_data_store(); if ( $this->get_id() > 0 ) { $this->data_store->read( $this ); } diff --git a/plugins/woocommerce-admin/src/Notes/NoteTraits.php b/plugins/woocommerce-admin/src/Notes/NoteTraits.php index 2c090e5426a..4bf7f6f16c3 100644 --- a/plugins/woocommerce-admin/src/Notes/NoteTraits.php +++ b/plugins/woocommerce-admin/src/Notes/NoteTraits.php @@ -27,9 +27,11 @@ trait NoteTraits { /** * Check if the note has been previously added. + * + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function note_exists() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::NOTE_NAME ); return ! empty( $note_ids ); } @@ -38,6 +40,7 @@ trait NoteTraits { * Checks if a note can and should be added. * * @return bool + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function can_be_added() { $note = self::get_note(); @@ -62,6 +65,8 @@ trait NoteTraits { /** * Add the note if it passes predefined conditions. + * + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function possibly_add_note() { $note = self::get_note(); @@ -75,6 +80,8 @@ trait NoteTraits { /** * Alias this method for backwards compatibility. + * + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function add_note() { self::possibly_add_note(); @@ -84,9 +91,11 @@ trait NoteTraits { * Possibly delete the note, if it exists in the database. Note that this * is a hard delete, for where it doesn't make sense to soft delete or * action the note. + * + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function possibly_delete_note() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::NOTE_NAME ); foreach ( $note_ids as $note_id ) { @@ -102,9 +111,10 @@ trait NoteTraits { * Get if the note has been actioned. * * @return bool + * @throws NotesUnavailableException Throws exception when notes are unavailable. */ public static function has_note_been_actioned() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::NOTE_NAME ); if ( ! empty( $note_ids ) ) { diff --git a/plugins/woocommerce-admin/src/Notes/Notes.php b/plugins/woocommerce-admin/src/Notes/Notes.php index 20324a4a4a9..aa3c84451b7 100644 --- a/plugins/woocommerce-admin/src/Notes/Notes.php +++ b/plugins/woocommerce-admin/src/Notes/Notes.php @@ -34,7 +34,7 @@ class Notes { * @return array Array of arrays. */ public static function get_notes( $context = 'edit', $args = array() ) { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); $raw_notes = $data_store->get_notes( $args ); $notes = array(); foreach ( (array) $raw_notes as $raw_note ) { @@ -90,7 +90,7 @@ class Notes { * @return int */ public static function get_notes_count( $type = array(), $status = array() ) { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); return $data_store->get_notes_count( $type, $status ); } @@ -106,7 +106,7 @@ class Notes { return; } - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); foreach ( $names as $name ) { $note_ids = $data_store->get_notes_with_name( $name ); @@ -163,7 +163,7 @@ class Notes { * @return array Array of notes. */ public static function delete_all_notes() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); // Here we filter for the same params we are using to show the note list in client side. $raw_notes = $data_store->get_notes( array( @@ -196,7 +196,7 @@ class Notes { * Clear note snooze status if the reminder date has been reached. */ public static function unsnooze_notes() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); $raw_notes = $data_store->get_notes( array( 'status' => array( Note::E_WC_ADMIN_NOTE_SNOOZED ), @@ -247,7 +247,7 @@ class Notes { return; } - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); $notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_MARKETING ), @@ -266,7 +266,7 @@ class Notes { * Delete actioned survey notes. */ public static function possibly_delete_survey_notes() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); $notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_SURVEY ), @@ -290,7 +290,7 @@ class Notes { * @return string|bool The note status. */ public static function get_note_status( $note_name ) { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = self::load_data_store(); $note_ids = $data_store->get_notes_with_name( $note_name ); if ( empty( $note_ids ) ) { @@ -434,4 +434,25 @@ class Notes { } return $screen_name; } + + /** + * Loads the data store. + * + * If the "admin-note" data store is unavailable, attempts to load it + * will result in an exception. + * This method catches that exception and throws a custom one instead. + * + * @return \WC_Data_Store The "admin-note" data store. + * @throws NotesUnavailableException Throws exception if data store loading fails. + */ + public static function load_data_store() { + try { + return \WC_Data_Store::load( 'admin-note' ); + } catch ( \Exception $e ) { + throw new NotesUnavailableException( + 'woocommerce_admin_notes_unavailable', + __( 'Notes are unavailable because the "admin-note" data store cannot be loaded.', 'woocommerce-admin' ) + ); + } + } } diff --git a/plugins/woocommerce-admin/src/Notes/NotesUnavailableException.php b/plugins/woocommerce-admin/src/Notes/NotesUnavailableException.php new file mode 100644 index 00000000000..01ee2240737 --- /dev/null +++ b/plugins/woocommerce-admin/src/Notes/NotesUnavailableException.php @@ -0,0 +1,15 @@ +get_notes_with_name( self::NOTE_NAME ); diff --git a/plugins/woocommerce-admin/src/Notes/WooSubscriptionsNotes.php b/plugins/woocommerce-admin/src/Notes/WooSubscriptionsNotes.php index 1e483b829b1..2e8749c7b5f 100644 --- a/plugins/woocommerce-admin/src/Notes/WooSubscriptionsNotes.php +++ b/plugins/woocommerce-admin/src/Notes/WooSubscriptionsNotes.php @@ -104,7 +104,7 @@ class WooSubscriptionsNotes { */ public function check_connection() { if ( ! $this->is_connected() ) { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::CONNECTION_NOTE_NAME ); if ( ! empty( $note_ids ) ) { // We already have a connection note. Exit early. @@ -216,7 +216,7 @@ class WooSubscriptionsNotes { public function prune_inactive_subscription_notes() { $active_product_ids = $this->get_subscription_active_product_ids(); - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::SUBSCRIPTION_NOTE_NAME ); foreach ( (array) $note_ids as $note_id ) { @@ -239,7 +239,7 @@ class WooSubscriptionsNotes { public function find_note_for_product_id( $product_id ) { $product_id = intval( $product_id ); - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( self::SUBSCRIPTION_NOTE_NAME ); foreach ( (array) $note_ids as $note_id ) { $note = Notes::get_note( $note_id ); diff --git a/plugins/woocommerce-admin/src/RemoteInboxNotifications/SpecRunner.php b/plugins/woocommerce-admin/src/RemoteInboxNotifications/SpecRunner.php index 14a40f1f61d..898feaff7ba 100644 --- a/plugins/woocommerce-admin/src/RemoteInboxNotifications/SpecRunner.php +++ b/plugins/woocommerce-admin/src/RemoteInboxNotifications/SpecRunner.php @@ -21,7 +21,7 @@ class SpecRunner { * @param object $stored_state Stored state. */ public static function run_spec( $spec, $stored_state ) { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); // Create or update the note. $existing_note_ids = $data_store->get_notes_with_name( $spec->slug ); diff --git a/plugins/woocommerce-admin/tests/notes/class-wc-tests-learn-more-about-variable-product.php b/plugins/woocommerce-admin/tests/notes/class-wc-tests-learn-more-about-variable-product.php index 3b0e9d4c65b..9f31d3b4633 100644 --- a/plugins/woocommerce-admin/tests/notes/class-wc-tests-learn-more-about-variable-product.php +++ b/plugins/woocommerce-admin/tests/notes/class-wc-tests-learn-more-about-variable-product.php @@ -39,7 +39,7 @@ class WC_Tests_Learn_More_About_Variable_Product extends WC_Unit_Test_Case { wp_insert_post( $product ); // Then we should have LearnMoreAboutVariableProducts note. - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note_ids = $data_store->get_notes_with_name( LearnMoreAboutVariableProducts::NOTE_NAME ); $this->assertNotEmpty( $note_ids ); $this->assertCount( 1, $note_ids ); @@ -111,7 +111,7 @@ class WC_Tests_Learn_More_About_Variable_Product extends WC_Unit_Test_Case { * @return array */ protected function get_note_ids() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); return $data_store->get_notes_with_name( LearnMoreAboutVariableProducts::NOTE_NAME ); } diff --git a/plugins/woocommerce-admin/tests/notes/class-wc-tests-marketing-notes.php b/plugins/woocommerce-admin/tests/notes/class-wc-tests-marketing-notes.php index f8cb1be0c8c..afe892c4066 100644 --- a/plugins/woocommerce-admin/tests/notes/class-wc-tests-marketing-notes.php +++ b/plugins/woocommerce-admin/tests/notes/class-wc-tests-marketing-notes.php @@ -18,7 +18,7 @@ class WC_Tests_Marketing_Notes extends WC_Unit_Test_Case { * Tests that a marketing note can be added. */ public function test_add_remove_marketing_note() { - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $note = new Note(); $note->set_title( 'PHPUNIT_TEST_MARKETING_NOTE' ); @@ -64,7 +64,7 @@ class WC_Tests_Marketing_Notes extends WC_Unit_Test_Case { WooCommercePayments::possibly_add_note(); // Load all marketing notes and check that the note was not added. - $data_store = \WC_Data_Store::load( 'admin-note' ); + $data_store = Notes::load_data_store(); $notes = $data_store->get_notes( array( 'type' => array( Note::E_WC_ADMIN_NOTE_MARKETING ), diff --git a/plugins/woocommerce-admin/tests/notes/class-wc-tests-note-traits.php b/plugins/woocommerce-admin/tests/notes/class-wc-tests-note-traits.php new file mode 100644 index 00000000000..9670835d93e --- /dev/null +++ b/plugins/woocommerce-admin/tests/notes/class-wc-tests-note-traits.php @@ -0,0 +1,127 @@ +expectException( NotesUnavailableException::class ); + $callback(); + remove_filter( 'woocommerce_data_stores', '__return_empty_array' ); + } + + /** + * @doesNotPerformAssertions + * @dataProvider methods_never_causing_exception_provider + * + * @param callable $callback Tested NoteTraits method. + */ + public function test_no_exception_is_thrown_even_if_data_store_cannot_be_loaded( $callback ) { + add_filter( 'woocommerce_data_stores', '__return_empty_array' ); + $callback(); + remove_filter( 'woocommerce_data_stores', '__return_empty_array' ); + } + + /** + * Method required to use NoteTraits. + * + * @return Note + */ + public static function get_note() { + return new Note(); + } + + /** + * Data provider providing methods that should throw an exception + * only if the "admin-note" data store cannot be loaded. + * + * @return array[] + */ + public function methods_causing_exception_if_data_store_cannot_be_loaded_provider() { + return array( + array( + function () { + self::note_exists(); + }, + ), + array( + function () { + self::can_be_added(); + }, + ), + array( + function () { + self::possibly_add_note(); + }, + ), + array( + function () { + self::add_note(); + }, + ), + array( + function () { + self::possibly_delete_note(); + }, + ), + array( + function () { + self::has_note_been_actioned(); + }, + ), + ); + } + + /** + * Data provider providing methods that should not throw + * an exception regardless of the data store being available. + * + * @return array[] + */ + public function methods_never_causing_exception_provider() { + return array( + array( + function () { + self::wc_admin_active_for( 123 ); + }, + ), + ); + } + +} diff --git a/plugins/woocommerce-admin/tests/notes/class-wc-tests-notes.php b/plugins/woocommerce-admin/tests/notes/class-wc-tests-notes.php new file mode 100644 index 00000000000..59b45edb68e --- /dev/null +++ b/plugins/woocommerce-admin/tests/notes/class-wc-tests-notes.php @@ -0,0 +1,35 @@ +assertInstanceOf( \WC_Data_Store::class, Notes::load_data_store() ); + } + + /** + * If the "admin-note" data store does not exist, a custom + * exception should be thrown. + */ + public function test_exception_is_thrown_if_data_store_does_not_exist() { + add_filter( 'woocommerce_data_stores', '__return_empty_array' ); + $this->expectException( NotesUnavailableException::class ); + Notes::load_data_store(); + remove_filter( 'woocommerce_data_stores', '__return_empty_array' ); + } + +}