wp_list_authors ignores 'exclude_admin' arg when admin account has a display_name other then 'admin'

[raphaabreu]

Line 293 of author-template.php should be changed from:

if ( $exclude_admin && 'admin' == $author->display_name )

to:

if ( $exclude_admin && 'admin' == $author->user_login )

Thanks.

[knutsp]

'admin' should be avoided as user_login anyway. On a single site there is usually a site owner, identified by the user who has the same email as the one who installed it and may be still be the receiver of admin emails for the site.

if ( $exclude_admin && get_option('admin_email') == $author->user_email )

This will exclude one, or sometimes no users. Or just deprecate the whole $exclude_admin thing in favor of something more modern in terms of today's WordPress roles and capabilities.

[SergeyBiryukov]

Related: #9165, #9902

Introduced in [15620], previously user_login field was used for comparison.

The parameter itself was added in [2632]. We should probably deprecate it in favor of exclude (#9902).

Still, fixing it makes sense to me. The description refers to "the 'admin' user that is installed by default":
​http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/author-template.php#L246

Codex specifically refers to the login: "Exclude the 'admin' (login is admin) account from the list":
​http://codex.wordpress.org/Function_Reference/wp_list_authors#Parameters

The default username, however, can be manually picked now (#10396).

I guess we have the following options to fix this:

• 'admin' == $author->user_login.
• 1 == $author->ID (from #9165).
• get_option('admin_email') == $author->user_email (from comment:1).

[SergeyBiryukov]

In 23448:

Fix typo in phpdoc. see #23498.

[raphaabreu]

I am fine with deprecating it in favor of exclude.

However, I am updating several old installs, some of them have multiple accounts with the same user e-mail. The third option would create another issue where several authors would not be shown.

Either option 1 or 2 would work perfectly in my humble opinion.

[jonna]

Would it be possible for someone to post the code that would allow me to exclude admin, as well as other users (I think this is the correct way to go).

I really need to get this done and this bug has been up for 3 months with no response.

[SergeyBiryukov]

#26544 was marked as a duplicate.

[SergeyBiryukov]

Replying to SergeyBiryukov:

I guess we have the following options to fix this:

ticket:26544:wp-list-author-number-exclude-admin.diff​ implements a fourth option: populating $query_args['exclude'] with the list of all administrator IDs.

[MikeHansenMe]

I think excluding the admin before/during get users is important because it breaks the number arg. For example if I request 2 authors and admin is returned as one of them then gets excluded, it only returns 1 author even thought I requested 2. I have been looking at my previous patch and think I need to look at it a bit more but I do think it is a better approach because of the number issue.

[nacin]

Unit test, from #26538 (also by MikeHansenMe):

	/**
	 * @ticket 23498
	 */
	function test_wp_list_authors_number() {
		$expected['number'] = '<li><a href="http://example.org/?author=' . $this->users[1] . '" title="Posts by bob">bob</a></li><li><a href="http://example.org/?author=' . $this->users[2] . '" title="Posts by paul">paul</a></li>';
		$this->AssertEquals( $expected['number'], wp_list_authors( array( 'echo' => false, 'number' => 2 ) ) );
	}

[MikeHansenMe]

Refreshed because of the extract ticket and with unit tests

[donmhico]

Refreshed the patch for 5.3.

[donmhico]

The bug still persist in 5.3. I've attached a refreshed patch - 23498.4.diff - and I did some updates in the unit test.

[SergeyBiryukov]

In 45795:

Users: Make wp_list_authors() unit tests more readable.

See #23498.

[SergeyBiryukov]

Found a related issue while looking into this.

$query_args['fields'] = 'ids' added in [17100] works here, but only because $wpdb->users.ID is the default fallback​ in WP_User_Query::prepare_query().

$query_args['fields'] = 'ID' should be used instead.

23498.5.diff​ fixes that, uses the same approach for the get_users() call in the exclude_admin branch, and updates documentation.

The proposed unit test doesn't seem to be any different from test_wp_list_authors_default(). We already have a test for test_wp_list_authors_exclude_admin(), that seems enough here, but maybe could be expanded for multiple administrators.

Given that exclude_admin is true by default, I'm not 100% sure excluding all administrators would be the expected behavior for wp_list_authors(), but I can see it both ways. Marking for commit, unless there are any objections.

[SergeyBiryukov]

Replying to SergeyBiryukov:

We already have a test for test_wp_list_authors_exclude_admin(), that seems enough here, but maybe could be expanded for multiple administrators.

23498.6.diff​ adds a second administrator to the tests.

[SergeyBiryukov]

Replying to SergeyBiryukov:

Given that exclude_admin is true by default, I'm not 100% sure excluding all administrators would be the expected behavior for wp_list_authors(), but I can see it both ways.

Thought about this some more. Given that:

• exclude_admin parameter was limited to admin display name for the last 9 years, since [15620] / #10329.
• admin is no longer a default username on install for the last 6 years, since [24998] / #24078.

the effect of exclude_admin was pretty limited. The suggested change might result in unexpected behavior, where wp_list_authors() would no longer include users who are post authors, but also administrators.

Note: neither core nor bundled themes use the function, so third party themes are the main concern here.

Maybe changing the default value to false and adjusting the tests and documentation accordingly would make the most sense here?

[talldanwp]

@SergeyBiryukov This ticket was discussed during a triage session (link above this comment).

The feedback was generally in agreement with your last comment that it'd be less than ideal if the behaviour of the function changes considerably for those who've implemented it.

One thought that I had is that given the arguments are in an options array, it might be that exclude_admin can be deprecated and replaced with exclude_administrator_role which has the new behaviour proposed in the patch. That would ensure that anyone using the function in its existing capacity doesn't see any/much change.

[davidbaumwald]

@SergeyBiryukov With 5.3 Beta 3 shipping today, how do you feel about this ticket?

[kirasong]

Thanks everyone for your work and feedback on this!

Since we're nearing the 5.3 RC and it's been a couple of weeks since this ticket has seen movement, I'm going to go ahead and move it to Future Release.

Please feel free to move it back into the milestone if consensus is formed/an appropriate patch becomes available before RC!

[knutsp]

Suggestion:

1. Add exclude_administrators argument to exclude any with administrator role alone (unusual, but in this case it may open for some administrators to be included)

2. Soft deprecate exclude_admin (no deprecation warning emitted)

Update docs to reflect this.

[SergeyBiryukov]

#60071 was marked as a duplicate.