Opened 2 years ago

Last modified 10 days ago

#17807 reviewing defect (bug)

get_adjacent_post() doesn't work with custom taxonomies

Reported by: avaly Owned by: nacin
Priority: normal Milestone: Future Release
Component: Taxonomy Version: 3.1.3
Severity: normal Keywords: has-patch needs-testing
Cc: wpmuguru, thomasgriffinmedia@…, bill.erickson@…, ehitter@…, sabreuse@…, sascha@…, zanematthew@…, contact@…, mikkel@…, ben@…, jjeaton, sporkme

Description

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.

Example:

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

17807.patch (1.7 KB) - added by billerickson 23 months ago.
17807.2.patch (9.0 KB) - added by billerickson 23 months ago.
17807.3.patch (12.7 KB) - added by billerickson 22 months ago.
17807.4.patch (12.9 KB) - added by SergeyBiryukov 21 months ago.
17807.5.diff (19.2 KB) - added by ethitter 19 months ago.
17807.6.patch (19.3 KB) - added by ethitter 9 months ago.
Refreshed for 3.5
17807.5.patch (19.6 KB) - added by DrewAPicture 4 weeks ago.
Refresh at r24075

Download all attachments as: .zip

Change History (49)

  • Version set to 3.1.3

As a workaround, see my Smarter Navigation plugin.

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.

  • Cc wpmuguru added
  • Cc billerickson added
  • Cc thomasgriffinmedia@… added
  • 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' );

  • 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.

  • 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' ); ?>

  • Cc billerickson removed
  • Cc bill.erickson@… added
  • 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.

  • Milestone changed from Awaiting Review to 3.3

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.

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.

Marked #13096 as duplicate.

  • 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 21 months ago by scribu (next)
  • Keywords dev-feedback removed

Related: #15959

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

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

  • 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.

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.

  • 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?

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.

  • Cc sabreuse@… added
  • Cc sascha@… added
  • Cc zanematthew@… added
  • Cc contact@… added

Refreshed for 3.5

  • Keywords needs-testing added; 3.4-early removed
  • Cc mikkel@… added
  • Cc ben@… added
  • Cc jjeaton added
  • Cc sporkme added

comment:38 follow-up: ↓ 41   sporkme5 weeks 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?

  • Keywords needs-refresh added

Refresh at r24075

  • Keywords needs-refresh removed

comment:41 in reply to: ↑ 38   ethitter4 weeks 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.

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.

Note: See TracTickets for help on using tickets.