WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30354 closed defect (bug) (fixed)

get_posts_by_author_sql doesn't also return prefixed where is asked to

Reported by: pbearne Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.0
Component: Query Keywords: has-unit-tests has-patch
Focuses: administration Cc:
PR Number:

Description

In get_posts_by_author_sql() if you set the full to false it doesn't return the post type element of the query

Attachments (3)

30354.patch (875 bytes) - added by pbearne 5 years ago.
patch
30354_with_tests.patch (7.0 KB) - added by pbearne 5 years ago.
patch with unit tests
30354b.patch (7.8 KB) - added by pbearne 5 years ago.
patch with unit tests and extra SQL removed from wp_list_authors()

Download all attachments as: .zip

Change History (17)

@pbearne
5 years ago

patch

#1 @pbearne
5 years ago

  • Keywords has-patch added

@pbearne
5 years ago

patch with unit tests

#2 @pbearne
5 years ago

  • Keywords has-unit-tests added

Hi Guys

I have added some unit tests for this patch

This ticket was mentioned in Slack in #core by pbearne. View the logs.


5 years ago

#4 @pbearne
5 years ago

Thinking about this an example of the bad out put might help

if you call

get_posts_by_author_sql( 'page' ); or get_posts_by_author_sql( 'post', true );

which the same with the default
you get this string

"WHERE post_type = 'post' AND (post_status = 'publish')"

But if you call get_posts_by_author_sql( 'post', false ); which set the $full to false and not return the "WHERE" you get

" (post_status = 'publish')"

not

" post_type = 'post' AND (post_status = 'publish')"

as you would expect

this was due to a bad path in the $full route

I hope this ticket makes more sense now

Paul

#5 @johnbillion
5 years ago

  • Version changed from trunk to 3.0

Thanks for the patch.

wp_list_authors() calls get_private_posts_cap_sql() which calls get_posts_by_author_sql() with the $full parameter set to false. How is this affected by your patch?

#6 @pbearne
5 years ago

Good spot

The patch adds the missing " post_type = 'post' AND " to the returned SQL

so if you are happy I will create a new patch to remove it from the call in

foreach ( (array) $wpdb->get_results( "SELECT DISTINCT post_author, COUNT(ID) AS count FROM $wpdb->posts WHERE post_type = 'post' AND " . get_private_posts_cap_sql( 'post' ) . " GROUP BY post_author" ) as $row ) {

Currently its add and extra post_type = 'post' AND not good SQL but not broken

Thnaks

Paul

@pbearne
5 years ago

patch with unit tests and extra SQL removed from wp_list_authors()

#7 @pbearne
5 years ago

Added a patch to remove the extra SQL in wp_list_authors()

and run the Units for the function Tests_User_ListAuthors and they all Passed

paul

#8 @pbearne
5 years ago

I have had look in core code and this function is called 4 times and only once with the false option

the updated patch fixes that call

I also did a Google search for the function call and got not results so it looks safe to patch

the worse case it the we will end up with 2 "and post_type = 'post' " in the SQL

Paul

#9 @SergeyBiryukov
5 years ago

  • Keywords 4.2-early added
  • Milestone changed from Awaiting Review to Future Release

#10 @iseulde
5 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

This ticket was mentioned in Slack in #core by pbearne. View the logs.


5 years ago

#12 @boonebgorges
5 years ago

  • Keywords 4.2-early removed

pbearne - Thanks for the patches and for the tests. The logic looks good - I'm going to take this opportunity to simplify the function more generally just a bit.

As for the tests, your tests are strictly correct, but writing unit tests of this level of specificity for SQL strings is not ideal. If one little thing changes in the SQL generation - like, one extra space character is inserted - the tests will all need to be rewritten. I'm going to change the tests to be more focused, and then we'll rely on the count_user_posts() and wp_list_authors() tests (both of which functions depend on get_posts_by_author_sql()) to check that the overall syntax is correct.

#13 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 31653:

When passing $full to get_posts_by_author_sql(), make sure a 'post_type' clause is included in results.

This change makes the 'post_type' clause in wp_list_authors() redundant, so
we remove it. Third-party plugins using get_posts_by_author_sql() may have
similarly redundant clauses, but this won't change the results returned by the
SQL queries.

Also adds unit tests for get_posts_by_author_sql().

Props pbearne.
Fixes #30354.

#14 @pbearne
5 years ago

Thanks boonebgorges much better code ;-)

Note: See TracTickets for help on using tickets.