WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#14405 closed enhancement (fixed)

New Conditional Template Tag: is_multi_author()

Reported by: iandstewart Owned by: westi
Milestone: 3.2 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch commit
Focuses: Cc:

Description

With the popularity of publicly-released magazine themes it'd be great if themers had an easy way to check for the existence of multiple published authors on a blog.

It'd be useful for single author blogs as well--themers can hide author names and links when there is only 1 published author.

Attachments (6)

is-multi-author.diff (1.3 KB) - added by iandstewart 4 years ago.
saner-is-multi-author.diff (522 bytes) - added by iandstewart 4 years ago.
is_multi_author_cached.diff (1.1 KB) - added by westi 4 years ago.
Check for multi author and wp cache the result, clear on any post transition
is_multi_author_cached.2.14405.diff (1.1 KB) - added by filosofo 4 years ago.
is_multi_author_cached.3.14405.diff (1.1 KB) - added by filosofo 4 years ago.
14405.transient.1.diff (1.1 KB) - added by wpmuguru 3 years ago.
use a transient instead of persistent cache

Download all attachments as: .zip

Change History (39)

iandstewart4 years ago

comment:1 filosofo4 years ago

I don't understand all the extra logic that you have there, the results of which don't seem to be used.

Can't you just do something like the following?

function is_multi_author() {
   global $wpdb;
   return (bool) ( 1 < $wpdb->get_var("SELECT COUNT(DISTINCT(post_author)) FROM $wpdb->posts WHERE post_type = 'post' AND " . get_private_posts_cap_sql( 'post' ) );
}

comment:2 iandstewart4 years ago

  • Cc ian@… added

Indeed. That was a hacked up wp_list_authors. I'll attach your (far more elegant) version, filosofo.

comment:3 follow-up: wpmuguru4 years ago

Why not keep track of the multi_author status via the save_post hook?

If the proposed version was implemented and used in a theme it would add a database query to every pageload.

comment:4 in reply to: ↑ 3 lancewillett4 years ago

+1 for the idea, love that it will solve the problem of single-author sites and not wanting the redundant author name and link on each post.

Replying to wpmuguru:

Why not keep track of the multi_author status via the save_post hook?

How would you suggest keeping track of the value? Even if it was stored as an option or meta value somewhere it'd still require a database query to get that value, too, right?

comment:5 nacin4 years ago

Realistically it could be an autoloaded boolean option.

comment:6 nacin4 years ago

Posted this on wpdevel, http://wpdevel.wordpress.com/2010/08/30/at-this-weeks-dev-chat-well-start-t/#comment-10095.

Would be dirt cheap to store it in an autoloaded option that stores either 0 or 1, and we can toggle it based on user creation. This probably can also assist in our other instances where we don’t show author drop-downs in the admin area based on the number of non-subscriber users.

Wouldn't you know I had the same idea five weeks ago, as the previous comment? :)

comment:7 nacin4 years ago

On user creation, or on post creation. Probably a combination of both (and role changes).

comment:8 westi4 years ago

I don't think this should care about private posts.

Just is more than one author with published posts.

westi4 years ago

Check for multi author and wp cache the result, clear on any post transition

comment:9 westi4 years ago

Simple patch with caching.

Clears on post transitions.

Could use a transient instead but this feels like something that should cache for as long as possible if we have a cache and fall back on the query otherwise as it isn't too slow anyway.

comment:10 follow-up: nacin4 years ago

We can use the publish_post hook instead. This will add an extra query to basically any site that doesn't use a persistent cache. While I like that we can leverage the idea of having a persistent cache for many situations, this seems like a good case IMO for just a flag that gets updated on publish_post.

comment:11 in reply to: ↑ 10 westi4 years ago

Replying to nacin:

We can use the publish_post hook instead. This will add an extra query to basically any site that doesn't use a persistent cache. While I like that we can leverage the idea of having a persistent cache for many situations, this seems like a good case IMO for just a flag that gets updated on publish_post.

Only sites that use the function.

And the query is super fast anyways

comment:12 follow-up: filosofo4 years ago

Rather than

SELECT COUNT( DISTINCT post_author )  FROM $wpdb->posts

how about instead

SELECT DISTINCT post_author FROM $wpdb->posts ... LIMIT 2

Since we only need 2 distinct authors to be multi-author, we can let the db engine stop once it's found 2 authors: it considers only the number of table rows it has to traverse to before finding 2 authors. In contrast, the former query actually considers all rows, according to EXPLAIN results.

Please see patch.

comment:13 in reply to: ↑ 12 westi4 years ago

Replying to filosofo:

Rather than

SELECT COUNT( DISTINCT post_author )  FROM $wpdb->posts

how about instead

SELECT DISTINCT post_author FROM $wpdb->posts ... LIMIT 2

Since we only need 2 distinct authors to be multi-author, we can let the db engine stop once it's found 2 authors: it considers only the number of table rows it has to traverse to before finding 2 authors. In contrast, the former query actually considers all rows, according to EXPLAIN results.

Please see patch.

Great thinking outside the box optimisation.

Thank you!

comment:14 filosofo4 years ago

is_multi_author_cached.3.14405.diff responds to my realization that the cache check wasn't going to work properly without checking the type: cache an integer to distinguish single-author (int) 0 from invalid cache (bool) false.

comment:15 follow-up: scribu4 years ago

We should use get_posts() here, so that we can set 'post_type' => 'any'. Would need #14777

comment:16 in reply to: ↑ 15 ; follow-up: westi4 years ago

Replying to scribu:

We should use get_posts() here, so that we can set 'post_type' => 'any'. Would need #14777

The demand for this is very Post specific though rather than CPT?

comment:17 in reply to: ↑ 16 nacin4 years ago

Replying to westi:

The demand for this is very Post specific though rather than CPT?

Agreed, that's why I didn't suggest it. Published posts seems fine.

comment:18 scribu4 years ago

Ok, but we should still use get_posts(), with no caching etc.

comment:19 nacin3 years ago

  • Keywords has-patch commit added
  • Milestone changed from Awaiting Review to 3.1

comment:20 markjaquith3 years ago

  • Milestone changed from 3.1 to Future Release

Punt.

comment:21 westi3 years ago

  • Keywords 3.2-early added

comment:22 iandstewart3 years ago

Is there a chance of this making 3.2? I'd love to have it available for the Twenty Eleven theme.

comment:23 lancewillett3 years ago

+1 for getting this in 3.2.

One of our biggest requests on WP.com is for a way to enable group blogs to show author information (or turn it off for one-person blogs).

comment:24 follow-up: nacin3 years ago

  • Milestone changed from Future Release to 3.2

I'm still not a fan of the patches here. Still feels to me like relying on persistent caching is kind of lame. But I can go for it.

comment:25 scribu3 years ago

Well, the other solution would be to have the user flip the switch manually, either through an admin option (gasp!) or through some code in functions.php.

Last edited 3 years ago by scribu (previous) (diff)

comment:26 in reply to: ↑ 24 ; follow-up: wpmuguru3 years ago

Replying to nacin:

I'm still not a fan of the patches here. Still feels to me like relying on persistent caching is kind of lame.

Same here. It's about the same amount of code to do this via an option in the options table.

comment:27 in reply to: ↑ 26 ; follow-up: westi3 years ago

Replying to wpmuguru:

Replying to nacin:

I'm still not a fan of the patches here. Still feels to me like relying on persistent caching is kind of lame.

Same here. It's about the same amount of code to do this via an option in the options table.

But it's not an option.

Maybe we should use a transient instead of directly caching so we get the benefit of the temporary persistence from that.

comment:28 westi3 years ago

  • Owner set to westi
  • Status changed from new to reviewing

comment:29 in reply to: ↑ 27 wpmuguru3 years ago

Replying to westi:

Maybe we should use a transient instead of directly caching so we get the benefit of the temporary persistence from that.

Sounds like a decent compromise.

wpmuguru3 years ago

use a transient instead of persistent cache

comment:30 lancewillett3 years ago

  • Cc lancewillett added

comment:31 westi3 years ago

  • Keywords 3.2-early removed

"is_multi_author_cached.3.14405.diff" uses a nice fast query and the object cache (if it exists)

This means that for sites with an object cache it can be used to avoid queries

For sites without they just get one fast query - better than using a transient which means two queries one to check the timeout and one to fetch the value.

comment:32 nacin3 years ago

Go for it.

comment:33 westi3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In [17901]:

Introduce new is_multi_author() template tag to make it easier for themes to have different behaviour when a site has more than one
author. Fixes #14405 props filosofo.

Note: See TracTickets for help on using tickets.