Make WordPress Core

Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#17807 closed defect (bug) (fixed)

get_adjacent_post() doesn't work with custom taxonomies

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

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

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

Download all attachments as: .zip

Change History (70)

#1 @avaly
13 years ago

  • Version set to 3.1.3

#2 @scribu
13 years ago

As a workaround, see my Smarter Navigation plugin.

#3 @avaly
13 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.

#4 @wpmuguru
13 years ago

  • Cc wpmuguru added

#5 @billerickson
13 years ago

  • Cc billerickson added

#6 @griffinjt
13 years ago

  • Cc thomasgriffinmedia@… added

@billerickson
13 years ago

#7 @billerickson
13 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' );

#8 @nacin
13 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.

#9 @billerickson
13 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' ); ?>

#10 @billerickson
13 years ago

  • Cc billerickson removed

#11 @billerickson
13 years ago

  • Cc bill.erickson@… added

#12 @billerickson
13 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.

#13 @SergeyBiryukov
13 years ago

  • Milestone changed from Awaiting Review to 3.3

#14 @scribu
13 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.

#15 @billerickson
13 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.

#17 @scribu
13 years ago

Marked #13096 as duplicate.

#18 @scribu
13 years ago

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

If not using WP_Query, we should at least use get_tax_sql().

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

#19 @scribu
13 years ago

  • Keywords dev-feedback removed

#20 @scribu
13 years ago

Related: #15959

#21 @SergeyBiryukov
13 years ago

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

#22 @scribu
13 years ago

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

@ethitter
13 years ago

#23 @ethitter
13 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.

#24 @ethitter
13 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.

#25 @nacin
13 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?

#26 @ethitter
13 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.

#27 @sabreuse
13 years ago

  • Cc sabreuse@… added

#28 @landwire
13 years ago

  • Cc sascha@… added

#29 @ZaneMatthew
13 years ago

  • Cc zanematthew@… added

#30 @stephenh1988
12 years ago

  • Cc contact@… added

@ethitter
12 years ago

Refreshed for 3.5

#31 @ethitter
12 years ago

  • Keywords needs-testing added; 3.4-early removed

#33 @mikkelbreum
12 years ago

  • Cc mikkel@… added

#35 @husobj
12 years ago

  • Cc ben@… added

#36 @jjeaton
11 years ago

  • Cc jjeaton added

#37 @sporkme
11 years ago

  • Cc sporkme added

#38 follow-up: @sporkme
11 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?

#39 @SergeyBiryukov
11 years ago

  • Keywords needs-refresh added

@DrewAPicture
11 years ago

Refresh at r24075

#40 @DrewAPicture
11 years ago

  • Keywords needs-refresh removed

#41 in reply to: ↑ 38 @ethitter
11 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.

#42 @geminorum
11 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.

#43 @RubenBristian
11 years ago

#24794 was marked as a duplicate.

@ethitter
11 years ago

Refreshed at r24829

#44 @ethitter
11 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.

#45 @wonderboymusic
11 years ago

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

#46 @ethitter
11 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.

#47 @nacin
11 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.

@ethitter
11 years ago

@ethitter
11 years ago

#48 @ethitter
11 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 11 years ago by ethitter (previous) (diff)

@ethitter
11 years ago

@ethitter
11 years ago

#49 @nacin
11 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.

#50 @helen
11 years ago

  • Milestone changed from Future Release to 3.8

#51 follow-ups: @husobj
11 years ago

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

#52 in reply to: ↑ 51 ; follow-up: @DrewAPicture
11 years 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 :)

#53 in reply to: ↑ 51 @ZaneMatthew
11 years 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.

#54 in reply to: ↑ 52 @husobj
11 years 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 :)

#55 follow-up: @bucketpress
11 years ago

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

#56 in reply to: ↑ 55 @ethitter
11 years 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.