Skip to content

feat(integrity-check): add --fix flag to reconcile membership discrepancies#307

Open
adekbadek wants to merge 20 commits intotrunkfrom
feat/reconcile-memberships-cli
Open

feat(integrity-check): add --fix flag to reconcile membership discrepancies#307
adekbadek wants to merge 20 commits intotrunkfrom
feat/reconcile-memberships-cli

Conversation

@adekbadek
Copy link
Copy Markdown
Member

@adekbadek adekbadek commented Mar 25, 2026

All Submissions:

Changes proposed in this Pull Request:

Adds a --fix flag to the existing wp newspack-network integrity-check command. When passed, the command goes beyond reporting discrepancies and actively reconciles them by dispatching membership update events.

Addresses NPPM-386.

How it works:

  1. The integrity check runs as usual, finding discrepancies between hub and nodes.
  2. With --fix, each discrepancy is classified and an action is determined:
    • Missing on node (hub has it, node doesn't): dispatches a membership update event so the node creates the membership on its next sync.
    • Missing on hub (node has it, hub doesn't): dispatches a membership update event so the hub creates the membership on its next sync.
    • Transfer (node has it under old email, hub has it under new email): detected via remote_id cross-referencing. The node's managed membership _remote_id is matched to the hub's membership_id. Dispatches a transfer event with previous_email so the node reassigns the membership using the transfer handling from feat(memberships): improve integrity check and handle ownership transfers #306.
    • Status mismatch: compares post_modified_gmt timestamps (parsed via strtotime()) to determine which side has fresher data. The newer side wins -- the hub is not blindly trusted as the source of truth.
  3. A table of planned actions is displayed, then events are dispatched with a progress bar.

Sync lag safety (5a48dba, cda0e25):

  • Before dispatching events, --fix queries each node's sync status via a new /integrity-check/sync-status REST endpoint.
  • If a node has unprocessed events in its queue, the command aborts and suggests running wp newspack-network sync-all on the node first -- discrepancies may resolve after syncing without creating new events.
  • Use --force to skip the sync lag check.
  • If managed-memberships data cannot be fetched from a node, reconciliation for that node is skipped entirely (prevents blind overwrites on API failure).

Review feedback (cda0e25):

  • All cross-site timestamp comparisons now use post_modified_gmt (UTC) instead of post_modified (local timezone), both in the hub's utility queries and the node's managed-memberships endpoint.
  • Timestamps are compared as integers via strtotime() instead of string comparison; parse failures are handled explicitly.
  • Event dispatch uses the hub membership's modification time instead of time(), making dispatch idempotent.
  • Hardcoded meta key strings in the managed-memberships endpoint replaced with Woocommerce_Memberships\Admin:: constants.

Supporting changes:

  • Integrity_Check_Utils now includes post_modified_gmt and membership_id in membership data queries (hashing is unchanged).
  • A new /integrity-check/managed-memberships REST endpoint on nodes returns managed membership details including post_modified_gmt and _remote_id, enabling both timestamp comparison and transfer detection.
  • A new /integrity-check/sync-status REST endpoint on nodes returns the last_processed_id for sync lag detection.

How to test the changes in this Pull Request:

  1. Set up a Hub + Node network with WooCommerce Memberships and at least one membership plan with a _newspack_network_id.
  2. Run wp newspack-network integrity-check --verbose on the hub to confirm existing discrepancies are reported as before.
  3. Run wp newspack-network integrity-check --fix --verbose on the hub.
  4. Verify the output shows a reconciliation section with an action table per node (columns: email, network_id, type, hub_status, node_status, action) and a summary of dispatched vs skipped actions.
  5. Check the hub's event log for newly created newspack_network_woo_membership_updated events.
  6. Verify nodes pick up the events on their next pull cycle.
  7. For transfer testing: transfer a membership from User A to User B on the hub, run --fix, and verify the action is push_transfer with the correct previous_email. Confirm the node reassigns the membership to User B.
  8. For status mismatch testing: manually set a membership to a different status on a node with a newer post_modified, run --fix, and verify the action is skip (node is newer).
  9. For sync lag testing: ensure a node has unprocessed events, run --fix, and verify it aborts with a suggestion to sync first. Run with --force to confirm it proceeds anyway.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

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.
@adekbadek adekbadek force-pushed the feat/reconcile-memberships-cli branch from 3258feb to db2a9fe Compare March 25, 2026 22:06
@adekbadek adekbadek changed the base branch from trunk to enhance/membership-sync-improvements March 25, 2026 22:06
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Hub-side wp newspack-network integrity-check command with a --fix mode that attempts to reconcile detected Woo Membership discrepancies by dispatching newspack_network_woo_membership_updated events, supported by additional membership fields and a new node REST endpoint to expose managed-membership timestamps for recency comparison.

Changes:

  • Adds --fix flag to the integrity-check WP-CLI command, including discrepancy classification and event dispatch.
  • Expands integrity-check membership query results to include post_modified and membership_id.
  • Adds a node REST endpoint to return network-managed membership details (including modification timestamps).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
includes/cli/class-integrity-check.php Adds --fix reconciliation flow, discrepancy classification, and membership update event dispatch.
includes/class-integrity-check-utils.php Extends membership query output with timestamp and membership ID fields used by reconciliation.
includes/node/class-integrity-check-endpoints.php Adds /integrity-check/managed-memberships endpoint to expose managed membership details for reconciliation.
tests/unit-tests/test-reconcile-memberships.php Adds unit tests covering discrepancy classification behavior used by --fix.
Comments suppressed due to low confidence (1)

includes/class-integrity-check-utils.php:109

  • The docblocks for execute_membership_query() / get_membership_data() still say they return only (email, status, network_id), but the returned arrays now also include post_modified and membership_id. Update the PHPDoc so downstream callers/tests know the shape of the data.
	/**
	 * Execute membership query and format results
	 *
	 * @param string   $query The SQL query.
	 * @param array    $prepare_args Arguments for wpdb->prepare.
	 * @param int|null $max_records Maximum number of records to return.
	 * @return array Array of (email, status, network_id) data
	 */
	private static function execute_membership_query( $query, $prepare_args, $max_records = null ) {
		global $wpdb;

		if ( $max_records ) {
			$query .= $wpdb->prepare( ' LIMIT %d', $max_records );
		}

		// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared
		$results = $wpdb->get_results( $wpdb->prepare( $query, ...$prepare_args ) );

		$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,
			];
		}

		return $membership_data;
	}

	/**
	 * Get all membership data
	 *
	 * @param int|null $max_records Maximum number of records to return (for testing).
	 * @return array Array of (email, status, network_id) data
	 */
	public static function get_membership_data( $max_records = null ) {
		if ( ! class_exists( 'WC_Memberships_User_Membership' ) ) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread includes/cli/class-integrity-check.php Outdated
Comment thread includes/cli/class-integrity-check.php Outdated
Comment thread includes/class-integrity-check-utils.php Outdated
Comment thread includes/node/class-integrity-check-endpoints.php Outdated
Comment thread tests/unit-tests/test-reconcile-memberships.php Outdated
Comment thread includes/node/class-integrity-check-endpoints.php Outdated
Comment thread includes/node/class-integrity-check-endpoints.php Outdated
Comment thread includes/cli/class-integrity-check.php
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit-tests/test-reconcile-memberships.php
Comment thread includes/cli/class-integrity-check.php Outdated
Comment thread includes/cli/class-integrity-check.php
Comment thread includes/cli/class-integrity-check.php
Base automatically changed from enhance/membership-sync-improvements to trunk April 9, 2026 07:24
@adekbadek adekbadek marked this pull request as ready for review April 9, 2026 07:26
@adekbadek adekbadek requested a review from a team as a code owner April 9, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants