diff --git a/Changelog.md b/Changelog.md index 5dd818e7..44fd2a47 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +## 3.0.2 +* Fix: opening the settings page on a blog with more users than the "Reference user" dropdown shows raised a PHP notice, which landed in the error log and in services such as Sentry. The limit is now reported as a hint below the dropdown, where it belongs. +* Internal: detect the truncated user list from the query itself instead of calling `count_users()`, which drops a costly query from the settings page on blogs with many users. + ## 3.0.1 * Fix: the WordPress.org deploy pushed the whole workspace instead of the built distribution. * No changes to the plugin itself - the shipped code is identical to 3.0.0. diff --git a/MultisiteLanguageSwitcher.php b/MultisiteLanguageSwitcher.php index 94345d6e..e0261299 100644 --- a/MultisiteLanguageSwitcher.php +++ b/MultisiteLanguageSwitcher.php @@ -3,7 +3,7 @@ * Multisite Language Switcher Plugin * * Plugin Name: Multisite Language Switcher - * Version: 3.0.1 + * Version: 3.0.2 * Plugin URI: http://msls.co/ * Description: A simple but powerful plugin that will help you to manage the relations of your contents in a multilingual multisite-installation. * Author: Dennis Ploetner @@ -42,7 +42,7 @@ * @author Dennis Ploetner */ if ( ! defined( 'MSLS_PLUGIN_VERSION' ) ) { - define( 'MSLS_PLUGIN_VERSION', '3.0.1' ); + define( 'MSLS_PLUGIN_VERSION', '3.0.2' ); define( 'MSLS_PLUGIN_PATH', plugin_basename( __FILE__ ) ); define( 'MSLS_PLUGIN__FILE__', __FILE__ ); diff --git a/docs/hooks.md b/docs/hooks.md index 8ef0b5ce..4711dfd3 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -223,6 +223,14 @@ Filter on the arguments passed to `get_users()` when MSLS builds its example by role, by capability, or by custom meta — so editors only see the intended reference accounts. +MSLS asks for one user more than it displays, because that extra row is how it +detects a truncated list without counting every user of the blog. Filtering +`number` here does not widen the dropdown: the result is still cut to +`msls_max_reference_users_count` entries. Lowering it shrinks the list and +also drops the "limited to n users" hint below it, because MSLS can then no +longer tell whether more users exist. Use `msls_max_reference_users_count` if +all you want is a different limit. + ### msls_blog_get_permalink Filter on the permalink resolved from a `Blog` object for the current @@ -286,14 +294,20 @@ a different role or a custom capability. Filter on the upper bound (default 100) of users listed in the "reference user" dropdown on the settings page. Increase the limit for networks with -many editors, or lower it to keep the dropdown light on big sites. +many editors, or lower it to keep the dropdown light on big sites. A value +below `1` is ignored and falls back to the default. + +When the blog holds more users than the limit, MSLS notes that below the +dropdown ("The user list has been limited to 100 users."). It does not raise +a PHP notice, so nothing lands in the error log. ### msls_reference_users Filter on the array of reference users — keyed by user ID and valued by -nicename — used to populate the "reference user" dropdown. Use it to -post-process the list after the user query has run, for example to relabel -entries or remove specific accounts. +nicename — used to populate the "reference user" dropdown. The array has +already been cut to `msls_max_reference_users_count` entries when it reaches +you. Use the hook to post-process the list, for example to relabel entries or +remove specific accounts. ### msls_admin_validate diff --git a/docs/snippets.md b/docs/snippets.md index 58f348ec..b921d998 100644 --- a/docs/snippets.md +++ b/docs/snippets.md @@ -203,6 +203,36 @@ remove_filter( ); ``` +## Widen the reference user dropdown + +The "Reference user" dropdown on the settings page lists at most 100 users, in +order of registration. On a blog with more accounts than that, MSLS notes the +limit below the dropdown. If the account you need is not among them, raise the +bound with `msls_max_reference_users_count`: + +```php +add_filter( 'msls_max_reference_users_count', function ( int $count ): int { + return 500; +} ); +``` + +A large blog is better served by narrowing the candidates instead of listing +more of them. `msls_get_users` filters the arguments of the underlying +`get_users()` call, so you can restrict the query to the roles that actually +qualify as a reference account: + +```php +add_filter( 'msls_get_users', function ( array $args ): array { + $args['role__in'] = array( 'administrator', 'editor' ); + + return $args; +} ); +``` + +Leave `number` alone in that callback: MSLS sets it to one above the display +limit and uses the extra row to detect a truncated list without counting every +user of the blog. + ## See also - [Public API Functions](api.md) — the full reference for every `msls_*` diff --git a/includes/Admin/Admin.php b/includes/Admin/Admin.php index 49bdfb92..3b8685a9 100644 --- a/includes/Admin/Admin.php +++ b/includes/Admin/Admin.php @@ -411,8 +411,20 @@ public function admin_display(): void { */ public function reference_user(): void { $max_users = (int) apply_filters( 'msls_max_reference_users_count', self::MAX_REFERENCE_USERS ); + if ( $max_users < 1 ) { + $max_users = self::MAX_REFERENCE_USERS; + } + + /** + * Ask for one user more than we show: the extra row is proof that the list is + * truncated, and it costs a lot less than counting every user of the blog. + */ + $users_collection = $this->collection->get_users( array( 'ID', 'user_nicename' ), $max_users + 1 ); - $users_collection = $this->collection->get_users( array( 'ID', 'user_nicename' ), $max_users ); + $is_limited = count( $users_collection ) > $max_users; + if ( $is_limited ) { + $users_collection = array_slice( $users_collection, 0, $max_users ); + } $reference_users = (array) apply_filters( 'msls_reference_users', @@ -421,6 +433,24 @@ public function reference_user(): void { // phpcs:ignore WordPress.Security.EscapeOutput echo ( new Select( 'reference_user', $reference_users, strval( $this->options->reference_user ) ) )->render(); + + if ( $is_limited ) { + printf( + '

%s

', + esc_html( + sprintf( + /* translators: %d: maximum number of users in the reference user dropdown */ + _n( + 'The user list has been limited to %d user.', + 'The user list has been limited to %d users.', + $max_users, + 'multisite-language-switcher' + ), + $max_users + ) + ) + ); + } } /** diff --git a/includes/Blog/Collection.php b/includes/Blog/Collection.php index 19c7f636..5e39b1cf 100644 --- a/includes/Blog/Collection.php +++ b/includes/Blog/Collection.php @@ -303,7 +303,7 @@ public function get_filtered( bool $filter = false ): array { } /** - * Gets the registered users of the current blog + * Gets the registered users of the current blog limited by the number of users and the fields to return. * * @param string|string[] $fields * @param int $number @@ -319,20 +319,6 @@ public function get_users( $fields = 'all', int $number = Admin::MAX_REFERENCE_U 'count_total' => false, ); - if ( 1 !== $number ) { // Check total users only if not fetching a single user. - $user_count = count_users(); - if ( $user_count['total_users'] > $number ) { - /* translators: %s: maximum number of users */ - $format = __( - 'Multisite Language Switcher: The user list has been limited to %d users.', - 'multisite-language-switcher' - ); - - // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error - trigger_error( esc_html( sprintf( $format, strval( $number ) ) ) ); - } - } - $args = (array) apply_filters( 'msls_get_users', $args ); return get_users( $args ); diff --git a/readme.txt b/readme.txt index 1fd3d643..ae2b33d1 100644 --- a/readme.txt +++ b/readme.txt @@ -6,7 +6,7 @@ Tags: multilingual, multisite, language, switcher, localization Requires at least: 6.1 Tested up to: 7.1 Requires PHP: 7.4 -Stable tag: 3.0.1 +Stable tag: 3.0.2 License: GPLv2 or later License URI: http://www.gnu.org/licenses/gpl-2.0.html @@ -93,6 +93,11 @@ Please visit the [MSLS website](https://msls.co/) or use the [WordPress support == Changelog == += 3.0.2 = + +* Fixed: with more than 100 users on a blog, opening the settings page raised a PHP notice that ended up in the error log (and in services such as Sentry). The truncated user list is now reported as a hint below the "Reference user" dropdown instead. +* Changed: MSLS no longer runs `count_users()` to detect the truncation, which removes a costly query from the settings page on blogs with many users. + = 3.0.1 = * Fixed: 3.0.0 could not be published on WordPress.org @@ -129,6 +134,12 @@ The full history is kept in the separate [Changelog](https://github.com/lloc/Mul == Upgrade Notice == += 3.0.2 = + +Bugfix release. The settings page no longer logs a PHP notice when the blog has more users +than the "Reference user" dropdown shows; the limit is displayed below the dropdown. The +`msls_max_reference_users_count` filter still adjusts that limit. + = 3.0.1 = Republishes 3.0.0, whose upload to WordPress.org failed. The plugin code is unchanged diff --git a/src/msls-widget-block/block.json b/src/msls-widget-block/block.json index ea9d5e9c..19b4c0b3 100644 --- a/src/msls-widget-block/block.json +++ b/src/msls-widget-block/block.json @@ -5,7 +5,7 @@ "icon": "translation", "category": "widgets", "name": "lloc/msls-widget-block", - "version": "3.0.1", + "version": "3.0.2", "description": "Review the settings for the Multisite Language Switcher plugin, as the block utilizes the API function `msls_the_switcher()` for its output.", "example": {}, "supports": { diff --git a/tests/phpunit/Admin/TestAdmin.php b/tests/phpunit/Admin/TestAdmin.php index 4584b0f3..11cf90cb 100644 --- a/tests/phpunit/Admin/TestAdmin.php +++ b/tests/phpunit/Admin/TestAdmin.php @@ -117,12 +117,75 @@ public function test_admin_display(): void { $obj->admin_display(); } - public function test_reference_user_over_max(): void { - $users = array( 1 => 'realloc' ); - Functions\expect( 'wp_list_pluck' )->once()->andReturn( $users ); + public function test_reference_user(): void { + Functions\expect( 'wp_list_pluck' )->once()->andReturn( array( 1 => 'realloc' ) ); + $obj = $this->AdminFactory(); - $this->expectOutputRegex( '/^', $output ); + $this->assertStringNotContainsString( 'class="description"', $output ); + } + + public function test_reference_user_over_max(): void { + $users = array(); + for ( $i = 1; $i <= Admin::MAX_REFERENCE_USERS + 1; $i++ ) { + $users[] = (object) array( + 'ID' => $i, + 'user_nicename' => 'user-' . $i, + ); + } + + Functions\expect( 'wp_list_pluck' )->once()->andReturnUsing( + function ( array $list ): array { + $this->assertCount( Admin::MAX_REFERENCE_USERS, $list ); + + return array_column( $list, 'user_nicename', 'ID' ); + } + ); + + $obj = $this->AdminFactory( $users ); + + $this->expectOutputRegex( + '#^

The user list has been limited to 100 users\.

$#s' + ); + $obj->reference_user(); + } + + public function test_reference_user_over_max_singular(): void { + $users = array( + (object) array( + 'ID' => 1, + 'user_nicename' => 'user-1', + ), + (object) array( + 'ID' => 2, + 'user_nicename' => 'user-2', + ), + ); + + Functions\when( 'apply_filters' )->alias( + function ( string $hook, $value ) { + return 'msls_max_reference_users_count' === $hook ? 1 : $value; + } + ); + + Functions\expect( 'wp_list_pluck' )->once()->andReturnUsing( + function ( array $list ): array { + $this->assertCount( 1, $list ); + + return array_column( $list, 'user_nicename', 'ID' ); + } + ); + + $obj = $this->AdminFactory( $users ); + + $this->expectOutputRegex( + '#^

The user list has been limited to 1 user\.

$#s' + ); $obj->reference_user(); } diff --git a/tests/phpunit/Blog/TestCollection.php b/tests/phpunit/Blog/TestCollection.php index f43d7801..91947b38 100644 --- a/tests/phpunit/Blog/TestCollection.php +++ b/tests/phpunit/Blog/TestCollection.php @@ -26,8 +26,6 @@ protected function setUp(): void { $this->captured_user_id = null; - Functions\when( 'count_users' )->justReturn( array( 'total_users' => self::TOTAL_USERS ) ); - $options = \Mockery::mock( Options::class ); $options->shouldReceive( 'get_order' )->andReturn( 'description' ); $options->shouldReceive( 'is_excluded' )->andReturn( false ); @@ -247,33 +245,13 @@ public function test_get_users_single(): void { public function test_get_users_massive(): void { Functions\expect( 'get_site_option' )->once()->andReturn( array() ); + Functions\expect( 'count_users' )->never(); $obj = new Collection(); $this->assertIsArray( $obj->get_users( array( 'ID' ), self::TOTAL_USERS ) ); } - public function test_get_users_max() { - Functions\expect( 'get_site_option' )->once()->andReturn( array() ); - - $max_users = 100; - - $obj = new Collection(); - - set_error_handler( - static function ( $errno, $errstr ) { - restore_error_handler(); - throw new \Exception( $errstr, $errno ); - }, - E_ALL - ); - - $this->expectException( \Exception::class ); - $this->expectExceptionMessage( "Multisite Language Switcher: The user list has been limited to {$max_users} users." ); - - $obj->get_users( 'all', $max_users ); - } - public function test_get_current_blog(): void { Functions\expect( 'get_site_option' )->once()->andReturn( array() ); diff --git a/tests/phpunit/MslsUnitTestCase.php b/tests/phpunit/MslsUnitTestCase.php index 438933ff..1da70d6a 100644 --- a/tests/phpunit/MslsUnitTestCase.php +++ b/tests/phpunit/MslsUnitTestCase.php @@ -16,6 +16,11 @@ protected function setUp(): void { \Mockery::namedMock( 'WooCommerce', \stdClass::class ); Functions\when( '__' )->returnArg(); + Functions\when( '_n' )->alias( + function ( string $single, string $plural, int $number ): string { + return 1 === $number ? $single : $plural; + } + ); Functions\when( 'esc_attr' )->returnArg(); Functions\when( 'esc_html' )->returnArg(); Functions\when( 'esc_html__' )->returnArg();