Skip to content

65341 ajax actions widgets order #11963

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.

Sign up for GitHub

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

Open
pbearne wants to merge 5 commits into
base: trunk
Choose a base branch
from
+128 −1
Open

65341 ajax actions widgets order #11963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f888576
Tests: Add unit tests for wp_ajax_widgets_order()
May 20, 2026
97a6be7
Merge branch 'trunk' into 65252-ajax-actions-widgets_order
pbearne May 25, 2026
388b875
Tests: Update wp_ajax_widgets_order() test to use exception assertions
May 25, 2026
211aa63
Fix: Add isset check for 'sidebars' in wp_ajax_widgets_order()
May 25, 2026
754efcb
Update widgetsOrder.php
pbearne May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/wp-admin/includes/ajax-actions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2321,7 +2321,7 @@ function wp_ajax_widgets_order() {
unset( $_POST['savewidgets'], $_POST['action'] );

// Save widgets order for all sidebars.
if ( is_array( $_POST['sidebars'] ) ) {
if ( isset( $_POST['sidebars'] ) && is_array( $_POST['sidebars'] ) ) {
$sidebars = array();

foreach ( wp_unslash( $_POST['sidebars'] ) as $key => $val ) {
Expand Down
127 changes: 127 additions & 0 deletions tests/phpunit/tests/admin/includes/ajax-actions/widgetsOrder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
<?php

/**
* Admin Ajax functions to be tested.
*/
require_once ABSPATH . 'wp-admin/includes/ajax-actions.php';

/**
* Testing wp_ajax_widgets_order() functionality.
*
* @package WordPress
* @subpackage UnitTests
* @since 3.1.0
Copy link
Copy Markdown

@mindctrl mindctrl May 26, 2026

Choose a reason for hiding this comment

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

Is this the correct @since version?

*
* @group ajax
*
* @covers ::wp_ajax_widgets_order
*/
class Tests_wp_ajax_widgets_order extends WP_Ajax_UnitTestCase {
Copy link
Copy Markdown

@mindctrl mindctrl May 26, 2026

Choose a reason for hiding this comment

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

I think the classname, or filename, needs to be changed? If running these tests in isolation, by referencing the filename, you get an error about being unable to find class widgetsOrder.

npm run test:php -- ./tests/phpunit/tests/admin/includes/ajax-actions/widgetsOrder.php --group ajax

> WordPress@7.1.0 test:php
> node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpunit ./tests/phpunit/tests/admin/includes/ajax-actions/widgetsOrder.php --group ajax

[dotenv@17.3.1] injecting env (19) from .env -- tip: ⚙️  override existing env vars with { override: true }
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
Class widgetsOrder could not be found in /var/www/tests/phpunit/tests/admin/includes/ajax-actions/widgetsOrder.php


/**
* Administrator user ID.
*
* @var int
*/
protected static $admin_id;

/**
* Setup test fixtures.
*
* @param WP_UnitTest_Factory $factory
*/
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ): void {
self::$admin_id = $factory->user->create( array( 'role' => 'administrator' ) );
}

/**
* Tests successful widgets order saving.
*
* @ticket 65252
*/
public function test_widgets_order_success(): void {
wp_set_current_user( self::$admin_id );

// Mock sidebars.
$sidebars = array(
'sidebar-1' => 'widget-1_text-1,widget-2_text-2',
'sidebar-2' => 'widget-3_search-1',
);

$_POST = array(
'action' => 'widgets-order',
'savewidgets' => wp_create_nonce( 'save-sidebar-widgets' ),
'sidebars' => $sidebars,
);

$this->expectException( WPAjaxDieStopException::class );
$this->expectExceptionMessage( '1' );

$this->_handleAjax( 'widgets-order' );

$this->assertSame( '1', $this->_last_response );

$updated_sidebars = wp_get_sidebars_widgets();
$this->assertContains( 'text-1', $updated_sidebars['sidebar-1'] );
$this->assertContains( 'text-2', $updated_sidebars['sidebar-1'] );
$this->assertContains( 'search-1', $updated_sidebars['sidebar-2'] );
Comment on lines +65 to +67
Copy link
Copy Markdown

@mindctrl mindctrl May 26, 2026

Choose a reason for hiding this comment

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

I don't think these assertions run because they come after phpunit catches the exception thrown by _handleAjax(). If you disable these, you'll see the assertion count remains the same. Same for line 62 I think.

Copy link
Copy Markdown

@mindctrl mindctrl May 26, 2026

Choose a reason for hiding this comment

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

I'm not sure if this is the best/proper approach, but if you do the test like this, it will run the other assertions:

		try {
			$this->_handleAjax( 'widgets-order' );
			$this->fail( 'Expected WPAjaxDieStopException was not thrown.' );
		} catch ( WPAjaxDieStopException $e ) {
			$this->assertSame( '1', $e->getMessage() );
		}

}

/**
* Tests failure due to invalid nonce.
*
* @ticket 65252
*/
public function test_widgets_order_invalid_nonce(): void {
wp_set_current_user( self::$admin_id );

$_POST = array(
'action' => 'widgets-order',
'savewidgets' => 'invalid-nonce',
);

$this->expectException( WPAjaxDieStopException::class );
$this->expectExceptionMessage( '-1' );

$this->_handleAjax( 'widgets-order' );
}

/**
* Tests failure due to insufficient permissions.
*
* @ticket 65252
*/
public function test_widgets_order_insufficient_permissions(): void {
$user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) );
wp_set_current_user( $user_id );

$_POST = array(
'action' => 'widgets-order',
'savewidgets' => wp_create_nonce( 'save-sidebar-widgets' ),
);

$this->expectException( WPAjaxDieStopException::class );
$this->expectExceptionMessage( '-1' );

$this->_handleAjax( 'widgets-order' );
}

/**
* Tests behavior when sidebars parameter is missing.
*
* @ticket 65252
*/
public function test_widgets_order_missing_sidebars(): void {
wp_set_current_user( self::$admin_id );

$_POST = array(
'action' => 'widgets-order',
'savewidgets' => wp_create_nonce( 'save-sidebar-widgets' ),
);

$this->expectException( WPAjaxDieStopException::class );
$this->expectExceptionMessage( '-1' );

$this->_handleAjax( 'widgets-order' );
}
}
Loading