Make WordPress Core

Opened 4 years ago

Closed 2 years ago

Last modified 20 months ago

#17807 closed defect (bug) (fixed)

get_adjacent_post() doesn't work with custom taxonomies

Reported by: avaly Owned by: nacin
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.1.3
Component: Taxonomy Keywords: has-patch needs-refresh needs-codex
Focuses: Cc:


If you use next_post_link('%link', '%title', true) or previous_post_link('%link', '%title', true) to get the adjacent post for a custom post type which has a taxonomy assigned to it, it doesn't work as intended.

The bug traces back to get_adjacent_post(). If the $in_same_cat parameter is true, then the SQL query built to get the posts are hardcoded using the default 'category' taxonomy. Instead it should allow a custom taxonomy as a parameter and use it in the queries.


Custom Post Type: product

Custom Taxonomy: color

SQL produced by get_adjacent_post() when calling next_post_link('%link', '%title', true):

SELECT p.* FROM wp_posts AS p INNER JOIN wp_term_relationships AS tr ON p.ID = tr.object_id INNER JOIN wp_term_taxonomy tt ON tr.term_taxonomy_id = tt.term_taxonomy_id AND tt.taxonomy = 'category' AND tt.term_id IN () WHERE p.post_date > '2011-06-14 19:37:08' AND p.post_type = 'product' AND p.post_status = 'publish' AND tt.taxonomy = 'category' ORDER BY p.post_date ASC LIMIT 1

Attachments (14)

17807.patch (1.7 KB) - added by billerickson 4 years ago.
17807.2.patch (9.0 KB) - added by billerickson 4 years ago.
17807.3.patch (12.7 KB) - added by billerickson 4 years ago.
17807.4.patch (12.9 KB) - added by SergeyBiryukov 4 years ago.
17807.5.diff (19.2 KB) - added by ethitter 4 years ago.
17807.6.patch (19.3 KB) - added by ethitter 3 years ago.
Refreshed for 3.5
17807.5.patch (19.6 KB) - added by DrewAPicture 2 years ago.
Refresh at r24075
17807.7.patch (19.6 KB) - added by ethitter 2 years ago.
Refreshed at r24829
17807-tests.patch (3.0 KB) - added by ethitter 2 years ago.
17807.8.patch (22.7 KB) - added by ethitter 2 years ago.
17807-tests.2.patch (3.1 KB) - added by ethitter 2 years ago.
17807.9.patch (25.8 KB) - added by ethitter 2 years ago.
17807.10.patch (28.1 KB) - added by ethitter 2 years ago.
17807.11.patch (28.2 KB) - added by ethitter 2 years ago.

Download all attachments as: .zip

Change History (70)

comment:1 @avaly4 years ago

  • Version set to 3.1.3

comment:2 @scribu4 years ago

As a workaround, see my Smarter Navigation plugin.

comment:3 @avaly4 years ago

Thanks for the suggestion, scribu. Though for such a simple task as getting these links I'd prefer a clean solution without using cookies. As a temporary workaround I used direct SQL queries to get the adjacent custom posts. I hope this bug get fixed though.

comment:4 @wpmuguru4 years ago

  • Cc wpmuguru added

comment:5 @billerickson4 years ago

  • Cc billerickson added

comment:6 @griffinjt4 years ago

  • Cc thomasgriffinmedia@… added

@billerickson4 years ago

comment:7 @billerickson4 years ago

  • Keywords has-patch needs-testing added

This patch let's you use the taxonomy slug as the value of $in_same_cat instead of just true. If your value for $in_same_cat is a taxonomy, it will use that taxonomy; else it uses 'category' (so setting $in_same_cat to true still works).

So to use the taxonomy 'type', use:

previous_post_link( '%link', '← Previous', 'type' );

comment:8 @nacin4 years ago

  • Keywords needs-testing removed

Interesting idea. Not a huge fan of overloading arguments beyond their original meanings. Might be worth it to just add a new taxonomy argument. Especially since there's also an exclude_categories parameter that we could not easily convert over to terms. (It's possible for in_same_cat to be false but categories to still be excluded -- overloading the argument would prevent excluding terms of another taxonomy.)

Either way:

  • The function argument will need new docs (and a new name, if we overload it). That includes in functions that use get_adjacent_post().
  • $taxonomy should go right into the string, no need to break out of double-quoted strings in PHP.
  • Even better, $taxonomy should get $wpdb->prepare() treatment.

Would like to see an approach that uses a new argument, I think.

@billerickson4 years ago

comment:9 @billerickson4 years ago

  • Keywords needs-testing added
  • Added a new argument called $taxonomy with a default value of 'category'
  • Fixed the strings (no breaking out of double-quotes)
  • Added the $taxonomy argument to all the other functions that use get_adjacent_post()
  • Updated documentation

I also considered changing $in_same_cat to $in_same_tax, and $excluded_categories to $excluded_terms, but I wasn't sure if it was worth all the edits just to rename variables - it doesn't add any additional functionality.

I tried testing the $excluded_categories but I couldn't get them to work in trunk, without my modifications. I might be doing something wrong (I've never used that feature before). I created three categories: Category 1, Category 2, and Exclude. I created two posts in Cat 1 and Cat 2 individually, and one post with all three. When I do the below code with true or false (5 = term_id of "exclude"), the next link still leads to the post in the exclude category:

<?php next_post_link( '%link', 'Next', true, '5' ); ?>

comment:10 @billerickson4 years ago

  • Cc billerickson removed

comment:11 @billerickson4 years ago

  • Cc bill.erickson@… added

comment:12 @billerickson4 years ago

  • Keywords dev-feedback added

In #17673 the $excluded_categories bug was identified here [1557] and fixed.

Nacin (or anyone else), should I update my code to include the #17673 since it's marked as commit, or should I keep it separate?

I've written a plugin that incorporates this patch because I needed it on a client project. I'd like to publicly release it to help with testing this patch.

comment:13 @SergeyBiryukov4 years ago

  • Milestone changed from Awaiting Review to 3.3

comment:14 @scribu4 years ago

A more generic approach would be to make get_adjacent_post() use WP_Query somehow so that you could use any query var that WP_Query accepts.

@billerickson4 years ago

comment:15 @billerickson4 years ago

I've updated my patch to work with the updates from [18478]. I've also created a plugin that simulates this patch for testing, and will post that once it's approved in the repo. It allows you to use functions like be_previous_post_link() and be_get_adjacent_post().

Scribu, I personally don't think it's worth rebuilding get_adjacent_post() and all the related functions to accept query vars other than taxonomy. I think this improvement is needed, and if it other query_vars in get_adjacent_post() become needed then we can generalize it.

comment:17 @scribu4 years ago

Marked #13096 as duplicate.

comment:18 @scribu4 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

If not using WP_Query, it should be possible at least to use get_tax_sql().

Version 0, edited 4 years ago by scribu (next)

comment:19 @scribu4 years ago

  • Keywords dev-feedback removed

comment:20 @scribu4 years ago

Related: #15959

@SergeyBiryukov4 years ago

comment:21 @SergeyBiryukov4 years ago

Updated the patch with Nacin's suggestions in ticket:13096:6 about $wpdb->prepare() and checking for taxonomy existence.

comment:22 @scribu4 years ago

Sergey, if we were to use get_tax_sql(), we wouldn't need to worry about escaping, taxonomy existance etc.

@ethitter4 years ago

comment:23 @ethitter4 years ago

  • Cc ehitter@… added
  • Keywords has-patch added; needs-patch removed

Building on what SergeyBiryukov had started, I cleaned up his usage of $wpdb->prepare().

In get_boundary_post(), I also changed how the get_posts() query arguments were built for situations where $excluded_terms was empty string that was converted to an array with single key whose value was 0.

Since get_boundary_post_rel_link() has been deprecated since SergeyBiryukov wrote his patch, I left it alone.

Finally, I cleaned up the functions I modified per the coding standards.

comment:24 @ethitter4 years ago

Forgot to mention that I explored using get_tax_sql() as scribu suggested, but because of how the get_adjacent_post() function builds its query, the usefulness of get_tax_sql() was too limited to make it worthwhile.

comment:25 @nacin4 years ago

  • Keywords 3.4-early added
  • Milestone changed from 3.3 to Future Release
  • Owner set to nacin
  • Status changed from new to reviewing

Nice. I meant to punt this out of the 3.3 milestone a while ago, so let's move this to 3.4-early. otto42 and I nearly tackled this a few weeks ago as we needed it for a project, but we realized it was just way too much churn (even minus the coding standards changes) for 3.3 at this point.

I discussed possibly leveraging WP_Query with ethitter at #wcphilly and now I'm curious if it's possible. Can anyone see something that would prevent us from dropping our direct queries here, beyond needing to eliminate the existing filters on the SQL?

comment:26 @ethitter4 years ago

Being somewhat invested in this already, I'll work on the WP_Query changes discussed at #wcphilly. If I can find something else 3.3-related, I'll wait on this until after that's out, otherwise I'll work on this in the meantime.

comment:27 @sabreuse4 years ago

  • Cc sabreuse@… added

comment:28 @landwire4 years ago

  • Cc sascha@… added

comment:29 @ZaneMatthew4 years ago

  • Cc zanematthew@… added

comment:30 @stephenh19883 years ago

  • Cc contact@… added

@ethitter3 years ago

Refreshed for 3.5

comment:31 @ethitter3 years ago

  • Keywords needs-testing added; 3.4-early removed

comment:33 @mikkelbreum3 years ago

  • Cc mikkel@… added

comment:35 @husobj3 years ago

  • Cc ben@… added

comment:36 @jjeaton2 years ago

  • Cc jjeaton added

comment:37 @sporkme2 years ago

  • Cc sporkme added

comment:38 follow-up: @sporkme2 years ago

Last version of the patch fails on 3.5.1. Can we have an updated patch while waiting?

How are others working around this when using custom post types and custom taxonomies?

comment:39 @SergeyBiryukov2 years ago

  • Keywords needs-refresh added

@DrewAPicture2 years ago

Refresh at r24075

comment:40 @DrewAPicture2 years ago

  • Keywords needs-refresh removed

comment:41 in reply to: ↑ 38 @ethitter2 years ago

Replying to sporkme:

How are others working around this when using custom post types and custom taxonomies?

There are filters in get_adjacent_post(), the underlying function that powers the adjacent post link functions. See http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/link-template.php#L1173.

Perhaps 3.7 will be the release we can land this for. Unfortunately, until #18694 lands, we can't replace the direct SQL queries with WP_Query.

comment:42 @geminorum2 years ago

The dilemma would have been inverted, if a simple filter could override the entire query making process.

Also there are problems ordering only by post_date. we had to rewrite all the functions to just order custom post types by anything beside date.

Either way please don't forget to pass $taxonomy through all the filters.

comment:43 @RubenBristian2 years ago

#24794 was marked as a duplicate.

@ethitter2 years ago

Refreshed at r24829

comment:44 @ethitter2 years ago

17807.7.patch is updated for WP 3.6 and applies cleanly as of r24829. All is well in my testing, and I'm working on unit tests to confirm this now.

comment:45 @wonderboymusic2 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Future Release to 3.7

@ethitter2 years ago

comment:46 @ethitter2 years ago

I worked on tests for this, but either I'm doing something wrong or my PHPUnit install is borked; this is my first attempt at writing unit tests, so the former is quite likely. 17807-tests.patch reflects how far I got.

comment:47 @nacin2 years ago

  • Keywords needs-refresh added; needs-testing needs-unit-tests removed
  • Milestone changed from 3.7 to Future Release

Patch needs to be refreshed. (And is preferably made against develop.svn, with the tests in the same patch.) I'm happy to help shore the tests up for this one, and get this in probably into early 3.8.

@ethitter2 years ago

@ethitter2 years ago

@ethitter2 years ago

comment:48 @ethitter2 years ago

17807.9.patch is refreshed against develop.svn and combines the patch and tests.

The tests are passing now as well. I had get_post() where I meant get_posts(). :(

Last edited 2 years ago by ethitter (previous) (diff)

@ethitter2 years ago

@ethitter2 years ago

comment:49 @nacin2 years ago

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

In 25959:

Add a $taxonomy argument to each of the adjacent post functions.

Each took an array of category (IDs) when to search. Those can now be term IDs and each function now has $taxonomy = 'category' as an optional argument.

Functions affected: get_previous_post(), get_next_post(), get_adjacent_post(), get_adjacent_post_rel_link(), adjacent_posts_rel_link(), next_post_rel_link(), prev_post_rel_link(), get_boundary_post(), get_previous_post_link(), previous_post_link(), get_next_post_link(), next_post_link(), get_adjacent_post_link(), adjacent_post_link().

props ethitter.
finally fixes #17807.

comment:50 @helen2 years ago

  • Milestone changed from Future Release to 3.8

comment:51 follow-ups: @husobj21 months ago

Just a note, the documentation for affected functions needs updating in the codex.

comment:52 in reply to: ↑ 51 ; follow-up: @DrewAPicture21 months ago

  • Keywords needs-codex added

Replying to husobj:

Just a note, the documentation for affected functions needs updating in the codex.

FYI, the Codex is a wiki, and anyone with a WordPress.org account can edit it, even yourself if you're so inclined :)

comment:53 in reply to: ↑ 51 @ZaneMatthew21 months ago

Replying to husobj:

Just a note, the documentation for affected functions needs updating in the codex.

I made a very minor adjustment, and added it.

comment:54 in reply to: ↑ 52 @husobj21 months ago

Replying to DrewAPicture:

Replying to husobj:

Just a note, the documentation for affected functions needs updating in the codex.

FYI, the Codex is a wiki, and anyone with a WordPress.org account can edit it, even yourself if you're so inclined :)

Thanks DrewAPicture, yes I did know it's a wiki.
I just didn't have any time to make any updates yesterday and wanted to make a note so it didn't get forgotten :)

comment:55 follow-up: @bucketpress20 months ago

Does this solve the issue where we need to get adjacent posts based on post order other than post date?

comment:56 in reply to: ↑ 55 @ethitter20 months ago

Replying to bucketpress:

Does this solve the issue where we need to get adjacent posts based on post order other than post date?

No, but #26937 addresses that by switching to WP_Query. Currently only the filters available within get_adjacent_post() can be used to change the order.

Note: See TracTickets for help on using tickets.