From cf91a772f5fc7974e6fb24cef775e08f336a90d3 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 12:01:41 +0100 Subject: [PATCH 01/20] fix(integrity-check): show hub status for in-sync nodes instead of empty cell --- includes/cli/class-integrity-check.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index a9dbab4..b5aadbb 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -145,10 +145,10 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G // Prepare table data with node columns. $table_data = []; foreach ( $all_discrepancies as $discrepancy ) { - // Fill in missing node statuses with empty string. + // Fill in missing node statuses with the hub status (node is in sync with hub). foreach ( $node_columns as $column ) { if ( ! isset( $discrepancy[ $column ] ) && ! in_array( $column, [ 'email', 'network_id', 'hub_status' ] ) ) { - $discrepancy[ $column ] = ''; + $discrepancy[ $column ] = $discrepancy['hub_status']; } } $table_data[] = $discrepancy; From f85d2854755fc54a3039736d2dd4ed30d9cdc1aa Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 12:45:04 +0100 Subject: [PATCH 02/20] feat(memberships): propagate ownership transfers across the network Listen to wc_memberships_user_membership_transferred and dispatch a membership update event with previous_email. On nodes, find the existing managed membership by remote_id and reassign it. --- .../class-woocommerce-membership-updated.php | 126 +++++++++++ .../woocommerce-memberships/class-events.php | 32 +++ tests/unit-tests/mock-wc-memberships.php | 30 +++ tests/unit-tests/test-membership-transfer.php | 210 ++++++++++++++++++ 4 files changed, 398 insertions(+) create mode 100644 tests/unit-tests/mock-wc-memberships.php create mode 100644 tests/unit-tests/test-membership-transfer.php diff --git a/includes/incoming-events/class-woocommerce-membership-updated.php b/includes/incoming-events/class-woocommerce-membership-updated.php index 787dbf5..b4fd939 100644 --- a/includes/incoming-events/class-woocommerce-membership-updated.php +++ b/includes/incoming-events/class-woocommerce-membership-updated.php @@ -74,6 +74,13 @@ public function update_membership() { $user = User_Utils::get_or_create_user_by_email( $email, $this->get_site(), $this->data->user_id ?? '' ); + // Handle membership ownership transfer. + $previous_email = $this->get_previous_email(); + if ( $previous_email ) { + $this->transfer_membership( $user, $local_plan_id, $previous_email ); + return; + } + $user_membership = wc_memberships_get_user_membership( $user->ID, $local_plan_id ); if ( null === $user_membership ) { @@ -96,6 +103,116 @@ public function update_membership() { return; } + $this->apply_membership_update( $user_membership ); + } + + /** + * Transfer a managed membership from the previous owner to the new owner. + * + * Finds the existing membership by remote_id and reassigns it. + * Falls back to creating a new membership if the existing one can't be found. + * + * @param \WP_User $new_user The new owner. + * @param int $local_plan_id The local plan ID. + * @param string $previous_email The previous owner's email. + * @return void + */ + private function transfer_membership( $new_user, $local_plan_id, $previous_email ) { + global $wpdb; + + Debugger::log( 'Processing membership transfer from ' . $previous_email . ' to ' . $new_user->user_email ); + + $remote_membership_id = $this->get_membership_id(); + + // Find the existing managed membership by remote_id. + $existing_membership_id = $wpdb->get_var( // phpcs:ignore + $wpdb->prepare( + "SELECT post_id FROM $wpdb->postmeta + WHERE meta_key = %s AND meta_value = %s + AND post_id IN ( + SELECT post_id FROM $wpdb->postmeta + WHERE meta_key = %s AND meta_value = %s + ) + AND post_id IN ( + SELECT ID FROM $wpdb->posts WHERE post_type = 'wc_user_membership' AND post_parent = %d + )", + Memberships_Admin::REMOTE_ID_META_KEY, + $remote_membership_id, + Memberships_Admin::SITE_URL_META_KEY, + $this->get_site(), + $local_plan_id + ) + ); + + if ( ! $existing_membership_id ) { + Debugger::log( 'Managed membership not found by remote_id, falling back to previous owner lookup.' ); + + // Try finding by previous owner + plan. + $previous_user = get_user_by( 'email', $previous_email ); + if ( $previous_user ) { + $previous_membership = wc_memberships_get_user_membership( $previous_user->ID, $local_plan_id ); + if ( $previous_membership ) { + $existing_membership_id = $previous_membership->get_id(); + } + } + } + + if ( ! $existing_membership_id ) { + Debugger::log( 'No existing membership found to transfer, creating new one.' ); + + $user_membership = wc_memberships_create_user_membership( + [ + 'plan_id' => $local_plan_id, + 'user_id' => $new_user->ID, + ] + ); + + if ( is_wp_error( $user_membership ) || ! $user_membership instanceof WC_Memberships_User_Membership ) { + Debugger::log( 'Error creating membership for transfer.' ); + return; + } + + $this->apply_membership_update( $user_membership ); + return; + } + + // Reassign the membership to the new owner. + wp_update_post( + [ + 'ID' => $existing_membership_id, + 'post_author' => $new_user->ID, + ] + ); + + $user_membership = wc_memberships_get_user_membership( $existing_membership_id ); + + if ( ! $user_membership instanceof WC_Memberships_User_Membership ) { + Debugger::log( 'Error retrieving membership after transfer.' ); + return; + } + + $user_membership->add_note( + sprintf( + // translators: 1: previous owner email, 2: new owner email, 3: site URL. + __( 'Membership transferred from %1$s to %2$s via Newspack Network. Propagated from %3$s.', 'newspack-network' ), + $previous_email, + $new_user->user_email, + $this->get_site() + ) + ); + + $this->apply_membership_update( $user_membership ); + + Debugger::log( 'Membership transferred successfully.' ); + } + + /** + * Apply status, end date, and managed meta to a membership. + * + * @param WC_Memberships_User_Membership $user_membership The membership to update. + * @return void + */ + private function apply_membership_update( $user_membership ) { $status = $this->get_new_status(); $is_managed = get_post_meta( $user_membership->get_id(), Memberships_Admin::NETWORK_MANAGED_META_KEY, true ); @@ -172,4 +289,13 @@ public function get_membership_id() { public function get_end_date() { return $this->data->end_date ?? null; } + + /** + * Get the previous owner's email (set during ownership transfers). + * + * @return ?string + */ + public function get_previous_email() { + return $this->data->previous_email ?? null; + } } diff --git a/includes/woocommerce-memberships/class-events.php b/includes/woocommerce-memberships/class-events.php index 89fca61..d4d987e 100644 --- a/includes/woocommerce-memberships/class-events.php +++ b/includes/woocommerce-memberships/class-events.php @@ -48,6 +48,7 @@ public static function register_listeners() { Data_Events::register_listener( 'wc_memberships_user_membership_status_changed', 'newspack_network_woo_membership_updated', [ __CLASS__, 'membership_status_changed' ] ); Data_Events::register_listener( 'wc_memberships_user_membership_saved', 'newspack_network_woo_membership_updated', [ __CLASS__, 'membership_saved' ] ); Data_Events::register_listener( 'wc_memberships_user_membership_deleted', 'newspack_network_woo_membership_updated', [ __CLASS__, 'membership_deleted' ] ); + Data_Events::register_listener( 'wc_memberships_user_membership_transferred', 'newspack_network_woo_membership_updated', [ __CLASS__, 'membership_transferred' ] ); Data_Events::register_listener( 'newspack_network_save_membership_plan', 'newspack_network_membership_plan_updated', [ __CLASS__, 'membership_plan_updated' ] ); } @@ -187,6 +188,37 @@ public static function membership_saved( $plan, $args ) { ]; } + /** + * Transforms the data of the wc_memberships_user_membership_transferred hook to trigger the newspack_network_woo_membership_updated data event + * + * @param \WC_Memberships_User_Membership $user_membership The User Membership object that was transferred. + * @param \WP_User $new_owner The new owner of the membership. + * @param \WP_User $previous_owner The previous owner of the membership. + * @return array|void + */ + public static function membership_transferred( $user_membership, $new_owner, $previous_owner ) { + if ( self::$pause_events ) { + return; + } + + $plan_id = $user_membership->get_plan()->get_id(); + + $plan_network_id = get_post_meta( $plan_id, Admin::NETWORK_ID_META_KEY, true ); + if ( ! $plan_network_id ) { + return; + } + + return [ + 'email' => $new_owner->user_email, + 'user_id' => $new_owner->ID, + 'plan_network_id' => $plan_network_id, + 'membership_id' => $user_membership->get_id(), + 'new_status' => $user_membership->get_status(), + 'end_date' => $user_membership->get_end_date(), + 'previous_email' => $previous_owner->user_email, + ]; + } + /** * Triggers a data event when the membership plan is updated * diff --git a/tests/unit-tests/mock-wc-memberships.php b/tests/unit-tests/mock-wc-memberships.php new file mode 100644 index 0000000..59b27f1 --- /dev/null +++ b/tests/unit-tests/mock-wc-memberships.php @@ -0,0 +1,30 @@ +factory->user->create( [ 'user_email' => 'oldowner@example.com' ] ); + $new_user_id = $this->factory->user->create( [ 'user_email' => 'newowner@example.com' ] ); + + $plan_id = $this->factory->post->create( + [ + 'post_type' => Memberships_Admin::MEMBERSHIP_PLANS_CPT, + 'post_status' => 'publish', + 'post_title' => 'Test Plan', + ] + ); + update_post_meta( $plan_id, Memberships_Admin::NETWORK_ID_META_KEY, 'test-plan' ); + + $membership_id = $this->factory->post->create( + [ + 'post_type' => 'wc_user_membership', + 'post_status' => 'wcm-active', + 'post_author' => $old_user_id, + 'post_parent' => $plan_id, + ] + ); + update_post_meta( $membership_id, Memberships_Admin::NETWORK_MANAGED_META_KEY, true ); + update_post_meta( $membership_id, Memberships_Admin::REMOTE_ID_META_KEY, 999 ); + update_post_meta( $membership_id, Memberships_Admin::SITE_URL_META_KEY, 'https://hub.example.com' ); + + // Verify initial ownership. + $this->assertEquals( $old_user_id, (int) get_post( $membership_id )->post_author ); + + $transfer_event = new Woocommerce_Membership_Updated( + 'https://hub.example.com', + [ + 'email' => 'newowner@example.com', + 'user_id' => $new_user_id, + 'plan_network_id' => 'test-plan', + 'membership_id' => 999, + 'new_status' => 'active', + 'end_date' => null, + 'previous_email' => 'oldowner@example.com', + ], + time() + ); + + $transfer_method = new ReflectionMethod( Woocommerce_Membership_Updated::class, 'transfer_membership' ); + $transfer_method->setAccessible( true ); + + $new_user = get_user_by( 'id', $new_user_id ); + $transfer_method->invoke( $transfer_event, $new_user, $plan_id, 'oldowner@example.com' ); + + // Verify ownership was transferred via wp_update_post. + $this->assertEquals( $new_user_id, (int) get_post( $membership_id )->post_author ); + } + + /** + * Test that transfer does not affect memberships with a different remote_id. + */ + public function test_transfer_does_not_touch_unrelated_membership() { + $old_user_id = $this->factory->user->create( [ 'user_email' => 'oldowner2@example.com' ] ); + $new_user_id = $this->factory->user->create( [ 'user_email' => 'newowner2@example.com' ] ); + $other_user_id = $this->factory->user->create( [ 'user_email' => 'other@example.com' ] ); + + $plan_id = $this->factory->post->create( + [ + 'post_type' => Memberships_Admin::MEMBERSHIP_PLANS_CPT, + 'post_status' => 'publish', + ] + ); + update_post_meta( $plan_id, Memberships_Admin::NETWORK_ID_META_KEY, 'test-plan' ); + + // Target membership (remote_id = 999). + $target_membership_id = $this->factory->post->create( + [ + 'post_type' => 'wc_user_membership', + 'post_status' => 'wcm-active', + 'post_author' => $old_user_id, + 'post_parent' => $plan_id, + ] + ); + update_post_meta( $target_membership_id, Memberships_Admin::NETWORK_MANAGED_META_KEY, true ); + update_post_meta( $target_membership_id, Memberships_Admin::REMOTE_ID_META_KEY, 999 ); + update_post_meta( $target_membership_id, Memberships_Admin::SITE_URL_META_KEY, 'https://hub.example.com' ); + + // Unrelated membership (remote_id = 888). + $unrelated_membership_id = $this->factory->post->create( + [ + 'post_type' => 'wc_user_membership', + 'post_status' => 'wcm-active', + 'post_author' => $other_user_id, + 'post_parent' => $plan_id, + ] + ); + update_post_meta( $unrelated_membership_id, Memberships_Admin::NETWORK_MANAGED_META_KEY, true ); + update_post_meta( $unrelated_membership_id, Memberships_Admin::REMOTE_ID_META_KEY, 888 ); + update_post_meta( $unrelated_membership_id, Memberships_Admin::SITE_URL_META_KEY, 'https://hub.example.com' ); + + $transfer_event = new Woocommerce_Membership_Updated( + 'https://hub.example.com', + [ + 'email' => 'newowner2@example.com', + 'user_id' => $new_user_id, + 'plan_network_id' => 'test-plan', + 'membership_id' => 999, + 'new_status' => 'active', + 'end_date' => null, + 'previous_email' => 'oldowner2@example.com', + ], + time() + ); + + $transfer_method = new ReflectionMethod( Woocommerce_Membership_Updated::class, 'transfer_membership' ); + $transfer_method->setAccessible( true ); + + $new_user = get_user_by( 'id', $new_user_id ); + $transfer_method->invoke( $transfer_event, $new_user, $plan_id, 'oldowner2@example.com' ); + + // Target should be transferred. + $this->assertEquals( $new_user_id, (int) get_post( $target_membership_id )->post_author ); + + // Unrelated should not be touched. + $this->assertEquals( $other_user_id, (int) get_post( $unrelated_membership_id )->post_author ); + } + + /** + * Test that Events::membership_transferred returns null when events are paused. + */ + public function test_events_listener_paused() { + Memberships_Events::$pause_events = true; + $result = Memberships_Events::membership_transferred( null, null, null ); + $this->assertNull( $result ); + Memberships_Events::$pause_events = false; + } + + /** + * Test the full update_membership flow routes to transfer when previous_email is set. + * + * With stubbed WC functions (returning null), the method will find the plan via DB, + * enter the transfer path, do the wp_update_post, then gracefully exit when + * wc_memberships_get_user_membership returns null. + */ + public function test_update_membership_routes_to_transfer() { + $old_user_id = $this->factory->user->create( [ 'user_email' => 'oldowner3@example.com' ] ); + $new_user_id = $this->factory->user->create( [ 'user_email' => 'newowner3@example.com' ] ); + + $plan_id = $this->factory->post->create( + [ + 'post_type' => Memberships_Admin::MEMBERSHIP_PLANS_CPT, + 'post_status' => 'publish', + ] + ); + update_post_meta( $plan_id, Memberships_Admin::NETWORK_ID_META_KEY, 'test-plan-full' ); + + $membership_id = $this->factory->post->create( + [ + 'post_type' => 'wc_user_membership', + 'post_status' => 'wcm-active', + 'post_author' => $old_user_id, + 'post_parent' => $plan_id, + ] + ); + update_post_meta( $membership_id, Memberships_Admin::NETWORK_MANAGED_META_KEY, true ); + update_post_meta( $membership_id, Memberships_Admin::REMOTE_ID_META_KEY, 777 ); + update_post_meta( $membership_id, Memberships_Admin::SITE_URL_META_KEY, 'https://hub.example.com' ); + + $transfer_event = new Woocommerce_Membership_Updated( + 'https://hub.example.com', + [ + 'email' => 'newowner3@example.com', + 'user_id' => $new_user_id, + 'plan_network_id' => 'test-plan-full', + 'membership_id' => 777, + 'new_status' => 'active', + 'end_date' => null, + 'previous_email' => 'oldowner3@example.com', + ], + time() + ); + + // Call the full update_membership flow (which now has the WC function stubs). + $transfer_event->update_membership(); + + // Verify the membership was transferred. + $this->assertEquals( $new_user_id, (int) get_post( $membership_id )->post_author ); + } +} From 765d84bcb40baa70c7c6045519c0b395e6063622 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 15:20:36 +0100 Subject: [PATCH 03/20] fix(memberships): address review feedback on transfer logic --- .../class-woocommerce-membership-updated.php | 18 +++++++++++++++--- tests/unit-tests/test-membership-transfer.php | 13 +++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/includes/incoming-events/class-woocommerce-membership-updated.php b/includes/incoming-events/class-woocommerce-membership-updated.php index b4fd939..c19d3f5 100644 --- a/includes/incoming-events/class-woocommerce-membership-updated.php +++ b/includes/incoming-events/class-woocommerce-membership-updated.php @@ -135,12 +135,17 @@ private function transfer_membership( $new_user, $local_plan_id, $previous_email ) AND post_id IN ( SELECT ID FROM $wpdb->posts WHERE post_type = 'wc_user_membership' AND post_parent = %d + ) + AND post_id IN ( + SELECT post_id FROM $wpdb->postmeta + WHERE meta_key = %s )", Memberships_Admin::REMOTE_ID_META_KEY, $remote_membership_id, Memberships_Admin::SITE_URL_META_KEY, $this->get_site(), - $local_plan_id + $local_plan_id, + Memberships_Admin::NETWORK_MANAGED_META_KEY ) ); @@ -177,13 +182,20 @@ private function transfer_membership( $new_user, $local_plan_id, $previous_email } // Reassign the membership to the new owner. - wp_update_post( + $updated_post_id = wp_update_post( [ 'ID' => $existing_membership_id, 'post_author' => $new_user->ID, - ] + ], + true ); + if ( is_wp_error( $updated_post_id ) || ! $updated_post_id ) { + $error_message = is_wp_error( $updated_post_id ) ? $updated_post_id->get_error_message() : 'Unknown error'; + Debugger::log( 'Error transferring membership: failed to update post author. ' . $error_message ); + return; + } + $user_membership = wc_memberships_get_user_membership( $existing_membership_id ); if ( ! $user_membership instanceof WC_Memberships_User_Membership ) { diff --git a/tests/unit-tests/test-membership-transfer.php b/tests/unit-tests/test-membership-transfer.php index 7b7d87b..17621d6 100644 --- a/tests/unit-tests/test-membership-transfer.php +++ b/tests/unit-tests/test-membership-transfer.php @@ -150,10 +150,15 @@ public function test_transfer_does_not_touch_unrelated_membership() { * Test that Events::membership_transferred returns null when events are paused. */ public function test_events_listener_paused() { - Memberships_Events::$pause_events = true; - $result = Memberships_Events::membership_transferred( null, null, null ); - $this->assertNull( $result ); - Memberships_Events::$pause_events = false; + $previous_pause_events = Memberships_Events::$pause_events; + + try { + Memberships_Events::$pause_events = true; + $result = Memberships_Events::membership_transferred( null, null, null ); + $this->assertNull( $result ); + } finally { + Memberships_Events::$pause_events = $previous_pause_events; + } } /** From 6d2e7fec3df8cc33633a2eebfc5883551b336a1c Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:12:47 +0100 Subject: [PATCH 04/20] refactor(integrity-check): add post_modified and membership_id to membership data --- includes/class-integrity-check-utils.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/includes/class-integrity-check-utils.php b/includes/class-integrity-check-utils.php index c252f6f..40f912f 100644 --- a/includes/class-integrity-check-utils.php +++ b/includes/class-integrity-check-utils.php @@ -26,10 +26,12 @@ private static function build_membership_query( $start_email = null, $end_email // phpcs:disable WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users $query = " - SELECT + SELECT u.user_email, p.post_status as status, - pm_network.meta_value as network_id + pm_network.meta_value as network_id, + p.post_modified, + p.ID as membership_id FROM {$wpdb->posts} p INNER JOIN {$wpdb->users} u ON p.post_author = u.ID INNER JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s @@ -86,9 +88,11 @@ private static function execute_membership_query( $query, $prepare_args, $max_re $membership_data = []; foreach ( $results as $result ) { $membership_data[] = [ - 'email' => strtolower( $result->user_email ), - 'status' => $result->status, - 'network_id' => $result->network_id, + 'email' => strtolower( $result->user_email ), + 'status' => $result->status, + 'network_id' => $result->network_id, + 'post_modified' => $result->post_modified, + 'membership_id' => (int) $result->membership_id, ]; } From 7e7875353cfafbb805d2a785ec4d6c81ddc2a30d Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:14:27 +0100 Subject: [PATCH 05/20] feat(integrity-check): add managed memberships endpoint for reconciliation --- .../node/class-integrity-check-endpoints.php | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 9ea7582..71fa51b 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -54,6 +54,20 @@ public static function register_routes() { ] ); + register_rest_route( + 'newspack-network/v1', + '/integrity-check/managed-memberships', + [ + [ + 'methods' => \WP_REST_Server::READABLE, + 'callback' => [ __CLASS__, 'handle_managed_memberships_request' ], + 'permission_callback' => function( $request ) { + return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); + }, + ], + ] + ); + register_rest_route( 'newspack-network/v1', '/integrity-check/range-hash', @@ -148,6 +162,55 @@ public static function handle_memberships_request( $request ) { ); } + /** + * Handles the managed memberships request. + * + * Returns all network-managed memberships with their remote_id and remote_site_url. + * + * @param \WP_REST_Request $request The REST request object. + */ + public static function handle_managed_memberships_request( $request ) { + global $wpdb; + + // phpcs:disable WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching + $results = $wpdb->get_results( + $wpdb->prepare( + "SELECT p.ID, p.post_status, p.post_modified, + u.user_email, + pm_remote.meta_value as remote_id, + pm_site.meta_value as remote_site_url, + pm_network.meta_value as network_id + FROM {$wpdb->posts} p + INNER JOIN {$wpdb->users} u ON p.post_author = u.ID + INNER JOIN {$wpdb->postmeta} pm_managed ON p.ID = pm_managed.post_id AND pm_managed.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_remote ON p.ID = pm_remote.post_id AND pm_remote.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_site ON p.ID = pm_site.post_id AND pm_site.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s + WHERE p.post_type = 'wc_user_membership'", + '_managed_by_newspack_network', + '_remote_id', + '_remote_site_url', + \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY + ) + ); + + $memberships = []; + foreach ( $results as $row ) { + $memberships[] = [ + 'email' => strtolower( $row->user_email ), + 'status' => $row->post_status, + 'network_id' => $row->network_id ?? '', + 'remote_id' => (int) $row->remote_id, + 'remote_site_url' => $row->remote_site_url ?? '', + 'post_modified' => $row->post_modified, + 'membership_id' => (int) $row->ID, + ]; + } + + return rest_ensure_response( [ 'memberships' => $memberships ] ); + } + /** * Handles the range hash request. * From c8121e9c30d5ace6c71bc3189f1c0c0860637a1a Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:16:59 +0100 Subject: [PATCH 06/20] feat(memberships): add reconcile-memberships CLI command --- includes/class-initializer.php | 1 + includes/cli/class-reconcile-memberships.php | 430 +++++++++++++++++++ 2 files changed, 431 insertions(+) create mode 100644 includes/cli/class-reconcile-memberships.php diff --git a/includes/class-initializer.php b/includes/class-initializer.php index 044b927..e1c1ada 100644 --- a/includes/class-initializer.php +++ b/includes/class-initializer.php @@ -58,6 +58,7 @@ public static function init() { Data_Backfill::init(); Membership_Dedupe::init(); CLI\Integrity_Check::init(); + CLI\Reconcile_Memberships::init(); Woocommerce\Events::init(); Woocommerce\Product_Admin::init(); diff --git a/includes/cli/class-reconcile-memberships.php b/includes/cli/class-reconcile-memberships.php new file mode 100644 index 0000000..9e5fae0 --- /dev/null +++ b/includes/cli/class-reconcile-memberships.php @@ -0,0 +1,430 @@ +] + * : Only reconcile with a specific node URL. + * + * ## EXAMPLES + * + * wp newspack-network reconcile-memberships + * wp newspack-network reconcile-memberships --live + * wp newspack-network reconcile-memberships --verbose + * wp newspack-network reconcile-memberships --node=https://example.com --live + * + * @param array $args The command arguments. + * @param array $assoc_args The command options. + * @return void + */ + public static function reconcile( $args, $assoc_args ) { + $is_live = isset( $assoc_args['live'] ); + $verbose = isset( $assoc_args['verbose'] ); + $node_filter = isset( $assoc_args['node'] ) ? $assoc_args['node'] : null; + + if ( $is_live ) { + WP_CLI::warning( 'Running in LIVE mode – events will be dispatched to fix discrepancies.' ); + } else { + WP_CLI::line( 'Running in DRY-RUN mode. Use --live to dispatch fix events.' ); + } + WP_CLI::line( '' ); + + // Step 1: Get hub membership data. + $hub_data = Integrity_Check_Utils::get_membership_data(); + + if ( $verbose ) { + WP_CLI::line( sprintf( '%d memberships found on the hub.', count( $hub_data ) ) ); + WP_CLI::line( '' ); + } + + // Build lookup keyed by email::network_id for fast access. + $hub_lookup = []; + foreach ( $hub_data as $item ) { + $key = $item['email'] . '::' . $item['network_id']; + $hub_lookup[ $key ] = $item; + } + + // Step 2: Get nodes to process. + $all_nodes = Nodes::get_all_nodes(); + + if ( $node_filter ) { + $all_nodes = array_filter( + $all_nodes, + function( $node ) use ( $node_filter ) { + return rtrim( $node->get_url(), '/' ) === rtrim( $node_filter, '/' ); + } + ); + + if ( empty( $all_nodes ) ) { + WP_CLI::error( sprintf( 'No node found with URL: %s', $node_filter ) ); + } + } + + if ( empty( $all_nodes ) ) { + WP_CLI::warning( 'No nodes found.' ); + return; + } + + // Step 3: Process each node. + $summary_counts = [ + 'missing_on_node' => 0, + 'missing_on_hub' => 0, + 'status_mismatch' => 0, + 'push_to_node' => 0, + 'skipped' => 0, + ]; + + foreach ( $all_nodes as $node ) { + WP_CLI::line( sprintf( 'Processing node: %s', $node->get_url() ) ); + + $node_memberships = self::get_node_membership_data( $node, $verbose ); + if ( null === $node_memberships ) { + WP_CLI::warning( sprintf( 'Skipping node %s due to fetch error.', $node->get_url() ) ); + WP_CLI::line( '' ); + continue; + } + + $node_managed_memberships = self::get_node_managed_memberships( $node, $verbose ); + if ( null === $node_managed_memberships ) { + WP_CLI::warning( sprintf( 'Could not fetch managed memberships from %s – timestamp comparison will use hub as authoritative.', $node->get_url() ) ); + $node_managed_memberships = []; + } + + if ( $verbose ) { + WP_CLI::line( sprintf( ' %d memberships found on node.', count( $node_memberships ) ) ); + WP_CLI::line( sprintf( ' %d managed memberships found on node.', count( $node_managed_memberships ) ) ); + } + + $discrepancies = self::classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_memberships ); + + if ( empty( $discrepancies ) ) { + WP_CLI::success( sprintf( 'Node %s is in sync with the hub.', $node->get_url() ) ); + WP_CLI::line( '' ); + continue; + } + + WP_CLI::line( sprintf( ' Found %d discrepancies:', count( $discrepancies ) ) ); + WP_CLI::line( '' ); + + // Display table of planned actions. + $table_columns = [ 'email', 'network_id', 'type', 'hub_status', 'node_status', 'action' ]; + WP_CLI\Utils\format_items( 'table', $discrepancies, $table_columns ); + WP_CLI::line( '' ); + + // Accumulate summary counts. + foreach ( $discrepancies as $discrepancy ) { + $summary_counts[ $discrepancy['type'] ]++; + if ( 'push_to_node' === $discrepancy['action'] ) { + $summary_counts['push_to_node']++; + } else { + $summary_counts['skipped']++; + } + } + + // In live mode, dispatch events for push_to_node actions. + if ( $is_live ) { + $dispatched = 0; + foreach ( $discrepancies as $discrepancy ) { + if ( 'push_to_node' !== $discrepancy['action'] ) { + continue; + } + $key = $discrepancy['email'] . '::' . $discrepancy['network_id']; + $hub_item = $hub_lookup[ $key ] ?? null; + if ( $hub_item ) { + self::dispatch_to_node( $hub_item ); + $dispatched++; + } + } + WP_CLI::success( sprintf( 'Dispatched %d event(s) for node %s.', $dispatched, $node->get_url() ) ); + } + + WP_CLI::line( '' ); + } + + // Print summary. + WP_CLI::line( '=== Summary ===' ); + WP_CLI::line( sprintf( 'Missing on node: %d', $summary_counts['missing_on_node'] ) ); + WP_CLI::line( sprintf( 'Missing on hub: %d', $summary_counts['missing_on_hub'] ) ); + WP_CLI::line( sprintf( 'Status mismatch: %d', $summary_counts['status_mismatch'] ) ); + WP_CLI::line( sprintf( 'Actions – push to node: %d', $summary_counts['push_to_node'] ) ); + WP_CLI::line( sprintf( 'Actions – skipped: %d', $summary_counts['skipped'] ) ); + + if ( ! $is_live && 0 < $summary_counts['push_to_node'] ) { + WP_CLI::line( '' ); + WP_CLI::line( sprintf( 'Run with --live to dispatch %d fix event(s).', $summary_counts['push_to_node'] ) ); + } + } + + /** + * Fetch all membership data from a node via the /integrity-check/memberships endpoint. + * + * @param \Newspack_Network\Hub\Node $node The node to query. + * @param bool $verbose Whether to output verbose information. + * @return array|null Array of membership items, or null on error. + */ + private static function get_node_membership_data( $node, $verbose = false ) { + $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/memberships', $node->get_url() ); + $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); // Cache-busting parameter. + + if ( $verbose ) { + WP_CLI::line( sprintf( ' Fetching memberships from: %s', $endpoint ) ); + } + + // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get + $response = wp_remote_get( + $endpoint, + [ + 'headers' => $node->get_authorization_headers( 'integrity-check' ), + 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout + ] + ); + + if ( is_wp_error( $response ) ) { + WP_CLI::warning( sprintf( 'Failed to fetch memberships from node %s: %s', $node->get_url(), $response->get_error_message() ) ); + return null; + } + + if ( 200 !== wp_remote_retrieve_response_code( $response ) ) { + WP_CLI::warning( sprintf( 'Non-200 response (%d) fetching memberships from node %s.', wp_remote_retrieve_response_code( $response ), $node->get_url() ) ); + return null; + } + + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + return $data['memberships'] ?? []; + } + + /** + * Fetch managed membership data from a node via the /integrity-check/managed-memberships endpoint. + * + * Returns items that include post_modified for timestamp comparison. + * + * @param \Newspack_Network\Hub\Node $node The node to query. + * @param bool $verbose Whether to output verbose information. + * @return array|null Array keyed by email::network_id, or null on error. + */ + private static function get_node_managed_memberships( $node, $verbose = false ) { + $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/managed-memberships', $node->get_url() ); + $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); // Cache-busting parameter. + + if ( $verbose ) { + WP_CLI::line( sprintf( ' Fetching managed memberships from: %s', $endpoint ) ); + } + + // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get + $response = wp_remote_get( + $endpoint, + [ + 'headers' => $node->get_authorization_headers( 'integrity-check' ), + 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout + ] + ); + + if ( is_wp_error( $response ) ) { + WP_CLI::warning( sprintf( 'Failed to fetch managed memberships from node %s: %s', $node->get_url(), $response->get_error_message() ) ); + return null; + } + + if ( 200 !== wp_remote_retrieve_response_code( $response ) ) { + WP_CLI::warning( sprintf( 'Non-200 response (%d) fetching managed memberships from node %s.', wp_remote_retrieve_response_code( $response ), $node->get_url() ) ); + return null; + } + + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $memberships = $data['memberships'] ?? []; + + // Build lookup keyed by email::network_id. + $lookup = []; + foreach ( $memberships as $item ) { + if ( empty( $item['network_id'] ) ) { + continue; + } + $key = $item['email'] . '::' . $item['network_id']; + $lookup[ $key ] = $item; + } + + return $lookup; + } + + /** + * Classify discrepancies between hub and node data. + * + * Compares hub and node membership data keyed by email::network_id and returns + * a list of discrepancy records describing the type and recommended action. + * + * Discrepancy types: + * - missing_on_node: Hub has the membership but the node does not → push_to_node. + * - missing_on_hub: Node has the membership but the hub does not → skip. + * - status_mismatch: Both have it with different statuses. + * Hub timestamp newer (or node timestamp unavailable) → push_to_node. + * Node timestamp newer → skip. + * + * @param array $hub_lookup Hub memberships keyed by email::network_id. + * @param array $node_memberships Raw node membership array (email, status, network_id). + * @param array $node_managed_lookup Node managed memberships keyed by email::network_id (includes post_modified). + * @return array Array of discrepancy records. + */ + private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_lookup ) { + // Build node lookup keyed by email::network_id. + $node_lookup = []; + foreach ( $node_memberships as $item ) { + $key = $item['email'] . '::' . $item['network_id']; + $node_lookup[ $key ] = $item; + } + + $all_keys = array_unique( array_merge( array_keys( $hub_lookup ), array_keys( $node_lookup ) ) ); + $discrepancies = []; + + foreach ( $all_keys as $key ) { + $hub_item = $hub_lookup[ $key ] ?? null; + $node_item = $node_lookup[ $key ] ?? null; + + $parts = explode( '::', $key, 2 ); + $email = $parts[0]; + $network_id = $parts[1] ?? ''; + + $hub_status = $hub_item ? $hub_item['status'] : ''; + $node_status = $node_item ? $node_item['status'] : ''; + + if ( null === $hub_item ) { + // Node has it, hub does not. + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'missing_on_hub', + 'hub_status' => '', + 'node_status' => $node_status, + 'action' => 'skip', + ]; + continue; + } + + if ( null === $node_item ) { + // Hub has it, node does not. + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'missing_on_node', + 'hub_status' => $hub_status, + 'node_status' => '', + 'action' => 'push_to_node', + ]; + continue; + } + + if ( $hub_status === $node_status ) { + // Statuses match – no discrepancy. + continue; + } + + // Status mismatch: compare timestamps to decide direction. + $hub_modified = $hub_item['post_modified'] ?? ''; + $node_modified = ''; + + if ( isset( $node_managed_lookup[ $key ] ) ) { + $node_modified = $node_managed_lookup[ $key ]['post_modified'] ?? ''; + } + + // Hub is authoritative when node timestamp is unavailable or hub is newer. + if ( empty( $node_modified ) || $hub_modified >= $node_modified ) { + $action = 'push_to_node'; + } else { + // Node has fresher data – log and skip. + WP_CLI::warning( + sprintf( + 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', + $email, + $network_id, + $hub_status, + $hub_modified, + $node_status, + $node_modified + ) + ); + $action = 'skip'; + } + + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'status_mismatch', + 'hub_status' => $hub_status, + 'node_status' => $node_status, + 'action' => $action, + ]; + } + + return $discrepancies; + } + + /** + * Dispatch a membership_updated event for the given hub membership item. + * + * Creates an event and persists it to the hub's event log. Nodes will pull + * it during their next sync cycle and update their local membership accordingly. + * + * @param array $hub_item A single hub membership record (email, status, network_id, membership_id). + * @return void + */ + private static function dispatch_to_node( $hub_item ) { + $event_data = [ + 'email' => $hub_item['email'], + 'user_id' => 0, + 'plan_network_id' => $hub_item['network_id'], + 'membership_id' => $hub_item['membership_id'], + 'new_status' => str_replace( 'wcm-', '', $hub_item['status'] ), + ]; + + $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( + get_bloginfo( 'url' ), + $event_data, + time() + ); + + $event->process_in_hub(); + } +} From 1d1451266be28877ab91499f01b49682aaddb5d1 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:19:12 +0100 Subject: [PATCH 07/20] test(memberships): add tests for reconcile discrepancy classification --- .../unit-tests/test-reconcile-memberships.php | 331 ++++++++++++++++++ 1 file changed, 331 insertions(+) create mode 100644 tests/unit-tests/test-reconcile-memberships.php diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php new file mode 100644 index 0000000..772c055 --- /dev/null +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -0,0 +1,331 @@ +setAccessible( true ); + return $classify_discrepancies_method; + } + + /** + * When hub and node data match exactly, there are no discrepancies. + */ + public function test_no_discrepancies_when_data_matches() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = [ + 'alice@example.com::plan-a' => [ + 'email' => 'alice@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-a', + 'post_modified' => '2024-01-15 10:00:00', + 'membership_id' => 101, + ], + ]; + + $node_memberships = [ + [ + 'email' => 'alice@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-a', + ], + ]; + + $node_managed_lookup = []; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertEmpty( $discrepancies ); + } + + /** + * When the hub has a membership the node doesn't, it is classified as + * missing_on_node with action push_to_node. + */ + public function test_missing_on_node_results_in_push_to_node_action() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = [ + 'bob@example.com::plan-b' => [ + 'email' => 'bob@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-b', + 'post_modified' => '2024-02-10 12:00:00', + 'membership_id' => 202, + ], + ]; + + $node_memberships = []; + $node_managed_lookup = []; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'bob@example.com', $discrepancies[0]['email'] ); + $this->assertEquals( 'plan-b', $discrepancies[0]['network_id'] ); + $this->assertEquals( 'missing_on_node', $discrepancies[0]['type'] ); + $this->assertEquals( 'wcm-active', $discrepancies[0]['hub_status'] ); + $this->assertEquals( '', $discrepancies[0]['node_status'] ); + $this->assertEquals( 'push_to_node', $discrepancies[0]['action'] ); + } + + /** + * When the node has a membership the hub doesn't, it is classified as + * missing_on_hub with action skip. + */ + public function test_missing_on_hub_results_in_skip_action() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = []; + + $node_memberships = [ + [ + 'email' => 'carol@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-c', + ], + ]; + + $node_managed_lookup = []; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'carol@example.com', $discrepancies[0]['email'] ); + $this->assertEquals( 'plan-c', $discrepancies[0]['network_id'] ); + $this->assertEquals( 'missing_on_hub', $discrepancies[0]['type'] ); + $this->assertEquals( '', $discrepancies[0]['hub_status'] ); + $this->assertEquals( 'wcm-cancelled', $discrepancies[0]['node_status'] ); + $this->assertEquals( 'skip', $discrepancies[0]['action'] ); + } + + /** + * When there is a status mismatch and the hub's post_modified is later than + * the node's, the hub is authoritative and the action is push_to_node. + */ + public function test_status_mismatch_hub_newer_results_in_push_to_node() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = [ + 'dan@example.com::plan-d' => [ + 'email' => 'dan@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-d', + 'post_modified' => '2024-06-20 09:00:00', + 'membership_id' => 303, + ], + ]; + + $node_memberships = [ + [ + 'email' => 'dan@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-d', + ], + ]; + + // Node managed lookup has an older timestamp than the hub. + $node_managed_lookup = [ + 'dan@example.com::plan-d' => [ + 'email' => 'dan@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-d', + 'post_modified' => '2024-06-10 08:00:00', + 'remote_id' => 303, + ], + ]; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'status_mismatch', $discrepancies[0]['type'] ); + $this->assertEquals( 'wcm-active', $discrepancies[0]['hub_status'] ); + $this->assertEquals( 'wcm-cancelled', $discrepancies[0]['node_status'] ); + $this->assertEquals( 'push_to_node', $discrepancies[0]['action'] ); + } + + /** + * When there is a status mismatch and the node's post_modified is later than + * the hub's, the node is treated as having fresher data and the action is skip. + */ + public function test_status_mismatch_node_newer_results_in_skip() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = [ + 'eve@example.com::plan-e' => [ + 'email' => 'eve@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-e', + 'post_modified' => '2024-03-01 07:00:00', + 'membership_id' => 404, + ], + ]; + + $node_memberships = [ + [ + 'email' => 'eve@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-e', + ], + ]; + + // Node managed lookup has a newer timestamp than the hub. + $node_managed_lookup = [ + 'eve@example.com::plan-e' => [ + 'email' => 'eve@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-e', + 'post_modified' => '2024-03-15 14:00:00', + 'remote_id' => 404, + ], + ]; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'status_mismatch', $discrepancies[0]['type'] ); + $this->assertEquals( 'skip', $discrepancies[0]['action'] ); + } + + /** + * When there is a status mismatch but the membership is absent from the node + * managed lookup, the hub is treated as authoritative and action is push_to_node. + */ + public function test_status_mismatch_no_node_timestamp_defaults_to_hub_authoritative() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + $hub_lookup = [ + 'frank@example.com::plan-f' => [ + 'email' => 'frank@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-f', + 'post_modified' => '2024-04-05 11:00:00', + 'membership_id' => 505, + ], + ]; + + $node_memberships = [ + [ + 'email' => 'frank@example.com', + 'status' => 'wcm-paused', + 'network_id' => 'plan-f', + ], + ]; + + // Node has the membership but it is not in the managed lookup, so no + // timestamp is available for comparison. + $node_managed_lookup = []; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'status_mismatch', $discrepancies[0]['type'] ); + $this->assertEquals( 'wcm-active', $discrepancies[0]['hub_status'] ); + $this->assertEquals( 'wcm-paused', $discrepancies[0]['node_status'] ); + $this->assertEquals( 'push_to_node', $discrepancies[0]['action'] ); + } + + /** + * Multiple discrepancies of different types are all returned in a single call. + */ + public function test_mixed_discrepancies_returns_all_types() { + $classify_discrepancies_method = $this->get_classify_discrepancies_method(); + + // Grace is on hub only → missing_on_node / push_to_node. + // Henry is on node only → missing_on_hub / skip. + // Iris has a status mismatch with hub newer → status_mismatch / push_to_node. + // Jane matches → no discrepancy. + $hub_lookup = [ + 'grace@example.com::plan-g' => [ + 'email' => 'grace@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-g', + 'post_modified' => '2024-05-01 10:00:00', + 'membership_id' => 601, + ], + 'iris@example.com::plan-i' => [ + 'email' => 'iris@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-i', + 'post_modified' => '2024-05-10 10:00:00', + 'membership_id' => 603, + ], + 'jane@example.com::plan-j' => [ + 'email' => 'jane@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-j', + 'post_modified' => '2024-05-12 10:00:00', + 'membership_id' => 604, + ], + ]; + + $node_memberships = [ + [ + 'email' => 'henry@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-h', + ], + [ + 'email' => 'iris@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-i', + ], + [ + 'email' => 'jane@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-j', + ], + ]; + + $node_managed_lookup = [ + 'iris@example.com::plan-i' => [ + 'email' => 'iris@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-i', + 'post_modified' => '2024-05-08 08:00:00', + 'remote_id' => 603, + ], + ]; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + $this->assertCount( 3, $discrepancies ); + + $discrepancy_types_by_email = []; + $discrepancy_actions_by_email = []; + foreach ( $discrepancies as $discrepancy ) { + $discrepancy_types_by_email[ $discrepancy['email'] ] = $discrepancy['type']; + $discrepancy_actions_by_email[ $discrepancy['email'] ] = $discrepancy['action']; + } + + $this->assertEquals( 'missing_on_node', $discrepancy_types_by_email['grace@example.com'] ); + $this->assertEquals( 'push_to_node', $discrepancy_actions_by_email['grace@example.com'] ); + + $this->assertEquals( 'missing_on_hub', $discrepancy_types_by_email['henry@example.com'] ); + $this->assertEquals( 'skip', $discrepancy_actions_by_email['henry@example.com'] ); + + $this->assertEquals( 'status_mismatch', $discrepancy_types_by_email['iris@example.com'] ); + $this->assertEquals( 'push_to_node', $discrepancy_actions_by_email['iris@example.com'] ); + + // Jane matches exactly and must not appear in discrepancies. + $this->assertArrayNotHasKey( 'jane@example.com', $discrepancy_types_by_email ); + } +} From 26db0c2a02312382b2bd503b709b3b5af5863081 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:20:41 +0100 Subject: [PATCH 08/20] fix(reconcile): guard WP_CLI calls in classify_discrepancies for test compatibility --- includes/cli/class-reconcile-memberships.php | 24 +++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/includes/cli/class-reconcile-memberships.php b/includes/cli/class-reconcile-memberships.php index 9e5fae0..6e044c4 100644 --- a/includes/cli/class-reconcile-memberships.php +++ b/includes/cli/class-reconcile-memberships.php @@ -374,17 +374,19 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, $action = 'push_to_node'; } else { // Node has fresher data – log and skip. - WP_CLI::warning( - sprintf( - 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', - $email, - $network_id, - $hub_status, - $hub_modified, - $node_status, - $node_modified - ) - ); + if ( defined( 'WP_CLI' ) && WP_CLI ) { + WP_CLI::warning( + sprintf( + 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', + $email, + $network_id, + $hub_status, + $hub_modified, + $node_status, + $node_modified + ) + ); + } $action = 'skip'; } From db2a9fe929f685771c623853dd43bcb73e146942 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 13:39:50 +0100 Subject: [PATCH 09/20] refactor(integrity-check): move reconciliation into --fix flag on integrity-check command --- includes/class-initializer.php | 1 - includes/cli/class-integrity-check.php | 249 +++++++++- includes/cli/class-reconcile-memberships.php | 432 ------------------ .../unit-tests/test-reconcile-memberships.php | 4 +- 4 files changed, 250 insertions(+), 436 deletions(-) delete mode 100644 includes/cli/class-reconcile-memberships.php diff --git a/includes/class-initializer.php b/includes/class-initializer.php index e1c1ada..044b927 100644 --- a/includes/class-initializer.php +++ b/includes/class-initializer.php @@ -58,7 +58,6 @@ public static function init() { Data_Backfill::init(); Membership_Dedupe::init(); CLI\Integrity_Check::init(); - CLI\Reconcile_Memberships::init(); Woocommerce\Events::init(); Woocommerce\Product_Admin::init(); diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index b5aadbb..f1d2c47 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -48,19 +48,24 @@ public static function register_commands() { * [--max=] * : Maximum number of memberships to process (for testing only - do not use in production). * + * [--fix] + * : Fix discrepancies by dispatching membership update events. + * * ## EXAMPLES * * wp newspack-network integrity-check * wp newspack-network integrity-check --verbose * wp newspack-network integrity-check --max=50 --verbose + * wp newspack-network integrity-check --fix * * @param array $args The command arguments. * @param array $assoc_args The command options. * @return void */ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore Generic.NamingConventions.ConstructorName.OldStyle - $verbose = isset( $assoc_args['verbose'] ) ? true : false; + $verbose = isset( $assoc_args['verbose'] ) ? true : false; $max_records = isset( $assoc_args['max'] ) ? intval( $assoc_args['max'] ) : null; + $fix = isset( $assoc_args['fix'] ); if ( $max_records ) { WP_CLI::warning( sprintf( 'Using --max=%d for testing. Do not use --max in production as it may produce false positives.', $max_records ) ); @@ -157,6 +162,62 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G // Display as table using WP-CLI's table formatter. WP_CLI\Utils\format_items( 'table', $table_data, $node_columns ); } + + if ( $fix ) { + WP_CLI::line( '' ); + WP_CLI::line( 'Analyzing discrepancies for reconciliation...' ); + + $total_dispatched = 0; + $total_skipped = 0; + + // Build hub lookup keyed by email::network_id for fast access. + $hub_lookup = []; + foreach ( $hub_data as $item ) { + $key = $item['email'] . '::' . $item['network_id']; + $hub_lookup[ $key ] = $item; + } + + foreach ( $discrepancies as $node ) { + $node_url = $node->get_url(); + WP_CLI::line( '' ); + WP_CLI::line( sprintf( 'Reconciling node: %s', $node_url ) ); + + // Get managed memberships from node for timestamp comparison. + $node_managed = self::get_node_managed_memberships( $node ); + + // Get full node membership data. + $node_data = self::get_node_membership_data( $node ); + + // Classify discrepancies. + $classified = self::classify_discrepancies( $hub_lookup, $node_data, $node_managed ); + + if ( empty( $classified ) ) { + WP_CLI::line( ' No actionable discrepancies.' ); + continue; + } + + // Display action table. + $action_columns = [ 'email', 'network_id', 'type', 'hub_status', 'node_status', 'action' ]; + WP_CLI\Utils\format_items( 'table', $classified, $action_columns ); + + // Dispatch events. + foreach ( $classified as $item ) { + if ( 'push_to_node' === $item['action'] ) { + $key = $item['email'] . '::' . $item['network_id']; + $hub_item = $hub_lookup[ $key ] ?? null; + if ( $hub_item ) { + self::dispatch_to_node( $hub_item ); + $total_dispatched++; + } + } else { + $total_skipped++; + } + } + } + + WP_CLI::line( '' ); + WP_CLI::success( sprintf( 'Reconciliation complete. Dispatched: %d, Skipped: %d.', $total_dispatched, $total_skipped ) ); + } } @@ -494,4 +555,190 @@ private static function get_node_range_data( $node, $start_email, $end_email, $m $data = self::get_node_range_request( $node, 'range-data', $start_email, $end_email, $max_records ); return $data['memberships'] ?? []; } + /** + * Fetch managed membership data from a node via the /integrity-check/managed-memberships endpoint. + * + * Returns items that include post_modified for timestamp comparison. + * + * @param \Newspack_Network\Hub\Node $node The node to query. + * @return array|null Array keyed by email::network_id, or null on error. + */ + private static function get_node_managed_memberships( $node ) { + $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/managed-memberships', $node->get_url() ); + $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); // Cache-busting parameter. + + // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get + $response = wp_remote_get( + $endpoint, + [ + 'headers' => $node->get_authorization_headers( 'integrity-check' ), + 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout + ] + ); + + if ( is_wp_error( $response ) ) { + WP_CLI::warning( sprintf( 'Failed to fetch managed memberships from node %s: %s', $node->get_url(), $response->get_error_message() ) ); + return null; + } + + if ( 200 !== wp_remote_retrieve_response_code( $response ) ) { + WP_CLI::warning( sprintf( 'Non-200 response (%d) fetching managed memberships from node %s.', wp_remote_retrieve_response_code( $response ), $node->get_url() ) ); + return null; + } + + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + $memberships = $data['memberships'] ?? []; + + // Build lookup keyed by email::network_id. + $lookup = []; + foreach ( $memberships as $item ) { + if ( empty( $item['network_id'] ) ) { + continue; + } + $key = $item['email'] . '::' . $item['network_id']; + $lookup[ $key ] = $item; + } + + return $lookup; + } + + /** + * Classify discrepancies between hub and node data. + * + * Compares hub and node membership data keyed by email::network_id and returns + * a list of discrepancy records describing the type and recommended action. + * + * Discrepancy types: + * - missing_on_node: Hub has the membership but the node does not → push_to_node. + * - missing_on_hub: Node has the membership but the hub does not → skip. + * - status_mismatch: Both have it with different statuses. + * Hub timestamp newer (or node timestamp unavailable) → push_to_node. + * Node timestamp newer → skip. + * + * @param array $hub_lookup Hub memberships keyed by email::network_id. + * @param array $node_memberships Raw node membership array (email, status, network_id). + * @param array $node_managed_lookup Node managed memberships keyed by email::network_id (includes post_modified). + * @return array Array of discrepancy records. + */ + private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_lookup ) { + // Build node lookup keyed by email::network_id. + $node_lookup = []; + foreach ( $node_memberships as $item ) { + $key = $item['email'] . '::' . $item['network_id']; + $node_lookup[ $key ] = $item; + } + + $all_keys = array_unique( array_merge( array_keys( $hub_lookup ), array_keys( $node_lookup ) ) ); + $discrepancies = []; + + foreach ( $all_keys as $key ) { + $hub_item = $hub_lookup[ $key ] ?? null; + $node_item = $node_lookup[ $key ] ?? null; + + $parts = explode( '::', $key, 2 ); + $email = $parts[0]; + $network_id = $parts[1] ?? ''; + + $hub_status = $hub_item ? $hub_item['status'] : ''; + $node_status = $node_item ? $node_item['status'] : ''; + + if ( null === $hub_item ) { + // Node has it, hub does not. + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'missing_on_hub', + 'hub_status' => '', + 'node_status' => $node_status, + 'action' => 'skip', + ]; + continue; + } + + if ( null === $node_item ) { + // Hub has it, node does not. + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'missing_on_node', + 'hub_status' => $hub_status, + 'node_status' => '', + 'action' => 'push_to_node', + ]; + continue; + } + + if ( $hub_status === $node_status ) { + // Statuses match – no discrepancy. + continue; + } + + // Status mismatch: compare timestamps to decide direction. + $hub_modified = $hub_item['post_modified'] ?? ''; + $node_modified = ''; + + if ( isset( $node_managed_lookup[ $key ] ) ) { + $node_modified = $node_managed_lookup[ $key ]['post_modified'] ?? ''; + } + + // Hub is authoritative when node timestamp is unavailable or hub is newer. + if ( empty( $node_modified ) || $hub_modified >= $node_modified ) { + $action = 'push_to_node'; + } else { + // Node has fresher data – log and skip. + if ( defined( 'WP_CLI' ) && WP_CLI ) { + WP_CLI::warning( + sprintf( + 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', + $email, + $network_id, + $hub_status, + $hub_modified, + $node_status, + $node_modified + ) + ); + } + $action = 'skip'; + } + + $discrepancies[] = [ + 'email' => $email, + 'network_id' => $network_id, + 'type' => 'status_mismatch', + 'hub_status' => $hub_status, + 'node_status' => $node_status, + 'action' => $action, + ]; + } + + return $discrepancies; + } + + /** + * Dispatch a membership_updated event for the given hub membership item. + * + * Creates an event and persists it to the hub's event log. Nodes will pull + * it during their next sync cycle and update their local membership accordingly. + * + * @param array $hub_item A single hub membership record (email, status, network_id, membership_id). + * @return void + */ + private static function dispatch_to_node( $hub_item ) { + $event_data = [ + 'email' => $hub_item['email'], + 'user_id' => 0, + 'plan_network_id' => $hub_item['network_id'], + 'membership_id' => $hub_item['membership_id'], + 'new_status' => str_replace( 'wcm-', '', $hub_item['status'] ), + ]; + + $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( + get_bloginfo( 'url' ), + $event_data, + time() + ); + + $event->process_in_hub(); + } } diff --git a/includes/cli/class-reconcile-memberships.php b/includes/cli/class-reconcile-memberships.php deleted file mode 100644 index 6e044c4..0000000 --- a/includes/cli/class-reconcile-memberships.php +++ /dev/null @@ -1,432 +0,0 @@ -] - * : Only reconcile with a specific node URL. - * - * ## EXAMPLES - * - * wp newspack-network reconcile-memberships - * wp newspack-network reconcile-memberships --live - * wp newspack-network reconcile-memberships --verbose - * wp newspack-network reconcile-memberships --node=https://example.com --live - * - * @param array $args The command arguments. - * @param array $assoc_args The command options. - * @return void - */ - public static function reconcile( $args, $assoc_args ) { - $is_live = isset( $assoc_args['live'] ); - $verbose = isset( $assoc_args['verbose'] ); - $node_filter = isset( $assoc_args['node'] ) ? $assoc_args['node'] : null; - - if ( $is_live ) { - WP_CLI::warning( 'Running in LIVE mode – events will be dispatched to fix discrepancies.' ); - } else { - WP_CLI::line( 'Running in DRY-RUN mode. Use --live to dispatch fix events.' ); - } - WP_CLI::line( '' ); - - // Step 1: Get hub membership data. - $hub_data = Integrity_Check_Utils::get_membership_data(); - - if ( $verbose ) { - WP_CLI::line( sprintf( '%d memberships found on the hub.', count( $hub_data ) ) ); - WP_CLI::line( '' ); - } - - // Build lookup keyed by email::network_id for fast access. - $hub_lookup = []; - foreach ( $hub_data as $item ) { - $key = $item['email'] . '::' . $item['network_id']; - $hub_lookup[ $key ] = $item; - } - - // Step 2: Get nodes to process. - $all_nodes = Nodes::get_all_nodes(); - - if ( $node_filter ) { - $all_nodes = array_filter( - $all_nodes, - function( $node ) use ( $node_filter ) { - return rtrim( $node->get_url(), '/' ) === rtrim( $node_filter, '/' ); - } - ); - - if ( empty( $all_nodes ) ) { - WP_CLI::error( sprintf( 'No node found with URL: %s', $node_filter ) ); - } - } - - if ( empty( $all_nodes ) ) { - WP_CLI::warning( 'No nodes found.' ); - return; - } - - // Step 3: Process each node. - $summary_counts = [ - 'missing_on_node' => 0, - 'missing_on_hub' => 0, - 'status_mismatch' => 0, - 'push_to_node' => 0, - 'skipped' => 0, - ]; - - foreach ( $all_nodes as $node ) { - WP_CLI::line( sprintf( 'Processing node: %s', $node->get_url() ) ); - - $node_memberships = self::get_node_membership_data( $node, $verbose ); - if ( null === $node_memberships ) { - WP_CLI::warning( sprintf( 'Skipping node %s due to fetch error.', $node->get_url() ) ); - WP_CLI::line( '' ); - continue; - } - - $node_managed_memberships = self::get_node_managed_memberships( $node, $verbose ); - if ( null === $node_managed_memberships ) { - WP_CLI::warning( sprintf( 'Could not fetch managed memberships from %s – timestamp comparison will use hub as authoritative.', $node->get_url() ) ); - $node_managed_memberships = []; - } - - if ( $verbose ) { - WP_CLI::line( sprintf( ' %d memberships found on node.', count( $node_memberships ) ) ); - WP_CLI::line( sprintf( ' %d managed memberships found on node.', count( $node_managed_memberships ) ) ); - } - - $discrepancies = self::classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_memberships ); - - if ( empty( $discrepancies ) ) { - WP_CLI::success( sprintf( 'Node %s is in sync with the hub.', $node->get_url() ) ); - WP_CLI::line( '' ); - continue; - } - - WP_CLI::line( sprintf( ' Found %d discrepancies:', count( $discrepancies ) ) ); - WP_CLI::line( '' ); - - // Display table of planned actions. - $table_columns = [ 'email', 'network_id', 'type', 'hub_status', 'node_status', 'action' ]; - WP_CLI\Utils\format_items( 'table', $discrepancies, $table_columns ); - WP_CLI::line( '' ); - - // Accumulate summary counts. - foreach ( $discrepancies as $discrepancy ) { - $summary_counts[ $discrepancy['type'] ]++; - if ( 'push_to_node' === $discrepancy['action'] ) { - $summary_counts['push_to_node']++; - } else { - $summary_counts['skipped']++; - } - } - - // In live mode, dispatch events for push_to_node actions. - if ( $is_live ) { - $dispatched = 0; - foreach ( $discrepancies as $discrepancy ) { - if ( 'push_to_node' !== $discrepancy['action'] ) { - continue; - } - $key = $discrepancy['email'] . '::' . $discrepancy['network_id']; - $hub_item = $hub_lookup[ $key ] ?? null; - if ( $hub_item ) { - self::dispatch_to_node( $hub_item ); - $dispatched++; - } - } - WP_CLI::success( sprintf( 'Dispatched %d event(s) for node %s.', $dispatched, $node->get_url() ) ); - } - - WP_CLI::line( '' ); - } - - // Print summary. - WP_CLI::line( '=== Summary ===' ); - WP_CLI::line( sprintf( 'Missing on node: %d', $summary_counts['missing_on_node'] ) ); - WP_CLI::line( sprintf( 'Missing on hub: %d', $summary_counts['missing_on_hub'] ) ); - WP_CLI::line( sprintf( 'Status mismatch: %d', $summary_counts['status_mismatch'] ) ); - WP_CLI::line( sprintf( 'Actions – push to node: %d', $summary_counts['push_to_node'] ) ); - WP_CLI::line( sprintf( 'Actions – skipped: %d', $summary_counts['skipped'] ) ); - - if ( ! $is_live && 0 < $summary_counts['push_to_node'] ) { - WP_CLI::line( '' ); - WP_CLI::line( sprintf( 'Run with --live to dispatch %d fix event(s).', $summary_counts['push_to_node'] ) ); - } - } - - /** - * Fetch all membership data from a node via the /integrity-check/memberships endpoint. - * - * @param \Newspack_Network\Hub\Node $node The node to query. - * @param bool $verbose Whether to output verbose information. - * @return array|null Array of membership items, or null on error. - */ - private static function get_node_membership_data( $node, $verbose = false ) { - $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/memberships', $node->get_url() ); - $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); // Cache-busting parameter. - - if ( $verbose ) { - WP_CLI::line( sprintf( ' Fetching memberships from: %s', $endpoint ) ); - } - - // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get - $response = wp_remote_get( - $endpoint, - [ - 'headers' => $node->get_authorization_headers( 'integrity-check' ), - 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout - ] - ); - - if ( is_wp_error( $response ) ) { - WP_CLI::warning( sprintf( 'Failed to fetch memberships from node %s: %s', $node->get_url(), $response->get_error_message() ) ); - return null; - } - - if ( 200 !== wp_remote_retrieve_response_code( $response ) ) { - WP_CLI::warning( sprintf( 'Non-200 response (%d) fetching memberships from node %s.', wp_remote_retrieve_response_code( $response ), $node->get_url() ) ); - return null; - } - - $data = json_decode( wp_remote_retrieve_body( $response ), true ); - return $data['memberships'] ?? []; - } - - /** - * Fetch managed membership data from a node via the /integrity-check/managed-memberships endpoint. - * - * Returns items that include post_modified for timestamp comparison. - * - * @param \Newspack_Network\Hub\Node $node The node to query. - * @param bool $verbose Whether to output verbose information. - * @return array|null Array keyed by email::network_id, or null on error. - */ - private static function get_node_managed_memberships( $node, $verbose = false ) { - $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/managed-memberships', $node->get_url() ); - $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); // Cache-busting parameter. - - if ( $verbose ) { - WP_CLI::line( sprintf( ' Fetching managed memberships from: %s', $endpoint ) ); - } - - // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get - $response = wp_remote_get( - $endpoint, - [ - 'headers' => $node->get_authorization_headers( 'integrity-check' ), - 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout - ] - ); - - if ( is_wp_error( $response ) ) { - WP_CLI::warning( sprintf( 'Failed to fetch managed memberships from node %s: %s', $node->get_url(), $response->get_error_message() ) ); - return null; - } - - if ( 200 !== wp_remote_retrieve_response_code( $response ) ) { - WP_CLI::warning( sprintf( 'Non-200 response (%d) fetching managed memberships from node %s.', wp_remote_retrieve_response_code( $response ), $node->get_url() ) ); - return null; - } - - $data = json_decode( wp_remote_retrieve_body( $response ), true ); - $memberships = $data['memberships'] ?? []; - - // Build lookup keyed by email::network_id. - $lookup = []; - foreach ( $memberships as $item ) { - if ( empty( $item['network_id'] ) ) { - continue; - } - $key = $item['email'] . '::' . $item['network_id']; - $lookup[ $key ] = $item; - } - - return $lookup; - } - - /** - * Classify discrepancies between hub and node data. - * - * Compares hub and node membership data keyed by email::network_id and returns - * a list of discrepancy records describing the type and recommended action. - * - * Discrepancy types: - * - missing_on_node: Hub has the membership but the node does not → push_to_node. - * - missing_on_hub: Node has the membership but the hub does not → skip. - * - status_mismatch: Both have it with different statuses. - * Hub timestamp newer (or node timestamp unavailable) → push_to_node. - * Node timestamp newer → skip. - * - * @param array $hub_lookup Hub memberships keyed by email::network_id. - * @param array $node_memberships Raw node membership array (email, status, network_id). - * @param array $node_managed_lookup Node managed memberships keyed by email::network_id (includes post_modified). - * @return array Array of discrepancy records. - */ - private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_lookup ) { - // Build node lookup keyed by email::network_id. - $node_lookup = []; - foreach ( $node_memberships as $item ) { - $key = $item['email'] . '::' . $item['network_id']; - $node_lookup[ $key ] = $item; - } - - $all_keys = array_unique( array_merge( array_keys( $hub_lookup ), array_keys( $node_lookup ) ) ); - $discrepancies = []; - - foreach ( $all_keys as $key ) { - $hub_item = $hub_lookup[ $key ] ?? null; - $node_item = $node_lookup[ $key ] ?? null; - - $parts = explode( '::', $key, 2 ); - $email = $parts[0]; - $network_id = $parts[1] ?? ''; - - $hub_status = $hub_item ? $hub_item['status'] : ''; - $node_status = $node_item ? $node_item['status'] : ''; - - if ( null === $hub_item ) { - // Node has it, hub does not. - $discrepancies[] = [ - 'email' => $email, - 'network_id' => $network_id, - 'type' => 'missing_on_hub', - 'hub_status' => '', - 'node_status' => $node_status, - 'action' => 'skip', - ]; - continue; - } - - if ( null === $node_item ) { - // Hub has it, node does not. - $discrepancies[] = [ - 'email' => $email, - 'network_id' => $network_id, - 'type' => 'missing_on_node', - 'hub_status' => $hub_status, - 'node_status' => '', - 'action' => 'push_to_node', - ]; - continue; - } - - if ( $hub_status === $node_status ) { - // Statuses match – no discrepancy. - continue; - } - - // Status mismatch: compare timestamps to decide direction. - $hub_modified = $hub_item['post_modified'] ?? ''; - $node_modified = ''; - - if ( isset( $node_managed_lookup[ $key ] ) ) { - $node_modified = $node_managed_lookup[ $key ]['post_modified'] ?? ''; - } - - // Hub is authoritative when node timestamp is unavailable or hub is newer. - if ( empty( $node_modified ) || $hub_modified >= $node_modified ) { - $action = 'push_to_node'; - } else { - // Node has fresher data – log and skip. - if ( defined( 'WP_CLI' ) && WP_CLI ) { - WP_CLI::warning( - sprintf( - 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', - $email, - $network_id, - $hub_status, - $hub_modified, - $node_status, - $node_modified - ) - ); - } - $action = 'skip'; - } - - $discrepancies[] = [ - 'email' => $email, - 'network_id' => $network_id, - 'type' => 'status_mismatch', - 'hub_status' => $hub_status, - 'node_status' => $node_status, - 'action' => $action, - ]; - } - - return $discrepancies; - } - - /** - * Dispatch a membership_updated event for the given hub membership item. - * - * Creates an event and persists it to the hub's event log. Nodes will pull - * it during their next sync cycle and update their local membership accordingly. - * - * @param array $hub_item A single hub membership record (email, status, network_id, membership_id). - * @return void - */ - private static function dispatch_to_node( $hub_item ) { - $event_data = [ - 'email' => $hub_item['email'], - 'user_id' => 0, - 'plan_network_id' => $hub_item['network_id'], - 'membership_id' => $hub_item['membership_id'], - 'new_status' => str_replace( 'wcm-', '', $hub_item['status'] ), - ]; - - $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( - get_bloginfo( 'url' ), - $event_data, - time() - ); - - $event->process_in_hub(); - } -} diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index 772c055..76600e1 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -5,7 +5,7 @@ * @package Newspack_Network */ -use Newspack_Network\CLI\Reconcile_Memberships; +use Newspack_Network\CLI\Integrity_Check; /** * Test the Reconcile_Memberships::classify_discrepancies method. @@ -20,7 +20,7 @@ class TestReconcileMemberships extends WP_UnitTestCase { * @return ReflectionMethod */ private function get_classify_discrepancies_method() { - $classify_discrepancies_method = new ReflectionMethod( Reconcile_Memberships::class, 'classify_discrepancies' ); + $classify_discrepancies_method = new ReflectionMethod( Integrity_Check::class, 'classify_discrepancies' ); $classify_discrepancies_method->setAccessible( true ); return $classify_discrepancies_method; } From c1b98f79363bcbe3e616b0be2c482dd389c4e4bf Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 25 Mar 2026 23:11:26 +0100 Subject: [PATCH 10/20] feat(integrity-check): detect membership transfers via remote_id cross-referencing --- includes/cli/class-integrity-check.php | 97 ++++++++++++++++++- .../unit-tests/test-reconcile-memberships.php | 97 +++++++++++++++++++ 2 files changed, 192 insertions(+), 2 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index f1d2c47..d154367 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -209,6 +209,12 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G self::dispatch_to_node( $hub_item ); $total_dispatched++; } + } elseif ( 'push_transfer' === $item['action'] ) { + $hub_item = $item['hub_data'] ?? null; + if ( $hub_item ) { + self::dispatch_to_node( $hub_item, $item['previous_email'] ); + $total_dispatched++; + } } else { $total_skipped++; } @@ -611,6 +617,7 @@ private static function get_node_managed_memberships( $node ) { * Discrepancy types: * - missing_on_node: Hub has the membership but the node does not → push_to_node. * - missing_on_hub: Node has the membership but the hub does not → skip. + * - transfer: Node has it under old email, hub has it under new email → push_transfer. * - status_mismatch: Both have it with different statuses. * Hub timestamp newer (or node timestamp unavailable) → push_to_node. * Node timestamp newer → skip. @@ -712,6 +719,87 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, ]; } + // Detect transfers: a missing_on_hub + missing_on_node pair for the same network_id + // where the node's managed membership remote_id matches a hub membership_id. + $missing_on_hub_indices = []; + $missing_on_node_indices = []; + foreach ( $discrepancies as $idx => $d ) { + if ( 'missing_on_hub' === $d['type'] ) { + $missing_on_hub_indices[ $d['network_id'] ][] = $idx; + } elseif ( 'missing_on_node' === $d['type'] ) { + $missing_on_node_indices[ $d['network_id'] ][] = $idx; + } + } + + // Build a hub membership_id → key lookup for matching. + $hub_id_to_key = []; + foreach ( $hub_lookup as $key => $item ) { + if ( ! empty( $item['membership_id'] ) ) { + $hub_id_to_key[ (int) $item['membership_id'] ] = $key; + } + } + + $indices_to_remove = []; + $transfers = []; + + foreach ( $missing_on_hub_indices as $network_id => $hub_indices ) { + if ( empty( $missing_on_node_indices[ $network_id ] ) ) { + continue; + } + + foreach ( $hub_indices as $hub_idx ) { + $old_email = $discrepancies[ $hub_idx ]['email']; + $managed_key = $old_email . '::' . $network_id; + $managed_item = $node_managed_lookup[ $managed_key ] ?? null; + + if ( ! $managed_item || empty( $managed_item['remote_id'] ) ) { + continue; + } + + $remote_id = (int) $managed_item['remote_id']; + $hub_key_for_id = $hub_id_to_key[ $remote_id ] ?? null; + + if ( ! $hub_key_for_id ) { + continue; + } + + $hub_item_for_transfer = $hub_lookup[ $hub_key_for_id ] ?? null; + if ( ! $hub_item_for_transfer ) { + continue; + } + + $new_email = $hub_item_for_transfer['email']; + + // Find the matching missing_on_node entry for the new email. + foreach ( $missing_on_node_indices[ $network_id ] as $node_idx ) { + if ( $discrepancies[ $node_idx ]['email'] === $new_email ) { + $indices_to_remove[] = $hub_idx; + $indices_to_remove[] = $node_idx; + + $transfers[] = [ + 'email' => $new_email, + 'network_id' => $network_id, + 'type' => 'transfer', + 'hub_status' => $hub_item_for_transfer['status'], + 'node_status' => $discrepancies[ $hub_idx ]['node_status'], + 'action' => 'push_transfer', + 'previous_email' => $old_email, + 'hub_data' => $hub_item_for_transfer, + ]; + break; + } + } + } + } + + // Remove matched pairs and add transfers. + if ( ! empty( $indices_to_remove ) ) { + foreach ( array_unique( $indices_to_remove ) as $idx ) { + unset( $discrepancies[ $idx ] ); + } + $discrepancies = array_merge( array_values( $discrepancies ), $transfers ); + } + return $discrepancies; } @@ -721,10 +809,11 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, * Creates an event and persists it to the hub's event log. Nodes will pull * it during their next sync cycle and update their local membership accordingly. * - * @param array $hub_item A single hub membership record (email, status, network_id, membership_id). + * @param array $hub_item A single hub membership record (email, status, network_id, membership_id). + * @param string $previous_email Optional previous owner email for transfer events. * @return void */ - private static function dispatch_to_node( $hub_item ) { + private static function dispatch_to_node( $hub_item, $previous_email = '' ) { $event_data = [ 'email' => $hub_item['email'], 'user_id' => 0, @@ -733,6 +822,10 @@ private static function dispatch_to_node( $hub_item ) { 'new_status' => str_replace( 'wcm-', '', $hub_item['status'] ), ]; + if ( ! empty( $previous_email ) ) { + $event_data['previous_email'] = $previous_email; + } + $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( get_bloginfo( 'url' ), $event_data, diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index 76600e1..243dc11 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -328,4 +328,101 @@ public function test_mixed_discrepancies_returns_all_types() { // Jane matches exactly and must not appear in discrepancies. $this->assertArrayNotHasKey( 'jane@example.com', $discrepancy_types_by_email ); } + + /** + * Test that a membership transfer is detected when missing_on_hub and missing_on_node + * entries for the same network_id are linked by the node's managed membership remote_id. + */ + public function test_transfer_detected_via_remote_id() { + $classify_discrepancies_method = new ReflectionMethod( Integrity_Check::class, 'classify_discrepancies' ); + $classify_discrepancies_method->setAccessible( true ); + + // Hub: membership 500 belongs to newowner@example.com (after transfer). + $hub_lookup = [ + 'newowner@example.com::plan-x' => [ + 'email' => 'newowner@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-x', + 'post_modified' => '2024-06-01 12:00:00', + 'membership_id' => 500, + ], + ]; + + // Node: still has it under the old owner. + $node_memberships = [ + [ + 'email' => 'oldowner@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-x', + ], + ]; + + // Node managed lookup: the old owner's membership points to remote_id 500. + $node_managed_lookup = [ + 'oldowner@example.com::plan-x' => [ + 'email' => 'oldowner@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-x', + 'post_modified' => '2024-04-01 12:00:00', + 'remote_id' => 500, + ], + ]; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + // Should be a single transfer, not two separate missing entries. + $this->assertCount( 1, $discrepancies ); + $this->assertEquals( 'transfer', $discrepancies[0]['type'] ); + $this->assertEquals( 'push_transfer', $discrepancies[0]['action'] ); + $this->assertEquals( 'newowner@example.com', $discrepancies[0]['email'] ); + $this->assertEquals( 'oldowner@example.com', $discrepancies[0]['previous_email'] ); + $this->assertEquals( 'plan-x', $discrepancies[0]['network_id'] ); + } + + /** + * Test that missing_on_hub entries without a matching remote_id are not converted to transfers. + */ + public function test_non_transfer_missing_on_hub_stays_as_skip() { + $classify_discrepancies_method = new ReflectionMethod( Integrity_Check::class, 'classify_discrepancies' ); + $classify_discrepancies_method->setAccessible( true ); + + $hub_lookup = [ + 'alice@example.com::plan-y' => [ + 'email' => 'alice@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-y', + 'post_modified' => '2024-06-01 12:00:00', + 'membership_id' => 600, + ], + ]; + + // Node has a membership under a different email, same plan, but remote_id doesn't match. + $node_memberships = [ + [ + 'email' => 'bob@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-y', + ], + ]; + + $node_managed_lookup = [ + 'bob@example.com::plan-y' => [ + 'email' => 'bob@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-y', + 'post_modified' => '2024-04-01 12:00:00', + 'remote_id' => 999, // Different from hub's membership_id 600. + ], + ]; + + $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); + + // Should be two separate entries, not a transfer. + $this->assertCount( 2, $discrepancies ); + + $types = array_column( $discrepancies, 'type' ); + $this->assertContains( 'missing_on_node', $types ); + $this->assertContains( 'missing_on_hub', $types ); + $this->assertNotContains( 'transfer', $types ); + } } From 5a48dba23803597a249e0e78f33c641dd68f82b0 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 10:35:29 +0100 Subject: [PATCH 11/20] feat(integrity-check): add sync lag check and progress bar for --fix --- includes/cli/class-integrity-check.php | 93 +++++++++++++++++++ .../node/class-integrity-check-endpoints.php | 27 ++++++ 2 files changed, 120 insertions(+) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index d154367..648265c 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -50,6 +50,11 @@ public static function register_commands() { * * [--fix] * : Fix discrepancies by dispatching membership update events. + * Checks node sync status first; if a node has unprocessed events, + * suggests running sync-all before --fix. Use --force to skip this check. + * + * [--force] + * : Skip the sync lag check and dispatch events even if nodes have unprocessed events. * * ## EXAMPLES * @@ -57,6 +62,7 @@ public static function register_commands() { * wp newspack-network integrity-check --verbose * wp newspack-network integrity-check --max=50 --verbose * wp newspack-network integrity-check --fix + * wp newspack-network integrity-check --fix --force * * @param array $args The command arguments. * @param array $assoc_args The command options. @@ -66,6 +72,7 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G $verbose = isset( $assoc_args['verbose'] ) ? true : false; $max_records = isset( $assoc_args['max'] ) ? intval( $assoc_args['max'] ) : null; $fix = isset( $assoc_args['fix'] ); + $force = isset( $assoc_args['force'] ); if ( $max_records ) { WP_CLI::warning( sprintf( 'Using --max=%d for testing. Do not use --max in production as it may produce false positives.', $max_records ) ); @@ -165,6 +172,32 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G if ( $fix ) { WP_CLI::line( '' ); + + // Check if any nodes are behind on sync before creating new events. + if ( ! $force ) { + $hub_latest_id = self::get_hub_latest_event_id(); + $nodes_behind = []; + + foreach ( $discrepancies as $node ) { + $sync_status = self::get_node_sync_status( $node ); + if ( null !== $sync_status && $sync_status < $hub_latest_id ) { + $pending = $hub_latest_id - $sync_status; + $nodes_behind[] = sprintf( ' %s: %d unprocessed events (last processed: %d, hub latest: %d)', $node->get_url(), $pending, $sync_status, $hub_latest_id ); + } + } + + if ( ! empty( $nodes_behind ) ) { + WP_CLI::warning( 'The following nodes have unprocessed events. Discrepancies may resolve after syncing:' ); + foreach ( $nodes_behind as $line ) { + WP_CLI::line( $line ); + } + WP_CLI::line( '' ); + WP_CLI::line( 'Run `wp newspack-network sync-all` on these nodes first, then re-run the integrity check.' ); + WP_CLI::line( 'Use --force to skip this check and dispatch events anyway.' ); + return; + } + } + WP_CLI::line( 'Analyzing discrepancies for reconciliation...' ); $total_dispatched = 0; @@ -200,6 +233,20 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G $action_columns = [ 'email', 'network_id', 'type', 'hub_status', 'node_status', 'action' ]; WP_CLI\Utils\format_items( 'table', $classified, $action_columns ); + // Count actionable items for progress. + $actionable = array_filter( + $classified, + function( $item ) { + return in_array( $item['action'], [ 'push_to_node', 'push_transfer' ], true ); + } + ); + $node_total = count( $actionable ); + $node_done = 0; + + if ( $node_total > 0 ) { + $progress = WP_CLI\Utils\make_progress_bar( sprintf( 'Dispatching %d events', $node_total ), $node_total ); + } + // Dispatch events. foreach ( $classified as $item ) { if ( 'push_to_node' === $item['action'] ) { @@ -208,17 +255,25 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G if ( $hub_item ) { self::dispatch_to_node( $hub_item ); $total_dispatched++; + $node_done++; + $progress->tick(); } } elseif ( 'push_transfer' === $item['action'] ) { $hub_item = $item['hub_data'] ?? null; if ( $hub_item ) { self::dispatch_to_node( $hub_item, $item['previous_email'] ); $total_dispatched++; + $node_done++; + $progress->tick(); } } else { $total_skipped++; } } + + if ( $node_total > 0 ) { + $progress->finish(); + } } WP_CLI::line( '' ); @@ -803,6 +858,44 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, return $discrepancies; } + /** + * Get the latest event log ID from the hub. + * + * @return int The latest event ID, or 0 if the log is empty. + */ + private static function get_hub_latest_event_id() { + $events = \Newspack_Network\Hub\Stores\Event_Log::get( [], 1, 1, 'DESC' ); + return ! empty( $events ) ? $events[0]->get_id() : 0; + } + + /** + * Query a node's last processed event ID via the sync-status endpoint. + * + * @param \Newspack_Network\Hub\Node $node The node to query. + * @return int|null The last processed ID, or null on error. + */ + private static function get_node_sync_status( $node ) { + $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/sync-status', $node->get_url() ); + $endpoint = add_query_arg( [ '_t' => time() ], $endpoint ); + + // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.wp_remote_get_wp_remote_get + $response = wp_remote_get( + $endpoint, + [ + 'headers' => $node->get_authorization_headers( 'integrity-check' ), + 'timeout' => 15, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout + ] + ); + + if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { + WP_CLI::warning( sprintf( 'Could not check sync status for %s, skipping sync lag check.', $node->get_url() ) ); + return null; + } + + $data = json_decode( wp_remote_retrieve_body( $response ), true ); + return $data['last_processed_id'] ?? null; + } + /** * Dispatch a membership_updated event for the given hub membership item. * diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 71fa51b..cc93e63 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -26,6 +26,20 @@ public static function init() { * Register the routes for the integrity check endpoints. */ public static function register_routes() { + register_rest_route( + 'newspack-network/v1', + '/integrity-check/sync-status', + [ + [ + 'methods' => \WP_REST_Server::READABLE, + 'callback' => [ __CLASS__, 'handle_sync_status_request' ], + 'permission_callback' => function( $request ) { + return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); + }, + ], + ] + ); + register_rest_route( 'newspack-network/v1', '/integrity-check/hash', @@ -258,4 +272,17 @@ public static function handle_range_data_request( $request ) { ] ); } + + /** + * Returns the node's last processed event ID for sync lag detection. + * + * @param \WP_REST_Request $request The REST request object. + */ + public static function handle_sync_status_request( $request ) { + return rest_ensure_response( + [ + 'last_processed_id' => (int) Pulling::get_last_processed_id(), + ] + ); + } } From cda0e257c46b62a95b14f338c6080f22f4d86020 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 11:06:03 +0100 Subject: [PATCH 12/20] fix(integrity-check): address review feedback on timestamps, constants, and safety --- includes/class-integrity-check-utils.php | 2 +- includes/cli/class-integrity-check.php | 70 ++++++++++++++----- .../node/class-integrity-check-endpoints.php | 13 ++-- .../unit-tests/test-reconcile-memberships.php | 2 +- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/includes/class-integrity-check-utils.php b/includes/class-integrity-check-utils.php index 40f912f..01d3b46 100644 --- a/includes/class-integrity-check-utils.php +++ b/includes/class-integrity-check-utils.php @@ -30,7 +30,7 @@ private static function build_membership_query( $start_email = null, $end_email u.user_email, p.post_status as status, pm_network.meta_value as network_id, - p.post_modified, + p.post_modified_gmt as post_modified, p.ID as membership_id FROM {$wpdb->posts} p INNER JOIN {$wpdb->users} u ON p.post_author = u.ID diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index 648265c..8a2d7c6 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -218,6 +218,11 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G // Get managed memberships from node for timestamp comparison. $node_managed = self::get_node_managed_memberships( $node ); + if ( null === $node_managed ) { + WP_CLI::warning( sprintf( 'Skipping reconciliation for %s – could not fetch managed memberships.', $node_url ) ); + continue; + } + // Get full node membership data. $node_data = self::get_node_membership_data( $node ); @@ -735,7 +740,7 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, continue; } - // Status mismatch: compare timestamps to decide direction. + // Status mismatch: compare timestamps (GMT) to decide direction. $hub_modified = $hub_item['post_modified'] ?? ''; $node_modified = ''; @@ -743,25 +748,48 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_modified = $node_managed_lookup[ $key ]['post_modified'] ?? ''; } - // Hub is authoritative when node timestamp is unavailable or hub is newer. - if ( empty( $node_modified ) || $hub_modified >= $node_modified ) { + // Hub is authoritative when node timestamp is unavailable. + if ( empty( $node_modified ) ) { $action = 'push_to_node'; } else { - // Node has fresher data – log and skip. - if ( defined( 'WP_CLI' ) && WP_CLI ) { - WP_CLI::warning( - sprintf( - 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', - $email, - $network_id, - $hub_status, - $hub_modified, - $node_status, - $node_modified - ) - ); + $hub_timestamp = $hub_modified ? strtotime( $hub_modified ) : false; + $node_timestamp = $node_modified ? strtotime( $node_modified ) : false; + + if ( false !== $hub_timestamp && false !== $node_timestamp ) { + if ( $hub_timestamp >= $node_timestamp ) { + $action = 'push_to_node'; + } else { + // Node has fresher data – log and skip. + if ( defined( 'WP_CLI' ) && WP_CLI ) { + WP_CLI::warning( + sprintf( + 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', + $email, + $network_id, + $hub_status, + $hub_modified, + $node_status, + $node_modified + ) + ); + } + $action = 'skip'; + } + } else { + // Timestamp parse failure – default to hub as authoritative. + if ( defined( 'WP_CLI' ) && WP_CLI ) { + WP_CLI::warning( + sprintf( + 'Unable to parse timestamps for %s (plan %s): hub=%s, node=%s – defaulting to hub.', + $email, + $network_id, + $hub_modified, + $node_modified + ) + ); + } + $action = 'push_to_node'; } - $action = 'skip'; } $discrepancies[] = [ @@ -919,10 +947,16 @@ private static function dispatch_to_node( $hub_item, $previous_email = '' ) { $event_data['previous_email'] = $previous_email; } + // Use the hub membership's modification time for idempotent dispatch. + $timestamp = ! empty( $hub_item['post_modified'] ) ? strtotime( $hub_item['post_modified'] ) : false; + if ( ! $timestamp ) { + $timestamp = time(); + } + $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( get_bloginfo( 'url' ), $event_data, - time() + $timestamp ); $event->process_in_hub(); diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index cc93e63..1b61059 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -177,9 +177,10 @@ public static function handle_memberships_request( $request ) { } /** - * Handles the managed memberships request. + * Returns all network-managed memberships. * - * Returns all network-managed memberships with their remote_id and remote_site_url. + * Each membership entry includes: email, status, network_id, remote_id, + * remote_site_url, post_modified (GMT), and membership_id. * * @param \WP_REST_Request $request The REST request object. */ @@ -190,7 +191,7 @@ public static function handle_managed_memberships_request( $request ) { // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching $results = $wpdb->get_results( $wpdb->prepare( - "SELECT p.ID, p.post_status, p.post_modified, + "SELECT p.ID, p.post_status, p.post_modified_gmt as post_modified, u.user_email, pm_remote.meta_value as remote_id, pm_site.meta_value as remote_site_url, @@ -202,9 +203,9 @@ public static function handle_managed_memberships_request( $request ) { LEFT JOIN {$wpdb->postmeta} pm_site ON p.ID = pm_site.post_id AND pm_site.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s WHERE p.post_type = 'wc_user_membership'", - '_managed_by_newspack_network', - '_remote_id', - '_remote_site_url', + \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_MANAGED_META_KEY, + \Newspack_Network\Woocommerce_Memberships\Admin::REMOTE_ID_META_KEY, + \Newspack_Network\Woocommerce_Memberships\Admin::SITE_URL_META_KEY, \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY ) ); diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index 243dc11..027fbbb 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -8,7 +8,7 @@ use Newspack_Network\CLI\Integrity_Check; /** - * Test the Reconcile_Memberships::classify_discrepancies method. + * Test the Newspack_Network\CLI\Integrity_Check::classify_discrepancies method. * * @group reconcile */ From 4634766c5dfe9afd963f2f4aae02badf24a20f59 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 11:23:16 +0100 Subject: [PATCH 13/20] fix(integrity-check): harden queries, error handling, and input sanitization --- includes/class-integrity-check-utils.php | 12 +- includes/cli/class-integrity-check.php | 104 ++++++++++++------ .../node/class-integrity-check-endpoints.php | 67 +++++------ 3 files changed, 116 insertions(+), 67 deletions(-) diff --git a/includes/class-integrity-check-utils.php b/includes/class-integrity-check-utils.php index 01d3b46..0d5c300 100644 --- a/includes/class-integrity-check-utils.php +++ b/includes/class-integrity-check-utils.php @@ -36,20 +36,22 @@ private static function build_membership_query( $start_email = null, $end_email INNER JOIN {$wpdb->users} u ON p.post_author = u.ID INNER JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s INNER JOIN ( - SELECT + SELECT p2.post_author, pm2.meta_value, - MAX(p2.post_date) as max_date + MAX(p2.post_modified_gmt) as max_modified FROM {$wpdb->posts} p2 INNER JOIN {$wpdb->postmeta} pm2 ON p2.post_parent = pm2.post_id AND pm2.meta_key = %s WHERE p2.post_type = 'wc_user_membership' + AND p2.post_status != 'trash' AND pm2.meta_value IS NOT NULL AND pm2.meta_value != '' GROUP BY p2.post_author, pm2.meta_value - ) latest ON p.post_author = latest.post_author - AND pm_network.meta_value = latest.meta_value - AND p.post_date = latest.max_date + ) latest ON p.post_author = latest.post_author + AND pm_network.meta_value = latest.meta_value + AND p.post_modified_gmt = latest.max_modified WHERE p.post_type = 'wc_user_membership' + AND p.post_status != 'trash' AND pm_network.meta_value IS NOT NULL AND pm_network.meta_value != ''"; diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index 8a2d7c6..aec7d6a 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -52,6 +52,7 @@ public static function register_commands() { * : Fix discrepancies by dispatching membership update events. * Checks node sync status first; if a node has unprocessed events, * suggests running sync-all before --fix. Use --force to skip this check. + * For a dry run, omit --fix: the command will report discrepancies without dispatching. * * [--force] * : Skip the sync lag check and dispatch events even if nodes have unprocessed events. @@ -98,6 +99,10 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G foreach ( $nodes as $node ) { $node_hash = self::get_node_hash( $node, $max_records ); + if ( null === $node_hash ) { + continue; // Warning already logged. + } + if ( $verbose ) { WP_CLI::line( sprintf( 'Node %s hash: %s', $node->get_url(), $node_hash ) ); } @@ -226,6 +231,11 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G // Get full node membership data. $node_data = self::get_node_membership_data( $node ); + if ( null === $node_data ) { + WP_CLI::warning( sprintf( 'Skipping reconciliation for %s – could not fetch membership data.', $node_url ) ); + continue; + } + // Classify discrepancies. $classified = self::classify_discrepancies( $hub_lookup, $node_data, $node_managed ); @@ -253,31 +263,33 @@ function( $item ) { } // Dispatch events. - foreach ( $classified as $item ) { - if ( 'push_to_node' === $item['action'] ) { - $key = $item['email'] . '::' . $item['network_id']; - $hub_item = $hub_lookup[ $key ] ?? null; - if ( $hub_item ) { - self::dispatch_to_node( $hub_item ); - $total_dispatched++; - $node_done++; - $progress->tick(); - } - } elseif ( 'push_transfer' === $item['action'] ) { - $hub_item = $item['hub_data'] ?? null; - if ( $hub_item ) { - self::dispatch_to_node( $hub_item, $item['previous_email'] ); - $total_dispatched++; - $node_done++; - $progress->tick(); + try { + foreach ( $classified as $item ) { + if ( 'push_to_node' === $item['action'] ) { + $key = $item['email'] . '::' . $item['network_id']; + $hub_item = $hub_lookup[ $key ] ?? null; + if ( $hub_item ) { + self::dispatch_to_node( $hub_item ); + $total_dispatched++; + $node_done++; + $progress->tick(); + } + } elseif ( 'push_transfer' === $item['action'] ) { + $hub_item = $item['hub_data'] ?? null; + if ( $hub_item ) { + self::dispatch_to_node( $hub_item, $item['previous_email'] ); + $total_dispatched++; + $node_done++; + $progress->tick(); + } + } else { + $total_skipped++; } - } else { - $total_skipped++; } - } - - if ( $node_total > 0 ) { - $progress->finish(); + } finally { + if ( $node_total > 0 ) { + $progress->finish(); + } } } @@ -306,7 +318,8 @@ private static function get_node_membership_data( $node ) { ); if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { - WP_CLI::error( sprintf( 'Failed to get membership data from node: %s', $node->get_url() ) ); + WP_CLI::warning( sprintf( 'Failed to get membership data from node: %s', $node->get_url() ) ); + return null; } $body = wp_remote_retrieve_body( $response ); @@ -341,7 +354,8 @@ private static function get_node_hash( $node, $max_records = null ) { ); if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { - WP_CLI::error( sprintf( 'Failed to get hash from node: %s', $node->get_url() ) ); + WP_CLI::warning( sprintf( 'Failed to get hash from node: %s', $node->get_url() ) ); + return null; } $body = wp_remote_retrieve_body( $response ); @@ -386,6 +400,11 @@ private static function find_discrepancies_chunked( $hub_data, $node, $verbose = // Get corresponding chunk hash from node using range. $node_chunk_hash = self::get_node_range_hash( $node, $range['start'], $range['end'], $max_records ); + if ( null === $node_chunk_hash ) { + // Treat unreachable chunk as a full mismatch. + $node_chunk_hash = ''; + } + if ( $verbose ) { WP_CLI::line( sprintf( @@ -567,8 +586,8 @@ private static function get_node_range_request( $node, $endpoint_type, $start_em $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/%s', $node->get_url(), $endpoint_type ); $query_args = [ - 'start' => strtolower( $start_email ), - 'end' => strtolower( $end_email ), + 'start' => rawurlencode( strtolower( $start_email ) ), + 'end' => rawurlencode( strtolower( $end_email ) ), '_t' => time(), // Cache-busting parameter. ]; @@ -587,7 +606,8 @@ private static function get_node_range_request( $node, $endpoint_type, $start_em if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { $error_type = str_replace( '-', ' ', $endpoint_type ); - WP_CLI::error( sprintf( 'Failed to get %s from node: %s', $error_type, $node->get_url() ) ); + WP_CLI::warning( sprintf( 'Failed to get %s from node: %s', $error_type, $node->get_url() ) ); + return null; } $body = wp_remote_retrieve_body( $response ); @@ -605,6 +625,9 @@ private static function get_node_range_request( $node, $endpoint_type, $start_em */ private static function get_node_range_hash( $node, $start_email, $end_email, $max_records = null ) { $data = self::get_node_range_request( $node, 'range-hash', $start_email, $end_email, $max_records ); + if ( null === $data ) { + return null; + } return $data['hash'] ?? ''; } @@ -619,6 +642,9 @@ private static function get_node_range_hash( $node, $start_email, $end_email, $m */ private static function get_node_range_data( $node, $start_email, $end_email, $max_records = null ) { $data = self::get_node_range_request( $node, 'range-data', $start_email, $end_email, $max_records ); + if ( null === $data ) { + return []; + } return $data['memberships'] ?? []; } /** @@ -752,8 +778,8 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, if ( empty( $node_modified ) ) { $action = 'push_to_node'; } else { - $hub_timestamp = $hub_modified ? strtotime( $hub_modified ) : false; - $node_timestamp = $node_modified ? strtotime( $node_modified ) : false; + $hub_timestamp = self::parse_gmt_timestamp( $hub_modified ); + $node_timestamp = self::parse_gmt_timestamp( $node_modified ); if ( false !== $hub_timestamp && false !== $node_timestamp ) { if ( $hub_timestamp >= $node_timestamp ) { @@ -924,6 +950,22 @@ private static function get_node_sync_status( $node ) { return $data['last_processed_id'] ?? null; } + /** + * Parse a GMT timestamp string into a Unix timestamp. + * + * Uses explicit UTC timezone to avoid dependence on the server's default timezone. + * + * @param string $date_string A date string in 'Y-m-d H:i:s' format (GMT). + * @return int|false Unix timestamp, or false on parse failure. + */ + private static function parse_gmt_timestamp( $date_string ) { + if ( empty( $date_string ) ) { + return false; + } + $dt = \DateTimeImmutable::createFromFormat( 'Y-m-d H:i:s', $date_string, new \DateTimeZone( 'UTC' ) ); + return $dt ? $dt->getTimestamp() : false; + } + /** * Dispatch a membership_updated event for the given hub membership item. * @@ -948,7 +990,7 @@ private static function dispatch_to_node( $hub_item, $previous_email = '' ) { } // Use the hub membership's modification time for idempotent dispatch. - $timestamp = ! empty( $hub_item['post_modified'] ) ? strtotime( $hub_item['post_modified'] ) : false; + $timestamp = ! empty( $hub_item['post_modified'] ) ? self::parse_gmt_timestamp( $hub_item['post_modified'] ) : false; if ( ! $timestamp ) { $timestamp = time(); } diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 1b61059..407cdf4 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -33,9 +33,7 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_sync_status_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], ], ] ); @@ -47,9 +45,7 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_hash_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], ], ] ); @@ -61,9 +57,7 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_memberships_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], ], ] ); @@ -75,9 +69,7 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_managed_memberships_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], ], ] ); @@ -89,21 +81,22 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_range_hash_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], 'args' => [ 'start' => [ - 'required' => true, - 'type' => 'string', + 'required' => true, + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', ], 'end' => [ - 'required' => true, - 'type' => 'string', + 'required' => true, + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', ], 'max' => [ - 'required' => false, - 'type' => 'integer', + 'required' => false, + 'type' => 'integer', + 'sanitize_callback' => 'absint', ], ], ], @@ -117,21 +110,22 @@ public static function register_routes() { [ 'methods' => \WP_REST_Server::READABLE, 'callback' => [ __CLASS__, 'handle_range_data_request' ], - 'permission_callback' => function( $request ) { - return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); - }, + 'permission_callback' => [ __CLASS__, 'check_permission' ], 'args' => [ 'start' => [ - 'required' => true, - 'type' => 'string', + 'required' => true, + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', ], 'end' => [ - 'required' => true, - 'type' => 'string', + 'required' => true, + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', ], 'max' => [ - 'required' => false, - 'type' => 'integer', + 'required' => false, + 'type' => 'integer', + 'sanitize_callback' => 'absint', ], ], ], @@ -202,7 +196,8 @@ public static function handle_managed_memberships_request( $request ) { LEFT JOIN {$wpdb->postmeta} pm_remote ON p.ID = pm_remote.post_id AND pm_remote.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_site ON p.ID = pm_site.post_id AND pm_site.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s - WHERE p.post_type = 'wc_user_membership'", + WHERE p.post_type = 'wc_user_membership' + AND p.post_status != 'trash'", \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_MANAGED_META_KEY, \Newspack_Network\Woocommerce_Memberships\Admin::REMOTE_ID_META_KEY, \Newspack_Network\Woocommerce_Memberships\Admin::SITE_URL_META_KEY, @@ -286,4 +281,14 @@ public static function handle_sync_status_request( $request ) { ] ); } + + /** + * Permission callback for all integrity check endpoints. + * + * @param \WP_REST_Request $request The REST request object. + * @return bool + */ + public static function check_permission( $request ) { + return \Newspack_Network\Rest_Authenticaton::verify_signature( $request, 'integrity-check', Settings::get_secret_key() ); + } } From 6cce58656fb98f4c5561f8ce335ebf135dad96a5 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 12:13:27 +0100 Subject: [PATCH 14/20] feat(integrity-check): fix missing_on_hub discrepancies by pulling from node --- includes/cli/class-integrity-check.php | 50 +++++++++++++++++-- .../unit-tests/test-reconcile-memberships.php | 5 +- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index aec7d6a..e7c4832 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -252,7 +252,7 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G $actionable = array_filter( $classified, function( $item ) { - return in_array( $item['action'], [ 'push_to_node', 'push_transfer' ], true ); + return in_array( $item['action'], [ 'push_to_node', 'push_transfer', 'pull_to_hub' ], true ); } ); $node_total = count( $actionable ); @@ -282,6 +282,14 @@ function( $item ) { $node_done++; $progress->tick(); } + } elseif ( 'pull_to_hub' === $item['action'] ) { + $node_item_data = $item['node_data'] ?? null; + if ( $node_item_data && ! empty( $node_item_data['membership_id'] ) ) { + self::dispatch_to_hub( $node_item_data, $node_url ); + $total_dispatched++; + $node_done++; + $progress->tick(); + } } else { $total_skipped++; } @@ -702,7 +710,7 @@ private static function get_node_managed_memberships( $node ) { * * Discrepancy types: * - missing_on_node: Hub has the membership but the node does not → push_to_node. - * - missing_on_hub: Node has the membership but the hub does not → skip. + * - missing_on_hub: Node has the membership but the hub does not → pull_to_hub. * - transfer: Node has it under old email, hub has it under new email → push_transfer. * - status_mismatch: Both have it with different statuses. * Hub timestamp newer (or node timestamp unavailable) → push_to_node. @@ -736,14 +744,15 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_status = $node_item ? $node_item['status'] : ''; if ( null === $hub_item ) { - // Node has it, hub does not. + // Node has it, hub does not – pull from node to hub. $discrepancies[] = [ 'email' => $email, 'network_id' => $network_id, 'type' => 'missing_on_hub', 'hub_status' => '', 'node_status' => $node_status, - 'action' => 'skip', + 'action' => 'pull_to_hub', + 'node_data' => $node_item, ]; continue; } @@ -966,6 +975,39 @@ private static function parse_gmt_timestamp( $date_string ) { return $dt ? $dt->getTimestamp() : false; } + /** + * Dispatch a membership_updated event from node data to create the membership on the hub. + * + * Used for missing_on_hub discrepancies: the node has a membership that the hub doesn't. + * Creates an event attributed to the node so the hub processes it locally. + * + * @param array $node_item A node membership record (email, status, network_id, membership_id). + * @param string $node_url The node's URL (used as the event's originating site). + * @return void + */ + private static function dispatch_to_hub( $node_item, $node_url ) { + $event_data = [ + 'email' => $node_item['email'], + 'user_id' => 0, + 'plan_network_id' => $node_item['network_id'], + 'membership_id' => $node_item['membership_id'] ?? 0, + 'new_status' => str_replace( 'wcm-', '', $node_item['status'] ), + ]; + + $timestamp = ! empty( $node_item['post_modified'] ) ? self::parse_gmt_timestamp( $node_item['post_modified'] ) : false; + if ( ! $timestamp ) { + $timestamp = time(); + } + + $event = new \Newspack_Network\Incoming_Events\Woocommerce_Membership_Updated( + $node_url, + $event_data, + $timestamp + ); + + $event->process_in_hub(); + } + /** * Dispatch a membership_updated event for the given hub membership item. * diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index 027fbbb..0c75307 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -114,7 +114,8 @@ public function test_missing_on_hub_results_in_skip_action() { $this->assertEquals( 'missing_on_hub', $discrepancies[0]['type'] ); $this->assertEquals( '', $discrepancies[0]['hub_status'] ); $this->assertEquals( 'wcm-cancelled', $discrepancies[0]['node_status'] ); - $this->assertEquals( 'skip', $discrepancies[0]['action'] ); + $this->assertEquals( 'pull_to_hub', $discrepancies[0]['action'] ); + $this->assertArrayHasKey( 'node_data', $discrepancies[0] ); } /** @@ -320,7 +321,7 @@ public function test_mixed_discrepancies_returns_all_types() { $this->assertEquals( 'push_to_node', $discrepancy_actions_by_email['grace@example.com'] ); $this->assertEquals( 'missing_on_hub', $discrepancy_types_by_email['henry@example.com'] ); - $this->assertEquals( 'skip', $discrepancy_actions_by_email['henry@example.com'] ); + $this->assertEquals( 'pull_to_hub', $discrepancy_actions_by_email['henry@example.com'] ); $this->assertEquals( 'status_mismatch', $discrepancy_types_by_email['iris@example.com'] ); $this->assertEquals( 'push_to_node', $discrepancy_actions_by_email['iris@example.com'] ); From ee2550f74cdd0f104eef355a9a6dfae08c7fd0cc Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 12:41:28 +0100 Subject: [PATCH 15/20] fix(integrity-check): sync lag check should only consider pullable event types --- includes/cli/class-integrity-check.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index e7c4832..52ce79e 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -922,12 +922,20 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, } /** - * Get the latest event log ID from the hub. + * Get the latest pullable event log ID from the hub. + * + * Only considers event types that nodes actually pull (ACTIONS_THAT_NODES_PULL), + * avoiding false positives from non-pullable events like order_changed. * * @return int The latest event ID, or 0 if the log is empty. */ private static function get_hub_latest_event_id() { - $events = \Newspack_Network\Hub\Stores\Event_Log::get( [], 1, 1, 'DESC' ); + $events = \Newspack_Network\Hub\Stores\Event_Log::get( + [ 'action_name_in' => \Newspack_Network\Accepted_Actions::ACTIONS_THAT_NODES_PULL ], + 1, + 1, + 'DESC' + ); return ! empty( $events ) ? $events[0]->get_id() : 0; } From e6c51399ca6d58243c0007042dbde4a8b069495f Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 12:59:08 +0100 Subject: [PATCH 16/20] fix(integrity-check): skip push_to_node when node lacks the membership plan --- includes/cli/class-integrity-check.php | 69 ++++++++++++------- .../node/class-integrity-check-endpoints.php | 18 ++++- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index 52ce79e..24d3c9e 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -178,29 +178,36 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G if ( $fix ) { WP_CLI::line( '' ); - // Check if any nodes are behind on sync before creating new events. - if ( ! $force ) { - $hub_latest_id = self::get_hub_latest_event_id(); - $nodes_behind = []; - - foreach ( $discrepancies as $node ) { - $sync_status = self::get_node_sync_status( $node ); - if ( null !== $sync_status && $sync_status < $hub_latest_id ) { - $pending = $hub_latest_id - $sync_status; - $nodes_behind[] = sprintf( ' %s: %d unprocessed events (last processed: %d, hub latest: %d)', $node->get_url(), $pending, $sync_status, $hub_latest_id ); + // Query sync status and plan availability from all nodes. + $node_plan_ids = []; // Keyed by node URL. + $nodes_behind = []; + + foreach ( $discrepancies as $node ) { + $sync_status = self::get_node_sync_status( $node ); + if ( null === $sync_status ) { + continue; + } + $node_plan_ids[ $node->get_url() ] = $sync_status['plan_network_ids']; + + if ( ! $force ) { + $hub_latest_id = self::get_hub_latest_event_id(); + $last_id = $sync_status['last_processed_id']; + if ( null !== $last_id && $last_id < $hub_latest_id ) { + $pending = $hub_latest_id - $last_id; + $nodes_behind[] = sprintf( ' %s: %d unprocessed events (last processed: %d, hub latest: %d)', $node->get_url(), $pending, $last_id, $hub_latest_id ); } } + } - if ( ! empty( $nodes_behind ) ) { - WP_CLI::warning( 'The following nodes have unprocessed events. Discrepancies may resolve after syncing:' ); - foreach ( $nodes_behind as $line ) { - WP_CLI::line( $line ); - } - WP_CLI::line( '' ); - WP_CLI::line( 'Run `wp newspack-network sync-all` on these nodes first, then re-run the integrity check.' ); - WP_CLI::line( 'Use --force to skip this check and dispatch events anyway.' ); - return; + if ( ! $force && ! empty( $nodes_behind ) ) { + WP_CLI::warning( 'The following nodes have unprocessed events. Discrepancies may resolve after syncing:' ); + foreach ( $nodes_behind as $line ) { + WP_CLI::line( $line ); } + WP_CLI::line( '' ); + WP_CLI::line( 'Run `wp newspack-network sync-all` on these nodes first, then re-run the integrity check.' ); + WP_CLI::line( 'Use --force to skip this check and dispatch events anyway.' ); + return; } WP_CLI::line( 'Analyzing discrepancies for reconciliation...' ); @@ -237,7 +244,8 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G } // Classify discrepancies. - $classified = self::classify_discrepancies( $hub_lookup, $node_data, $node_managed ); + $available_plans = $node_plan_ids[ $node_url ] ?? []; + $classified = self::classify_discrepancies( $hub_lookup, $node_data, $node_managed, $available_plans ); if ( empty( $classified ) ) { WP_CLI::line( ' No actionable discrepancies.' ); @@ -719,9 +727,10 @@ private static function get_node_managed_memberships( $node ) { * @param array $hub_lookup Hub memberships keyed by email::network_id. * @param array $node_memberships Raw node membership array (email, status, network_id). * @param array $node_managed_lookup Node managed memberships keyed by email::network_id (includes post_modified). + * @param array $node_plan_ids Plan network IDs available on the node (empty = no filtering). * @return array Array of discrepancy records. */ - private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_lookup ) { + private static function classify_discrepancies( $hub_lookup, $node_memberships, $node_managed_lookup, $node_plan_ids = [] ) { // Build node lookup keyed by email::network_id. $node_lookup = []; foreach ( $node_memberships as $item ) { @@ -759,13 +768,18 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, if ( null === $node_item ) { // Hub has it, node does not. + // Skip if the node doesn't have the plan – can't create the membership. + $action = 'push_to_node'; + if ( ! empty( $node_plan_ids ) && ! in_array( $network_id, $node_plan_ids, true ) ) { + $action = 'skip_no_plan'; + } $discrepancies[] = [ 'email' => $email, 'network_id' => $network_id, 'type' => 'missing_on_node', 'hub_status' => $hub_status, 'node_status' => '', - 'action' => 'push_to_node', + 'action' => $action, ]; continue; } @@ -940,10 +954,10 @@ private static function get_hub_latest_event_id() { } /** - * Query a node's last processed event ID via the sync-status endpoint. + * Query a node's sync status and plan availability via the sync-status endpoint. * * @param \Newspack_Network\Hub\Node $node The node to query. - * @return int|null The last processed ID, or null on error. + * @return array|null Array with 'last_processed_id' and 'plan_network_ids', or null on error. */ private static function get_node_sync_status( $node ) { $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/sync-status', $node->get_url() ); @@ -959,12 +973,15 @@ private static function get_node_sync_status( $node ) { ); if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { - WP_CLI::warning( sprintf( 'Could not check sync status for %s, skipping sync lag check.', $node->get_url() ) ); + WP_CLI::warning( sprintf( 'Could not check sync status for %s.', $node->get_url() ) ); return null; } $data = json_decode( wp_remote_retrieve_body( $response ), true ); - return $data['last_processed_id'] ?? null; + return [ + 'last_processed_id' => $data['last_processed_id'] ?? null, + 'plan_network_ids' => $data['plan_network_ids'] ?? [], + ]; } /** diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 407cdf4..156ea2b 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -275,9 +275,25 @@ public static function handle_range_data_request( $request ) { * @param \WP_REST_Request $request The REST request object. */ public static function handle_sync_status_request( $request ) { + global $wpdb; + + // Collect plan network IDs available on this node. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching + $plan_network_ids = $wpdb->get_col( + $wpdb->prepare( + "SELECT pm.meta_value FROM {$wpdb->postmeta} pm + INNER JOIN {$wpdb->posts} p ON pm.post_id = p.ID + WHERE p.post_type = %s AND pm.meta_key = %s + AND pm.meta_value IS NOT NULL AND pm.meta_value != ''", + \Newspack_Network\Woocommerce_Memberships\Admin::MEMBERSHIP_PLANS_CPT, + \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY + ) + ); + return rest_ensure_response( [ - 'last_processed_id' => (int) Pulling::get_last_processed_id(), + 'last_processed_id' => (int) Pulling::get_last_processed_id(), + 'plan_network_ids' => $plan_network_ids ?: [], ] ); } From 976ea142bd7279a99b9c7e2b8bc33bbb278f2117 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 26 Mar 2026 13:02:52 +0100 Subject: [PATCH 17/20] fix(integrity-check): fix linting issues --- includes/node/class-integrity-check-endpoints.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 156ea2b..4c63320 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -292,8 +292,8 @@ public static function handle_sync_status_request( $request ) { return rest_ensure_response( [ - 'last_processed_id' => (int) Pulling::get_last_processed_id(), - 'plan_network_ids' => $plan_network_ids ?: [], + 'last_processed_id' => (int) Pulling::get_last_processed_id(), + 'plan_network_ids' => ! empty( $plan_network_ids ) ? $plan_network_ids : [], ] ); } From 29552b046dea253aaeef1a463338e5070116b4f1 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Fri, 27 Mar 2026 11:34:16 +0100 Subject: [PATCH 18/20] fix(integrity-check): address review feedback --- includes/cli/class-integrity-check.php | 8 ++------ tests/unit-tests/test-reconcile-memberships.php | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index 24d3c9e..d095f2b 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -264,7 +264,6 @@ function( $item ) { } ); $node_total = count( $actionable ); - $node_done = 0; if ( $node_total > 0 ) { $progress = WP_CLI\Utils\make_progress_bar( sprintf( 'Dispatching %d events', $node_total ), $node_total ); @@ -279,7 +278,6 @@ function( $item ) { if ( $hub_item ) { self::dispatch_to_node( $hub_item ); $total_dispatched++; - $node_done++; $progress->tick(); } } elseif ( 'push_transfer' === $item['action'] ) { @@ -287,7 +285,6 @@ function( $item ) { if ( $hub_item ) { self::dispatch_to_node( $hub_item, $item['previous_email'] ); $total_dispatched++; - $node_done++; $progress->tick(); } } elseif ( 'pull_to_hub' === $item['action'] ) { @@ -295,7 +292,6 @@ function( $item ) { if ( $node_item_data && ! empty( $node_item_data['membership_id'] ) ) { self::dispatch_to_hub( $node_item_data, $node_url ); $total_dispatched++; - $node_done++; $progress->tick(); } } else { @@ -602,8 +598,8 @@ private static function get_node_range_request( $node, $endpoint_type, $start_em $endpoint = sprintf( '%s/wp-json/newspack-network/v1/integrity-check/%s', $node->get_url(), $endpoint_type ); $query_args = [ - 'start' => rawurlencode( strtolower( $start_email ) ), - 'end' => rawurlencode( strtolower( $end_email ) ), + 'start' => strtolower( $start_email ), + 'end' => strtolower( $end_email ), '_t' => time(), // Cache-busting parameter. ]; diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index 0c75307..f6de5ea 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -89,9 +89,9 @@ public function test_missing_on_node_results_in_push_to_node_action() { /** * When the node has a membership the hub doesn't, it is classified as - * missing_on_hub with action skip. + * missing_on_hub with action pull_to_hub. */ - public function test_missing_on_hub_results_in_skip_action() { + public function test_missing_on_hub_results_in_pull_to_hub_action() { $classify_discrepancies_method = $this->get_classify_discrepancies_method(); $hub_lookup = []; From c350b2d79e30a908198dc390da67c3c4d4fe7988 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Mon, 30 Mar 2026 12:09:35 +0200 Subject: [PATCH 19/20] fix(integrity-check): use subscription presence instead of timestamps for status mismatch --- includes/class-integrity-check-utils.php | 15 +- includes/cli/class-integrity-check.php | 67 ++----- .../unit-tests/test-reconcile-memberships.php | 165 ++++++++---------- 3 files changed, 100 insertions(+), 147 deletions(-) diff --git a/includes/class-integrity-check-utils.php b/includes/class-integrity-check-utils.php index 0d5c300..01c741e 100644 --- a/includes/class-integrity-check-utils.php +++ b/includes/class-integrity-check-utils.php @@ -31,10 +31,12 @@ private static function build_membership_query( $start_email = null, $end_email p.post_status as status, pm_network.meta_value as network_id, p.post_modified_gmt as post_modified, - p.ID as membership_id + p.ID as membership_id, + CASE WHEN pm_sub.meta_value IS NOT NULL AND pm_sub.meta_value != '' THEN 1 ELSE 0 END as has_subscription FROM {$wpdb->posts} p INNER JOIN {$wpdb->users} u ON p.post_author = u.ID INNER JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_sub ON p.ID = pm_sub.post_id AND pm_sub.meta_key = '_subscription_id' INNER JOIN ( SELECT p2.post_author, @@ -90,11 +92,12 @@ private static function execute_membership_query( $query, $prepare_args, $max_re $membership_data = []; foreach ( $results as $result ) { $membership_data[] = [ - 'email' => strtolower( $result->user_email ), - 'status' => $result->status, - 'network_id' => $result->network_id, - 'post_modified' => $result->post_modified, - 'membership_id' => (int) $result->membership_id, + 'email' => strtolower( $result->user_email ), + 'status' => $result->status, + 'network_id' => $result->network_id, + 'post_modified' => $result->post_modified, + 'membership_id' => (int) $result->membership_id, + 'has_subscription' => (bool) ( $result->has_subscription ?? false ), ]; } diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index d095f2b..ec8e7d2 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -717,8 +717,8 @@ private static function get_node_managed_memberships( $node ) { * - missing_on_hub: Node has the membership but the hub does not → pull_to_hub. * - transfer: Node has it under old email, hub has it under new email → push_transfer. * - status_mismatch: Both have it with different statuses. - * Hub timestamp newer (or node timestamp unavailable) → push_to_node. - * Node timestamp newer → skip. + * Side with a subscription attached is authoritative. + * If neither has a subscription, hub wins by default. * * @param array $hub_lookup Hub memberships keyed by email::network_id. * @param array $node_memberships Raw node membership array (email, status, network_id). @@ -785,59 +785,22 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, continue; } - // Status mismatch: compare timestamps (GMT) to decide direction. - $hub_modified = $hub_item['post_modified'] ?? ''; - $node_modified = ''; + // Status mismatch: the side with a subscription attached is authoritative. + // A subscription (any status) is the source of truth for membership status. + $hub_has_sub = ! empty( $hub_item['has_subscription'] ); + $node_has_sub = ! empty( $node_item['has_subscription'] ); - if ( isset( $node_managed_lookup[ $key ] ) ) { - $node_modified = $node_managed_lookup[ $key ]['post_modified'] ?? ''; - } - - // Hub is authoritative when node timestamp is unavailable. - if ( empty( $node_modified ) ) { + if ( $hub_has_sub && ! $node_has_sub ) { $action = 'push_to_node'; + } elseif ( ! $hub_has_sub && $node_has_sub ) { + // Node has a subscription, hub does not – node is authoritative. + $action = 'pull_to_hub'; } else { - $hub_timestamp = self::parse_gmt_timestamp( $hub_modified ); - $node_timestamp = self::parse_gmt_timestamp( $node_modified ); - - if ( false !== $hub_timestamp && false !== $node_timestamp ) { - if ( $hub_timestamp >= $node_timestamp ) { - $action = 'push_to_node'; - } else { - // Node has fresher data – log and skip. - if ( defined( 'WP_CLI' ) && WP_CLI ) { - WP_CLI::warning( - sprintf( - 'Status mismatch for %s (plan %s): hub=%s (%s), node=%s (%s) – node is newer, skipping.', - $email, - $network_id, - $hub_status, - $hub_modified, - $node_status, - $node_modified - ) - ); - } - $action = 'skip'; - } - } else { - // Timestamp parse failure – default to hub as authoritative. - if ( defined( 'WP_CLI' ) && WP_CLI ) { - WP_CLI::warning( - sprintf( - 'Unable to parse timestamps for %s (plan %s): hub=%s, node=%s – defaulting to hub.', - $email, - $network_id, - $hub_modified, - $node_modified - ) - ); - } - $action = 'push_to_node'; - } + // Both or neither have a subscription – default to hub. + $action = 'push_to_node'; } - $discrepancies[] = [ + $discrepancy = [ 'email' => $email, 'network_id' => $network_id, 'type' => 'status_mismatch', @@ -845,6 +808,10 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, 'node_status' => $node_status, 'action' => $action, ]; + if ( 'pull_to_hub' === $action ) { + $discrepancy['node_data'] = $node_item; + } + $discrepancies[] = $discrepancy; } // Detect transfers: a missing_on_hub + missing_on_node pair for the same network_id diff --git a/tests/unit-tests/test-reconcile-memberships.php b/tests/unit-tests/test-reconcile-memberships.php index f6de5ea..8c19c58 100644 --- a/tests/unit-tests/test-reconcile-memberships.php +++ b/tests/unit-tests/test-reconcile-memberships.php @@ -119,40 +119,32 @@ public function test_missing_on_hub_results_in_pull_to_hub_action() { } /** - * When there is a status mismatch and the hub's post_modified is later than - * the node's, the hub is authoritative and the action is push_to_node. + * Status mismatch where hub has a subscription: hub is authoritative, push to node. */ - public function test_status_mismatch_hub_newer_results_in_push_to_node() { + public function test_status_mismatch_hub_has_subscription_results_in_push_to_node() { $classify_discrepancies_method = $this->get_classify_discrepancies_method(); $hub_lookup = [ 'dan@example.com::plan-d' => [ - 'email' => 'dan@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-d', - 'post_modified' => '2024-06-20 09:00:00', - 'membership_id' => 303, + 'email' => 'dan@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-d', + 'post_modified' => '2024-06-20 09:00:00', + 'membership_id' => 303, + 'has_subscription' => true, ], ]; $node_memberships = [ [ - 'email' => 'dan@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-d', + 'email' => 'dan@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-d', + 'has_subscription' => false, ], ]; - // Node managed lookup has an older timestamp than the hub. - $node_managed_lookup = [ - 'dan@example.com::plan-d' => [ - 'email' => 'dan@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-d', - 'post_modified' => '2024-06-10 08:00:00', - 'remote_id' => 303, - ], - ]; + $node_managed_lookup = []; $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); @@ -164,75 +156,68 @@ public function test_status_mismatch_hub_newer_results_in_push_to_node() { } /** - * When there is a status mismatch and the node's post_modified is later than - * the hub's, the node is treated as having fresher data and the action is skip. + * Status mismatch where node has a subscription but hub does not: + * node is authoritative, pull to hub. */ - public function test_status_mismatch_node_newer_results_in_skip() { + public function test_status_mismatch_node_has_subscription_results_in_pull_to_hub() { $classify_discrepancies_method = $this->get_classify_discrepancies_method(); $hub_lookup = [ 'eve@example.com::plan-e' => [ - 'email' => 'eve@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-e', - 'post_modified' => '2024-03-01 07:00:00', - 'membership_id' => 404, + 'email' => 'eve@example.com', + 'status' => 'wcm-expired', + 'network_id' => 'plan-e', + 'post_modified' => '2024-03-01 07:00:00', + 'membership_id' => 404, + 'has_subscription' => false, ], ]; $node_memberships = [ [ - 'email' => 'eve@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-e', + 'email' => 'eve@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-e', + 'has_subscription' => true, ], ]; - // Node managed lookup has a newer timestamp than the hub. - $node_managed_lookup = [ - 'eve@example.com::plan-e' => [ - 'email' => 'eve@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-e', - 'post_modified' => '2024-03-15 14:00:00', - 'remote_id' => 404, - ], - ]; + $node_managed_lookup = []; $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); $this->assertCount( 1, $discrepancies ); $this->assertEquals( 'status_mismatch', $discrepancies[0]['type'] ); - $this->assertEquals( 'skip', $discrepancies[0]['action'] ); + $this->assertEquals( 'pull_to_hub', $discrepancies[0]['action'] ); + $this->assertArrayHasKey( 'node_data', $discrepancies[0] ); } /** - * When there is a status mismatch but the membership is absent from the node - * managed lookup, the hub is treated as authoritative and action is push_to_node. + * Status mismatch where neither side has a subscription: hub wins by default. */ - public function test_status_mismatch_no_node_timestamp_defaults_to_hub_authoritative() { + public function test_status_mismatch_no_subscriptions_defaults_to_hub() { $classify_discrepancies_method = $this->get_classify_discrepancies_method(); $hub_lookup = [ 'frank@example.com::plan-f' => [ - 'email' => 'frank@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-f', - 'post_modified' => '2024-04-05 11:00:00', - 'membership_id' => 505, + 'email' => 'frank@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-f', + 'post_modified' => '2024-04-05 11:00:00', + 'membership_id' => 505, + 'has_subscription' => false, ], ]; $node_memberships = [ [ - 'email' => 'frank@example.com', - 'status' => 'wcm-paused', - 'network_id' => 'plan-f', + 'email' => 'frank@example.com', + 'status' => 'wcm-paused', + 'network_id' => 'plan-f', + 'has_subscription' => false, ], ]; - // Node has the membership but it is not in the managed lookup, so no - // timestamp is available for comparison. $node_managed_lookup = []; $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); @@ -251,60 +236,58 @@ public function test_mixed_discrepancies_returns_all_types() { $classify_discrepancies_method = $this->get_classify_discrepancies_method(); // Grace is on hub only → missing_on_node / push_to_node. - // Henry is on node only → missing_on_hub / skip. - // Iris has a status mismatch with hub newer → status_mismatch / push_to_node. + // Henry is on node only → missing_on_hub / pull_to_hub. + // Iris has a status mismatch, hub has subscription → status_mismatch / push_to_node. // Jane matches → no discrepancy. $hub_lookup = [ 'grace@example.com::plan-g' => [ - 'email' => 'grace@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-g', - 'post_modified' => '2024-05-01 10:00:00', - 'membership_id' => 601, + 'email' => 'grace@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-g', + 'post_modified' => '2024-05-01 10:00:00', + 'membership_id' => 601, + 'has_subscription' => true, ], 'iris@example.com::plan-i' => [ - 'email' => 'iris@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-i', - 'post_modified' => '2024-05-10 10:00:00', - 'membership_id' => 603, + 'email' => 'iris@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-i', + 'post_modified' => '2024-05-10 10:00:00', + 'membership_id' => 603, + 'has_subscription' => true, ], 'jane@example.com::plan-j' => [ - 'email' => 'jane@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-j', - 'post_modified' => '2024-05-12 10:00:00', - 'membership_id' => 604, + 'email' => 'jane@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-j', + 'post_modified' => '2024-05-12 10:00:00', + 'membership_id' => 604, + 'has_subscription' => true, ], ]; $node_memberships = [ [ - 'email' => 'henry@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-h', + 'email' => 'henry@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-h', + 'has_subscription' => false, ], [ - 'email' => 'iris@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-i', + 'email' => 'iris@example.com', + 'status' => 'wcm-cancelled', + 'network_id' => 'plan-i', + 'has_subscription' => false, ], [ - 'email' => 'jane@example.com', - 'status' => 'wcm-active', - 'network_id' => 'plan-j', + 'email' => 'jane@example.com', + 'status' => 'wcm-active', + 'network_id' => 'plan-j', + 'has_subscription' => false, ], ]; - $node_managed_lookup = [ - 'iris@example.com::plan-i' => [ - 'email' => 'iris@example.com', - 'status' => 'wcm-cancelled', - 'network_id' => 'plan-i', - 'post_modified' => '2024-05-08 08:00:00', - 'remote_id' => 603, - ], - ]; + $node_managed_lookup = []; $discrepancies = $classify_discrepancies_method->invoke( null, $hub_lookup, $node_memberships, $node_managed_lookup ); From 794fbd4b2e6d2da3973907d50518ba4973d7526e Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Tue, 7 Apr 2026 12:30:13 +0200 Subject: [PATCH 20/20] feat(integrity-check): detect circular managed membership links --- includes/cli/class-integrity-check.php | 110 ++++++++++++++++++ .../node/class-integrity-check-endpoints.php | 19 +-- 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/includes/cli/class-integrity-check.php b/includes/cli/class-integrity-check.php index ec8e7d2..7874e2d 100644 --- a/includes/cli/class-integrity-check.php +++ b/includes/cli/class-integrity-check.php @@ -175,6 +175,62 @@ public static function integrity_check( $args, $assoc_args ) { // phpcs:ignore G WP_CLI\Utils\format_items( 'table', $table_data, $node_columns ); } + // Circular link detection: managed memberships that have a local subscription + // are incorrectly marked as managed – they should be the source. + $circular_links = []; + + // Check hub. + $hub_circulars = self::get_local_circular_links(); + if ( ! empty( $hub_circulars ) ) { + $circular_links[ get_option( 'siteurl' ) ] = $hub_circulars; + } + + // Check nodes. + foreach ( $nodes as $node ) { + $node_managed = self::get_node_managed_memberships( $node ); + if ( null === $node_managed ) { + continue; + } + $node_circulars = []; + foreach ( $node_managed as $item ) { + if ( ! empty( $item['has_subscription'] ) ) { + $node_circulars[] = $item; + } + } + if ( ! empty( $node_circulars ) ) { + $circular_links[ $node->get_url() ] = $node_circulars; + } + } + + if ( ! empty( $circular_links ) ) { + WP_CLI::line( '' ); + $total_circular = array_sum( array_map( 'count', $circular_links ) ); + WP_CLI::warning( sprintf( 'Found %d circular links (managed memberships with local subscriptions):', $total_circular ) ); + foreach ( $circular_links as $site_url => $items ) { + WP_CLI::line( sprintf( ' %s: %d', $site_url, count( $items ) ) ); + if ( $verbose ) { + foreach ( array_slice( $items, 0, 5 ) as $item ) { + WP_CLI::line( + sprintf( + ' %s (%s) #%d – has subscription but marked as managed, pointing to %s #%d', + $item['email'], + $item['network_id'], + $item['membership_id'], + $item['remote_site_url'] ?? '?', + $item['remote_id'] ?? 0 + ) + ); + } + if ( count( $items ) > 5 ) { + WP_CLI::line( sprintf( ' ... and %d more', count( $items ) - 5 ) ); + } + } + } + WP_CLI::line( '' ); + WP_CLI::line( 'These memberships are sources (they have a local subscription) but are incorrectly marked as network-managed.' ); + WP_CLI::line( 'To fix: remove _managed_by_newspack_network, _remote_id, and _remote_site_url meta from these memberships.' ); + } + if ( $fix ) { WP_CLI::line( '' ); @@ -902,6 +958,60 @@ private static function classify_discrepancies( $hub_lookup, $node_memberships, * Get the latest pullable event log ID from the hub. * * Only considers event types that nodes actually pull (ACTIONS_THAT_NODES_PULL), + * Find managed memberships on the local site that have a subscription. + * These are circular links: the membership is the source but incorrectly marked as managed. + * + * @return array Array of membership data with circular link issues. + */ + private static function get_local_circular_links() { + global $wpdb; + + $managed_key = \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_MANAGED_META_KEY; + $remote_id_key = \Newspack_Network\Woocommerce_Memberships\Admin::REMOTE_ID_META_KEY; + $site_url_key = \Newspack_Network\Woocommerce_Memberships\Admin::SITE_URL_META_KEY; + $network_key = \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY; + + // phpcs:disable WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching + $results = $wpdb->get_results( + $wpdb->prepare( + "SELECT p.ID as membership_id, LOWER(u.user_email) as email, + pm_network.meta_value as network_id, + pm_remote.meta_value as remote_id, + pm_site.meta_value as remote_site_url + FROM {$wpdb->posts} p + INNER JOIN {$wpdb->users} u ON p.post_author = u.ID + INNER JOIN {$wpdb->postmeta} pm_managed ON p.ID = pm_managed.post_id AND pm_managed.meta_key = %s + INNER JOIN {$wpdb->postmeta} pm_sub ON p.ID = pm_sub.post_id AND pm_sub.meta_key = '_subscription_id' AND pm_sub.meta_value != '' + LEFT JOIN {$wpdb->postmeta} pm_remote ON p.ID = pm_remote.post_id AND pm_remote.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_site ON p.ID = pm_site.post_id AND pm_site.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s + WHERE p.post_type = 'wc_user_membership' AND p.post_status != 'trash'", + $managed_key, + $remote_id_key, + $site_url_key, + $network_key + ) + ); + // phpcs:enable WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users + + $circulars = []; + foreach ( $results as $row ) { + $circulars[] = [ + 'email' => $row->email, + 'network_id' => $row->network_id ?? '', + 'membership_id' => (int) $row->membership_id, + 'remote_id' => (int) $row->remote_id, + 'remote_site_url' => $row->remote_site_url ?? '', + 'has_subscription' => true, + ]; + } + + return $circulars; + } + + /** + * Get the latest pullable event ID from the hub event log, * avoiding false positives from non-pullable events like order_changed. * * @return int The latest event ID, or 0 if the log is empty. diff --git a/includes/node/class-integrity-check-endpoints.php b/includes/node/class-integrity-check-endpoints.php index 4c63320..70c085d 100644 --- a/includes/node/class-integrity-check-endpoints.php +++ b/includes/node/class-integrity-check-endpoints.php @@ -189,13 +189,15 @@ public static function handle_managed_memberships_request( $request ) { u.user_email, pm_remote.meta_value as remote_id, pm_site.meta_value as remote_site_url, - pm_network.meta_value as network_id + pm_network.meta_value as network_id, + CASE WHEN pm_sub.meta_value IS NOT NULL AND pm_sub.meta_value != '' THEN 1 ELSE 0 END as has_subscription FROM {$wpdb->posts} p INNER JOIN {$wpdb->users} u ON p.post_author = u.ID INNER JOIN {$wpdb->postmeta} pm_managed ON p.ID = pm_managed.post_id AND pm_managed.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_remote ON p.ID = pm_remote.post_id AND pm_remote.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_site ON p.ID = pm_site.post_id AND pm_site.meta_key = %s LEFT JOIN {$wpdb->postmeta} pm_network ON p.post_parent = pm_network.post_id AND pm_network.meta_key = %s + LEFT JOIN {$wpdb->postmeta} pm_sub ON p.ID = pm_sub.post_id AND pm_sub.meta_key = '_subscription_id' WHERE p.post_type = 'wc_user_membership' AND p.post_status != 'trash'", \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_MANAGED_META_KEY, @@ -208,13 +210,14 @@ public static function handle_managed_memberships_request( $request ) { $memberships = []; foreach ( $results as $row ) { $memberships[] = [ - 'email' => strtolower( $row->user_email ), - 'status' => $row->post_status, - 'network_id' => $row->network_id ?? '', - 'remote_id' => (int) $row->remote_id, - 'remote_site_url' => $row->remote_site_url ?? '', - 'post_modified' => $row->post_modified, - 'membership_id' => (int) $row->ID, + 'email' => strtolower( $row->user_email ), + 'status' => $row->post_status, + 'network_id' => $row->network_id ?? '', + 'remote_id' => (int) $row->remote_id, + 'remote_site_url' => $row->remote_site_url ?? '', + 'post_modified' => $row->post_modified, + 'membership_id' => (int) $row->ID, + 'has_subscription' => (bool) $row->has_subscription, ]; }