Assorted changes 2

includes/class-users.php

@@ -7,6 +7,8 @@
 
 namespace Newspack_Network;
 
+use const Newspack_Network\constants\EVENT_LOG_PAGE_SLUG;
+
 /**
  * Class to handle the Users admin page
  */
@@ -75,7 +77,7 @@ public static function manage_users_custom_column( $value, $column_name, $user_i
 			$summary       = $last_activity->get_summary();
 			$event_log_url = add_query_arg(
 				[
-					'page'  => \Newspack_Network\Hub\Admin\Event_Log::PAGE_SLUG,
+					'page'  => EVENT_LOG_PAGE_SLUG,
 					'email' => $user->user_email,
 				],
 				admin_url( 'admin.php' )

includes/cli/class-membership-dedupe.php

@@ -39,7 +39,7 @@ public static function register_commands() {
 						[
 							'type'     => 'assoc',
 							'name'     => 'plan-id',
-							'optional' => false,
+							'optional' => true,
 						],
 						[
 							'type'     => 'flag',
@@ -64,62 +64,97 @@ public static function register_commands() {
 	 *
 	 *     wp newspack-network clean-up-duplicate-memberships --plan-id=1234
 	 *
+	 * ## OPTIONS
+	 *
+	 * [--plan-id]
+	 * : Membership plan ID to clean up. If not set, all synchronized plans will be cleaned up.
+	 *
+	 * [--live]
+	 * : Run the command in live mode, updating the users.
+	 *
+	 * [--csv]
+	 * : Output CSV.
+	 *
 	 * @param array $args Positional args.
 	 * @param array $assoc_args Associative args and flags.
 	 */
 	public static function clean_duplicate_memberships( $args, $assoc_args ) {
 		WP_CLI::line( '' );
-		$live = isset( $assoc_args['live'] );
-		$csv = isset( $assoc_args['csv'] );
-
-		$plan_id = $assoc_args['plan-id'];
-		if ( ! is_numeric( $plan_id ) ) {
-			WP_CLI::error( 'Membership plan ID must be numeric' );
-		}
-		$plan_id = (int) $plan_id;
 
+		$live = isset( $assoc_args['live'] );
 		if ( ! $live ) {
 			WP_CLI::line( 'Running in dry-run mode. Use --live flag to run in live mode.' );
 			WP_CLI::line( '' );
 		}
 
-		$user_ids = self::get_users_with_duplicate_membership( $plan_id );
-		WP_CLI::line( sprintf( '%d users found with duplicate memberships', count( $user_ids ) ) );
+		$csv = isset( $assoc_args['csv'] );
 
-		$duplicates = [];
-		foreach ( $user_ids as $user_id ) {
-			$memberships = get_posts(
+		$plan_id_from_args = isset( $assoc_args['plan-id'] ) ? $assoc_args['plan-id'] : null;
+		$plan_ids = [];
+		if ( $plan_id_from_args ) {
+			if ( ! is_numeric( $plan_id_from_args ) ) {
+				WP_CLI::error( 'Membership plan ID must be numeric' );
+			}
+			$plan_ids = [ (int) $plan_id_from_args ];
+		} else {
+			// Get all network-sync'd membership plans.
+			$plan_ids = get_posts(
 				[
-					'author'      => $user_id,
-					'post_type'   => 'wc_user_membership',
-					'post_status' => 'any',
-					'post_parent' => $plan_id,
+					'post_type'  => 'wc_membership_plan',
+					'fields'     => 'ids',
+					'meta_query' => [ // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query
+						[
+							'key'     => \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY,
+							'compare' => 'EXISTS',
+						],
+					],
 				]
 			);
+		}
 
-			foreach ( $memberships as $membership ) {
-				$user = get_user_by( 'id', $membership->post_author );
-				$duplicates[] = [
-					'user'         => $membership->post_author,
-					'email'        => $user->user_email,
-					'membership'   => $membership->ID,
-					'subscription' => get_post_meta( $membership->ID, '_subscription_id', true ),
-					'status'       => $membership->post_status,
-					'remote'       => get_post_meta( $membership->ID, '_remote_site_url', true ),
-				];
+		$user_ids = [];
+		foreach ( $plan_ids as $plan_id ) {
+			WP_CLI::line( sprintf( 'Checking plan #%d', $plan_id ) );
+
+			$user_ids = array_merge( $user_ids, self::get_users_with_duplicate_membership( $plan_id ) );
+			WP_CLI::line( sprintf( '%d users found with duplicate memberships', count( $user_ids ) ) );
+
+			$duplicates = [];
+			foreach ( $user_ids as $user_id ) {
+				$memberships = get_posts(
+					[
+						'author'      => $user_id,
+						'post_type'   => 'wc_user_membership',
+						'post_status' => 'any',
+						'post_parent' => $plan_id,
+					]
+				);
+
+				foreach ( $memberships as $membership ) {
+					$user = get_user_by( 'id', $membership->post_author );
+					if ( $user === false ) {
+						continue;
+					}
+					$duplicates[] = [
+						'user'         => $membership->post_author,
+						'email'        => $user->user_email,
+						'membership'   => $membership->ID,
+						'subscription' => get_post_meta( $membership->ID, '_subscription_id', true ),
+						'status'       => $membership->post_status,
+						'remote'       => get_post_meta( $membership->ID, '_remote_site_url', true ),
+					];
+				}
 			}
-		}
 
-		if ( $csv && ! empty( $duplicates ) ) {
-			WP_CLI::line( 'COPY AND PASTE THIS CSV: ' );
-			WP_CLI::line();
-			WP_CLI\Utils\format_items( 'csv', $duplicates, array_keys( $duplicates[0] ) );
-			WP_CLI::line();
-		}
+			if ( $csv && ! empty( $duplicates ) ) {
+				WP_CLI::line( 'COPY AND PASTE THIS CSV: ' );
+				WP_CLI::line();
+				WP_CLI\Utils\format_items( 'csv', $duplicates, array_keys( $duplicates[0] ) );
+				WP_CLI::line();
+			}
 
-		if ( $live ) {
-			WP_CLI::line( 'Deleting duplicates' );
-			self::deduplicate_memberships( $duplicates );
+			self::deduplicate_memberships( $duplicates, $live );
+			WP_CLI::line( '' );
 		}
 
 		WP_CLI::success( 'Done' );
@@ -151,8 +186,12 @@ private static function get_users_with_duplicate_membership( $plan_id ) {
 	 * De-duplicate memberships so that users only have one membership of a plan.
 	 *
 	 * @param array $duplicates Analyzed data from ::clean_duplicate_memberships.
+	 * @param bool  $live Whether to actually delete the duplicates.
 	 */
-	private static function deduplicate_memberships( $duplicates ) {
+	private static function deduplicate_memberships( $duplicates, $live ) {
+		if ( $live ) {
+			WP_CLI::line( 'Deleting duplicates' );
+		}
 		$userdata = [];
 
 		foreach ( $duplicates as $duplicate ) {
@@ -166,13 +205,17 @@ private static function deduplicate_memberships( $duplicates ) {
 		foreach ( $userdata as $email => $duplicates ) {
 			WP_CLI::line( sprintf( 'Processing %s', $email ) );
 			if ( count( $duplicates ) < 2 ) {
-				WP_CLI::line( '  - User does not have too many memberships' );
+				WP_CLI::line( '  - User has multiple memberships, but no duplicates' );
 			}
 
 			$memberships_to_delete = array_slice( $duplicates, 1 );
 			foreach ( $memberships_to_delete as $duplicate ) {
-				wp_delete_post( $duplicate['membership'], true );
-				WP_CLI::line( sprintf( '  - Deleted extra membership %d', $duplicate['membership'] ) );
+				if ( $live ) {
+					wp_delete_post( $duplicate['membership'], true );
+					WP_CLI::line( sprintf( '  - Deleted extra membership %d', $duplicate['membership'] ) );
+				} else {
+					WP_CLI::line( sprintf( '  - Would have deleted extra membership %d', $duplicate['membership'] ) );
+				}
 			}
 		}
 	}

includes/constants.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * Newspack Network related constants
- * 
+ *
  * @package Newspack
  */
 
@@ -15,3 +15,5 @@
 	'INVALID_SIGNATURE' => 'Invalid Signature.',
 	'INVALID_DATA'      => 'Bad request. Invalid Data.',
 ];
+
+const EVENT_LOG_PAGE_SLUG = 'newspack-network-event-log';

includes/hub/admin/class-event-log.php

@@ -8,14 +8,13 @@
 namespace Newspack_Network\Hub\Admin;
 
 use Newspack_Network\Admin as Network_Admin;
+use const Newspack_Network\constants\EVENT_LOG_PAGE_SLUG;
 
 /**
  * Class to handle the Event log admin page
  */
 class Event_Log {
 
-	const PAGE_SLUG = 'newspack-network-event-log';
-
 	/**
 	 * Runs the initialization.
 	 */
@@ -30,7 +29,7 @@ public static function init() {
 	 * @return void
 	 */
 	public static function add_admin_menu() {
-		Network_Admin::add_submenu_page( __( 'Event Log', 'newspack-network' ), self::PAGE_SLUG, [ __CLASS__, 'render_page' ] );
+		Network_Admin::add_submenu_page( __( 'Event Log', 'newspack-network' ), EVENT_LOG_PAGE_SLUG, [ __CLASS__, 'render_page' ] );
 	}
 
 	/**
@@ -39,7 +38,7 @@ public static function add_admin_menu() {
 	 * @return void
 	 */
 	public static function admin_enqueue_scripts() {
-		$page_slug = Network_Admin::PAGE_SLUG . '_page_' . self::PAGE_SLUG;
+		$page_slug = Network_Admin::PAGE_SLUG . '_page_' . EVENT_LOG_PAGE_SLUG;
 		if ( get_current_screen()->id !== $page_slug ) {
 			return;
 		}
@@ -62,7 +61,7 @@ public static function render_page() {
 
 		echo '<div class="wrap"><h2>', esc_html( __( 'Event Log', 'newspack-network' ) ), '</h2>';
 		echo '<form method="get">';
-		echo '<input type="hidden" name="page" value="' . esc_attr( self::PAGE_SLUG ) . '" />';
+		echo '<input type="hidden" name="page" value="' . esc_attr( EVENT_LOG_PAGE_SLUG ) . '" />';
 
 		$table->prepare_items();
 

includes/hub/class-pull-endpoint.php

@@ -61,42 +61,22 @@ public static function get_pull_limit() {
 	 * @return WP_REST_Response
 	 */
 	public static function handle_pull( $request ) {
+		$request_error = \Newspack_Network\Utils\Requests::get_request_to_hub_errors( $request );
+		if ( \is_wp_error( $request_error ) ) {
+			return new WP_REST_Response( [ 'error' => $request_error->get_error_message() ], 403 );
+		}
+
 		$site              = $request['site'];
 		$last_processed_id = $request['last_processed_id'];
 		$actions           = $request['actions'];
-		$signature         = $request['signature'];
-		$nonce             = $request['nonce'];
 
-		Debugger::log( 'Pull request received' );
-		Debugger::log( $site );
-		Debugger::log( $last_processed_id );
-		Debugger::log( $actions );
+		Debugger::log( sprintf( 'Pull request received from site %s, with last processed ID %d, for actions: %s.', $site, $last_processed_id, implode( ', ', $actions ) ) );
 
-		if ( empty( $site ) ||
-			empty( $actions ) ||
-			empty( $nonce ) ||
-			empty( $signature )
-		) {
+		if ( empty( $actions ) ) {
 			return new WP_REST_Response( array( 'error' => 'Bad request.' ), 400 );
 		}
 
 		$node = Nodes::get_node_by_url( $site );
-
-		if ( ! $node ) {
-			Debugger::log( 'Node not found.' );
-			return new WP_REST_Response( array( 'error' => 'Bad request. Site not registered in this Hub.' ), 403 );
-		}
-
-		$verified         = $node->decrypt_message( $signature, $nonce );
-		$verified_message = json_decode( $verified );
-
-		if ( ! $verified || ! is_object( $verified_message ) || (int) $last_processed_id !== (int) $verified_message->last_processed_id ) {
-			Debugger::log( 'Signature check failed' );
-			return new WP_REST_Response( array( 'error' => 'INVALID_SIGNATURE' ), 403 );
-		}
-
-		Debugger::log( 'Successfully verified request' );
-
 		$query_args = [
 			'excluded_node_id' => $node->get_id(),
 			'id_greater_than'  => $last_processed_id,

includes/hub/database/class-orders.php

@@ -12,7 +12,7 @@
 use Newspack_Network\Debugger;
 
 /**
- * Class to handle the ubscriptions post type registration
+ * Class to handle the Orders post type registration
  */
 class Orders {
 

includes/incoming-events/class-reader-registered.php

@@ -50,6 +50,15 @@ public function maybe_create_user() {
 
 		User_Update_Watcher::$enabled = false;
 
-		$user = User_Utils::get_or_create_user_by_email( $email, $this->get_site(), $this->data->user_id ?? '' );
+		// If a user exists, but has a non-synchronizable role, add a synchronizable role.
+		$existing_user = get_user_by( 'email', $email );
+		if ( $existing_user ) {
+			$synced_roles = \Newspack_Network\Utils\Users::get_synced_user_roles();
+			if ( ! array_intersect( $existing_user->roles, $synced_roles ) ) {
+				$existing_user->add_role( $synced_roles[0] );
+			}
+		} else {
+			$user = User_Utils::get_or_create_user_by_email( $email, $this->get_site(), $this->data->user_id ?? '', (array) $this->data );
+		}
 	}
 }

includes/node/class-pulling.php

@@ -132,49 +132,17 @@ public static function set_last_processed_id( $id ) {
 	}
 
 	/**
-	 * Gets the request parameters for the pull request
+	 * Makes a request to the Hub to pull data
 	 *
-	 * @return array
+	 * @return array|\WP_Error
 	 */
-	public static function get_request_params() {
+	public static function make_request() {
 		$params = [
 			'last_processed_id' => self::get_last_processed_id(),
 			'actions'           => Accepted_Actions::ACTIONS_THAT_NODES_PULL,
 			'site'              => get_bloginfo( 'url' ),
 		];
-		return self::sign_params( $params );
-	}
-
-	/**
-	 * Signs the request parameters with the Node's secret key
-	 *
-	 * @param array $params The request parameters.
-	 * @return array The params array with an additional signature key.
-	 */
-	public static function sign_params( $params ) {
-		$message             = wp_json_encode( $params );
-		$secret_key          = Settings::get_secret_key();
-		$nonce               = Crypto::generate_nonce();
-		$signature           = Crypto::encrypt_message( $message, $secret_key, $nonce );
-		$params['signature'] = $signature;
-		$params['nonce']     = $nonce;
-		return $params;
-	}
-
-	/**
-	 * Makes a request to the Hub to pull data
-	 *
-	 * @return array|\WP_Error
-	 */
-	public static function make_request() {
-		$url      = trailingslashit( Settings::get_hub_url() ) . 'wp-json/newspack-network/v1/pull';
-		$params   = self::get_request_params();
-		$response = wp_remote_post(
-			$url,
-			[
-				'body' => $params,
-			]
-		);
+		$response = \Newspack_Network\Utils\Requests::request_to_hub( 'wp-json/newspack-network/v1/pull', $params );
 		if ( is_wp_error( $response ) ) {
 			return $response;
 		}