Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh sidebars widgets global after calling wp_set_sidebars_widgets #1525

Open
wants to merge 13 commits into
base: trunk
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,9 @@ public function update_item( $request ) {
global $wp_widget_factory;

/*
* retrieve_widgets() contains logic to move "hidden" or "lost" widgets to the
* wp_inactive_widgets sidebar based on the contents of the $sidebars_widgets global.
*
* When batch requests are processed, this global is not properly updated by previous
* calls, resulting in widgets incorrectly being moved to the wp_inactive_widgets
* sidebar.
*
* See https://core.trac.wordpress.org/ticket/53657.
* The name retrieve_widgets() is somewhat misleading. It doesn't just "retrieve". It
* also moves any "hidden" or "lost" widgets to the wp_inactive_widgets sidebar.
*/
wp_get_sidebars_widgets();
$this->retrieve_widgets();

$widget_id = $request['id'];
Expand Down Expand Up @@ -367,16 +360,9 @@ public function delete_item( $request ) {
global $wp_widget_factory, $wp_registered_widget_updates;

/*
* retrieve_widgets() contains logic to move "hidden" or "lost" widgets to the
* wp_inactive_widgets sidebar based on the contents of the $sidebars_widgets global.
*
* When batch requests are processed, this global is not properly updated by previous
* calls, resulting in widgets incorrectly being moved to the wp_inactive_widgets
* sidebar.
*
* See https://core.trac.wordpress.org/ticket/53657.
* The name retrieve_widgets() is somewhat misleading. It doesn't just "retrieve". It also
* moves any "hidden" or "lost" widgets to the wp_inactive_widgets sidebar.
*/
wp_get_sidebars_widgets();
$this->retrieve_widgets();

$widget_id = $request['id'];
Expand Down
34 changes: 26 additions & 8 deletions src/wp-includes/widgets.php
Original file line number Diff line number Diff line change
Expand Up @@ -1072,22 +1072,37 @@ function wp_get_sidebar( $id ) {
* Set the sidebar widget option to update sidebars.
*
* @since 2.2.0
* @since 6.2.0 Added the optional $refresh_global_sidebars_widgets argument.
adamziel marked this conversation as resolved.
Show resolved Hide resolved
* @access private
*
* @global array $_wp_sidebars_widgets
* @param array $sidebars_widgets Sidebar widgets and their settings.
* @global array $sidebars_widgets
*
* @param array $new_sidebars_widgets Sidebar widgets and their settings.
* @param bool $refresh_global_sidebars_widgets Optional. Whether to update $sidebars_widgets
* global. Default true.
*/
function wp_set_sidebars_widgets( $sidebars_widgets ) {
global $_wp_sidebars_widgets;
function wp_set_sidebars_widgets( $new_sidebars_widgets, $refresh_global_sidebars_widgets = true ) {
adamziel marked this conversation as resolved.
Show resolved Hide resolved
global $_wp_sidebars_widgets, $sidebars_widgets;

// Clear cached value used in wp_get_sidebars_widgets().
$_wp_sidebars_widgets = null;

if ( ! isset( $sidebars_widgets['array_version'] ) ) {
$sidebars_widgets['array_version'] = 3;
if ( ! isset( $new_sidebars_widgets['array_version'] ) ) {
$new_sidebars_widgets['array_version'] = 3;
}

update_option( 'sidebars_widgets', $sidebars_widgets );
update_option( 'sidebars_widgets', $new_sidebars_widgets );

// Refresh the $sidebars_widgets global.
if ( $refresh_global_sidebars_widgets ) {
$sidebars_widgets = wp_get_sidebars_widgets();
/*
* The name retrieve_widgets() is somewhat misleading. It doesn't just "retrieve". It also
* moves any "hidden" or "lost" widgets to the wp_inactive_widgets sidebar.
*/
retrieve_widgets( true );
adamziel marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
Expand Down Expand Up @@ -1353,8 +1368,11 @@ function retrieve_widgets( $theme_changed = false ) {
$sidebars_widgets['wp_inactive_widgets'] = array_merge( $lost_widgets, (array) $sidebars_widgets['wp_inactive_widgets'] );

if ( 'customize' !== $theme_changed ) {
// Update the widgets settings in the database.
wp_set_sidebars_widgets( $sidebars_widgets );
/*
* The second parameter is false to avoid recursion: refreshing the global
* $sidebars_widgets entails calling retrieve_widgets().
desrosj marked this conversation as resolved.
Show resolved Hide resolved
*/
wp_set_sidebars_widgets( $sidebars_widgets, false );
draganescu marked this conversation as resolved.
Show resolved Hide resolved
}

return $sidebars_widgets;
Expand Down
84 changes: 84 additions & 0 deletions tests/phpunit/tests/rest-api/rest-widgets-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,90 @@ public function test_create_item_second_instance() {
);
}

/**
* Tests that running multiple request handlers (create and delete) deletes the intended widget.
*
* See this comment for more details:
* https://github.com/WordPress/gutenberg/issues/33335#issuecomment-879903958
*
* @ticket 53816
adamziel marked this conversation as resolved.
Show resolved Hide resolved
* @covers WP_REST_Widgets_Controller::create_item
* @covers WP_REST_Widgets_Controller::delete_item
*/
public function test_create_and_delete() {
$this->setup_widget(
'text',
1,
array(
'text' => 'Custom text test',
)
);
$this->setup_sidebar(
'sidebar-1',
array(
'name' => 'Test sidebar',
),
array( 'text-1' )
);

// Create a new text widget.
$request = new WP_REST_Request( 'POST', '/wp/v2/widgets' );
adamziel marked this conversation as resolved.
Show resolved Hide resolved
$request->set_body_params(
array(
'id_base' => 'text',
'sidebar' => 'sidebar-1',
'instance' => array(
'encoded' => base64_encode(
serialize(
array(
'text' => 'Updated text test',
)
)
),
'hash' => wp_hash(
serialize(
array(
'text' => 'Updated text test',
)
)
),
),
)
);
rest_get_server()->dispatch( $request );

// Delete an old text widget (not the one we just created).
$request = new WP_REST_Request( 'DELETE', '/wp/v2/widgets/text-1' );
$request->set_query_params( array( 'force' => true ) );
rest_do_request( $request );

// Get a list of all widgets.
$request = new WP_REST_Request( 'GET', '/wp/v2/widgets' );
$request['context'] = 'edit';
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();
$data = $this->remove_links( $data );

/*
* Confirm that we deleted exactly the widget that we wanted, and
* no other one. This tests against a regression in running multiple
* request handlers during the same run. See the following comment for more details:
* https://github.com/WordPress/gutenberg/issues/33335#issuecomment-879903958
*/
$this->assertCount( 1, $data, 'The text-1 widget was not deleted' );
$this->assertSame( 'text-2', $data[0]['id'], 'The text-2 widget was not preserved' );
$this->assertSame( 'sidebar-1', $data[0]['sidebar'], 'The text-2 widget is no longer assigned to sidebar-1' );
$this->assertSameSetsWithIndex(
array(
'text' => 'Updated text test',
'title' => '',
'filter' => false,
),
$data[0]['instance']['raw'],
'The content of the text-2 widget changed'
);
}

/**
* @ticket 41683
*/
Expand Down