Re-assign existing webhooks during user deletion (#37814)
Co-authored-by: James Collins <james@om4.com.au> Co-authored-by: Nestor Soriano <konamiman@konamiman.com> Includes UI detailing how many webhooks are assigned to each of the users being deleted, and explaining how the re-assignment works.
This commit is contained in:
parent
4b3479595b
commit
8dc16fa48d
|
@ -0,0 +1,4 @@
|
|||
Significance: minor
|
||||
Type: enhancement
|
||||
|
||||
When deleting an administrator user, any existing webhook(s) owned to the user being deleted are re-assigned to the nominated user if the "Attribute all content to" option is chosen, or re-assigned to user id zero. This helps avoid `woocommerce_rest_cannot_view` webhook payload errors.
|
|
@ -18,6 +18,7 @@ use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore;
|
|||
use Automattic\WooCommerce\Internal\ProductDownloads\ApprovedDirectories\Register as ProductDownloadDirectories;
|
||||
use Automattic\WooCommerce\Internal\RestockRefundedItemsAdjuster;
|
||||
use Automattic\WooCommerce\Internal\Settings\OptionSanitizer;
|
||||
use Automattic\WooCommerce\Internal\Utilities\WebhookUtil;
|
||||
use Automattic\WooCommerce\Proxies\LegacyProxy;
|
||||
|
||||
/**
|
||||
|
@ -240,6 +241,7 @@ final class WooCommerce {
|
|||
$container->get( OptionSanitizer::class );
|
||||
$container->get( BatchProcessingController::class );
|
||||
$container->get( FeaturesController::class );
|
||||
$container->get( WebhookUtil::class );
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -282,6 +282,7 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface {
|
|||
$exclude = '';
|
||||
$date_created = '';
|
||||
$date_modified = '';
|
||||
$user_id = '';
|
||||
|
||||
if ( ! empty( $args['include'] ) ) {
|
||||
$args['include'] = implode( ',', wp_parse_id_list( $args['include'] ) );
|
||||
|
@ -293,6 +294,10 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface {
|
|||
$exclude = 'AND webhook_id NOT IN (' . $args['exclude'] . ')';
|
||||
}
|
||||
|
||||
if ( ! empty( $args['user_id'] ) ) {
|
||||
$user_id = $wpdb->prepare( 'AND `user_id` = %d', absint( $args['user_id'] ) );
|
||||
}
|
||||
|
||||
if ( ! empty( $args['after'] ) || ! empty( $args['before'] ) ) {
|
||||
$args['after'] = empty( $args['after'] ) ? '0000-00-00' : $args['after'];
|
||||
$args['before'] = empty( $args['before'] ) ? current_time( 'mysql', 1 ) : $args['before'];
|
||||
|
@ -326,6 +331,7 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface {
|
|||
{$exclude}
|
||||
{$date_created}
|
||||
{$date_modified}
|
||||
{$user_id}
|
||||
{$order}
|
||||
{$limit}
|
||||
{$offset}"
|
||||
|
@ -349,6 +355,7 @@ class WC_Webhook_Data_Store implements WC_Webhook_Data_Store_Interface {
|
|||
{$exclude}
|
||||
{$date_created}
|
||||
{$date_modified}
|
||||
{$user_id}
|
||||
{$order}
|
||||
{$limit}
|
||||
{$offset}"
|
||||
|
|
|
@ -11,6 +11,7 @@ use Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider
|
|||
use Automattic\WooCommerce\Internal\Utilities\COTMigrationUtil;
|
||||
use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil;
|
||||
use Automattic\WooCommerce\Internal\Utilities\HtmlSanitizer;
|
||||
use Automattic\WooCommerce\Internal\Utilities\WebhookUtil;
|
||||
use Automattic\WooCommerce\Proxies\LegacyProxy;
|
||||
use Automattic\WooCommerce\Utilities\PluginUtil;
|
||||
use Automattic\WooCommerce\Utilities\OrderUtil;
|
||||
|
@ -31,6 +32,7 @@ class UtilsClassesServiceProvider extends AbstractServiceProvider {
|
|||
OrderUtil::class,
|
||||
PluginUtil::class,
|
||||
COTMigrationUtil::class,
|
||||
WebhookUtil::class,
|
||||
);
|
||||
|
||||
/**
|
||||
|
@ -44,5 +46,6 @@ class UtilsClassesServiceProvider extends AbstractServiceProvider {
|
|||
->addArgument( LegacyProxy::class );
|
||||
$this->share( COTMigrationUtil::class )
|
||||
->addArguments( array( CustomOrdersTableController::class, DataSynchronizer::class ) );
|
||||
$this->share( WebhookUtil::class );
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,136 @@
|
|||
<?php
|
||||
/**
|
||||
* WebhookUtil class file.
|
||||
*/
|
||||
|
||||
namespace Automattic\WooCommerce\Internal\Utilities;
|
||||
|
||||
use Automattic\WooCommerce\Internal\Traits\AccessiblePrivateMethods;
|
||||
|
||||
/**
|
||||
* Class with utility methods for dealing with webhooks.
|
||||
*/
|
||||
class WebhookUtil {
|
||||
|
||||
use AccessiblePrivateMethods;
|
||||
|
||||
/**
|
||||
* Creates a new instance of the class.
|
||||
*/
|
||||
public function __construct() {
|
||||
self::add_action( 'deleted_user', array( $this, 'reassign_webhooks_to_new_user_id' ), 10, 2 );
|
||||
self::add_action( 'delete_user_form', array( $this, 'maybe_render_user_with_webhooks_warning' ), 10, 2 );
|
||||
}
|
||||
|
||||
/**
|
||||
* Whenever a user is deleted, re-assign their webhooks to the new user.
|
||||
*
|
||||
* If re-assignment isn't selected during deletion, assign the webhooks to user_id 0,
|
||||
* so that an admin can edit and re-save them in order to get them to be assigned to a valid user.
|
||||
*
|
||||
* @param int $old_user_id ID of the deleted user.
|
||||
* @param int|null $new_user_id ID of the user to reassign existing data to, or null if no re-assignment is requested.
|
||||
*
|
||||
* @return void
|
||||
* @since 7.8.0
|
||||
*/
|
||||
private function reassign_webhooks_to_new_user_id( int $old_user_id, ?int $new_user_id ): void {
|
||||
$webhook_ids = $this->get_webhook_ids_for_user( $old_user_id );
|
||||
|
||||
foreach ( $webhook_ids as $webhook_id ) {
|
||||
$webhook = new \WC_Webhook( $webhook_id );
|
||||
$webhook->set_user_id( $new_user_id ?? 0 );
|
||||
$webhook->save();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* When users are about to be deleted show an informative text if they have webhooks assigned.
|
||||
*
|
||||
* @param \WP_User $current_user The current logged in user.
|
||||
* @param array $userids Array with the ids of the users that are about to be deleted.
|
||||
* @return void
|
||||
* @since 7.8.0
|
||||
*/
|
||||
private function maybe_render_user_with_webhooks_warning( \WP_User $current_user, array $userids ): void {
|
||||
global $wpdb;
|
||||
|
||||
$at_least_one_user_with_webhooks = false;
|
||||
|
||||
foreach ( $userids as $user_id ) {
|
||||
$webhook_ids = $this->get_webhook_ids_for_user( $user_id );
|
||||
if ( empty( $webhook_ids ) ) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$at_least_one_user_with_webhooks = true;
|
||||
|
||||
$user_data = get_userdata( $user_id );
|
||||
$user_login = false === $user_data ? '' : $user_data->user_login;
|
||||
$webhooks_count = count( $webhook_ids );
|
||||
|
||||
$text = sprintf(
|
||||
/* translators: 1 = user id, 2 = user login, 3 = webhooks count */
|
||||
_nx(
|
||||
'User #%1$s %2$s has created %3$d WooCommerce webhook.',
|
||||
'User #%1$s %2$s has created %3$d WooCommerce webhooks.',
|
||||
$webhooks_count,
|
||||
'user webhook count',
|
||||
'woocommerce'
|
||||
),
|
||||
$user_id,
|
||||
$user_login,
|
||||
$webhooks_count
|
||||
);
|
||||
|
||||
echo '<p>' . esc_html( $text ) . '</p>';
|
||||
}
|
||||
|
||||
if ( ! $at_least_one_user_with_webhooks ) {
|
||||
return;
|
||||
}
|
||||
|
||||
$webhooks_settings_url = esc_url_raw( admin_url( 'admin.php?page=wc-settings&tab=advanced§ion=webhooks' ) );
|
||||
|
||||
// This block of code is copied from WordPress' users.php.
|
||||
// phpcs:disable WooCommerce.Commenting.CommentHooks, WordPress.DB.PreparedSQL.NotPrepared
|
||||
$users_have_content = (bool) apply_filters( 'users_have_additional_content', false, $userids );
|
||||
if ( ! $users_have_content ) {
|
||||
if ( $wpdb->get_var( "SELECT ID FROM {$wpdb->posts} WHERE post_author IN( " . implode( ',', $userids ) . ' ) LIMIT 1' ) ) {
|
||||
$users_have_content = true;
|
||||
} elseif ( $wpdb->get_var( "SELECT link_id FROM {$wpdb->links} WHERE link_owner IN( " . implode( ',', $userids ) . ' ) LIMIT 1' ) ) {
|
||||
$users_have_content = true;
|
||||
}
|
||||
}
|
||||
// phpcs:enable WooCommerce.Commenting.CommentHooks, WordPress.DB.PreparedSQL.NotPrepared
|
||||
|
||||
if ( $users_have_content ) {
|
||||
$text = __( 'If the "Delete all content" option is selected, the affected WooCommerce webhooks will <b>not</b> be deleted and will be attributed to user id 0.<br/>', 'woocommerce' );
|
||||
} else {
|
||||
$text = __( 'The affected WooCommerce webhooks will <b>not</b> be deleted and will be attributed to user id 0.<br/>', 'woocommerce' );
|
||||
}
|
||||
|
||||
$text .= sprintf(
|
||||
/* translators: 1 = url of the WooCommerce webhooks settings page */
|
||||
__( 'After that they can be reassigned to the logged-in user by going to the <a href="%1$s">WooCommerce webhooks settings page</a> and re-saving them.', 'woocommerce' ),
|
||||
$webhooks_settings_url
|
||||
);
|
||||
|
||||
echo '<p>' . wp_kses_post( $text ) . '</p>';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the ids of the webhooks assigned to a given user.
|
||||
*
|
||||
* @param int $user_id User id.
|
||||
* @return int[] Array of webhook ids.
|
||||
*/
|
||||
private function get_webhook_ids_for_user( int $user_id ): array {
|
||||
$data_store = \WC_Data_Store::load( 'webhook' );
|
||||
return $data_store->search_webhooks(
|
||||
array(
|
||||
'user_id' => $user_id,
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
|
@ -34,4 +34,82 @@ class WC_Webhook_Test extends WC_Unit_Test_Case {
|
|||
$this->assertFalse( $call_is_valid_function->call( $webhook, $product->get_id() ) );
|
||||
}
|
||||
|
||||
/**
|
||||
* @testDox Check that a deleted administrator user (with content re-assigned to another user)
|
||||
* does not cause webhook payloads to fail.
|
||||
*/
|
||||
public function test_payload_for_deleted_user_id_with_reassign() {
|
||||
$admin_user_id_1 = wp_insert_user(
|
||||
array(
|
||||
'user_login' => 'test_admin',
|
||||
'user_pass' => 'password',
|
||||
'role' => 'administrator',
|
||||
)
|
||||
);
|
||||
|
||||
$webhook = new WC_Webhook();
|
||||
$webhook->set_topic( 'order.created' );
|
||||
$webhook->set_user_id( $admin_user_id_1 );
|
||||
$webhook->save();
|
||||
|
||||
$order = \Automattic\WooCommerce\RestApi\UnitTests\Helpers\OrderHelper::create_order();
|
||||
|
||||
$payload = $webhook->build_payload( $order->get_id() );
|
||||
$this->assertArrayNotHasKey( 'code', $payload );
|
||||
$this->assertArrayHasKey( 'id', $payload );
|
||||
$this->assertSame( $order->get_id(), $payload['id'] );
|
||||
|
||||
// Create a second admin user and delete the first one, reassigning existing content to the second user.
|
||||
$admin_user_id_2 = wp_insert_user(
|
||||
array(
|
||||
'user_login' => 'test_admin2',
|
||||
'user_pass' => 'password',
|
||||
'role' => 'administrator',
|
||||
)
|
||||
);
|
||||
wp_delete_user( $admin_user_id_1, $admin_user_id_2 );
|
||||
|
||||
// Re-load the webhook from the database.
|
||||
$webhook = new WC_Webhook( $webhook->get_id() );
|
||||
// Confirm user_id has been updated to the second admin user.
|
||||
$this->assertSame( $admin_user_id_2, $webhook->get_user_id() );
|
||||
|
||||
$this->assertArrayNotHasKey( 'code', $payload );
|
||||
$this->assertArrayHasKey( 'id', $payload );
|
||||
$this->assertSame( $order->get_id(), $payload['id'] );
|
||||
}
|
||||
|
||||
/**
|
||||
* @testDox Check that a deleted administrator user (without content re-assigned to another user)
|
||||
* has all webhooks changed to user_id zero.
|
||||
*/
|
||||
public function test_payload_for_deleted_user_id_without_reassign() {
|
||||
$admin_user_id = wp_insert_user(
|
||||
array(
|
||||
'user_login' => 'test_admin',
|
||||
'user_pass' => 'password',
|
||||
'role' => 'administrator',
|
||||
)
|
||||
);
|
||||
|
||||
$webhook1 = new WC_Webhook();
|
||||
$webhook1->set_topic( 'order.created' );
|
||||
$webhook1->set_user_id( $admin_user_id );
|
||||
$webhook1->save();
|
||||
|
||||
$webhook2 = new WC_Webhook();
|
||||
$webhook2->set_topic( 'order.created' );
|
||||
$webhook2->set_user_id( 999 );
|
||||
$webhook2->save();
|
||||
|
||||
wp_delete_user( $admin_user_id );
|
||||
|
||||
// Re-load the webhooks from the database.
|
||||
$webhook1 = new WC_Webhook( $webhook1->get_id() );
|
||||
$webhook2 = new WC_Webhook( $webhook2->get_id() );
|
||||
// Confirm user_id has been updated to zero for the first webhook only.
|
||||
$this->assertSame( 0, $webhook1->get_user_id() );
|
||||
$this->assertSame( 999, $webhook2->get_user_id() );
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -0,0 +1,49 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Tests relating to the WC_Webhook_Data_Store class.
|
||||
*/
|
||||
class WC_Webhook_Data_Store_Test extends WC_Unit_Test_Case {
|
||||
|
||||
/**
|
||||
* Verify that the search_webhooks() method returns the correct results when searching by user ID.
|
||||
*/
|
||||
public function test_search_webhooks_by_user_id() {
|
||||
|
||||
$store = new WC_Webhook_Data_Store();
|
||||
|
||||
$this->assertEmpty(
|
||||
$store->search_webhooks(
|
||||
array(
|
||||
'user_id' => 1,
|
||||
)
|
||||
)
|
||||
);
|
||||
|
||||
$webhook1 = new WC_Webhook();
|
||||
$webhook1->set_user_id( 1 );
|
||||
$webhook1->save();
|
||||
|
||||
$webhook2 = new WC_Webhook();
|
||||
$webhook2->set_user_id( 2 );
|
||||
$webhook2->save();
|
||||
|
||||
$this->assertSame(
|
||||
array( $webhook1->get_id() ),
|
||||
$store->search_webhooks(
|
||||
array(
|
||||
'user_id' => 1,
|
||||
)
|
||||
)
|
||||
);
|
||||
$this->assertSame(
|
||||
array( $webhook2->get_id() ),
|
||||
$store->search_webhooks(
|
||||
array(
|
||||
'user_id' => 2,
|
||||
)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in New Issue