Fix: ErrorException when user list has been limitated

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.

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 <re@lloc.de>
  */
 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__ );
 

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
 

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_*`

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(
+				'<p class="description">%s</p>',
+				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
+					)
+				)
+			);
+		}
 	}
 
 	/**

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 );

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

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": {

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( '/^<select id="reference_user" name="msls\[reference_user\]">.*$/' );
+		ob_start();
+		$obj->reference_user();
+		$output = (string) ob_get_clean();
+
+		$this->assertStringStartsWith( '<select id="reference_user" name="msls[reference_user]">', $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(
+			'#^<select id="reference_user" name="msls\[reference_user\]">.*</select><p class="description">The user list has been limited to 100 users\.</p>$#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(
+			'#^<select id="reference_user" name="msls\[reference_user\]">.*</select><p class="description">The user list has been limited to 1 user\.</p>$#s'
+		);
 		$obj->reference_user();
 	}
 

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() );
 

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();