Opened 2 years ago
Last modified 10 days ago
#17807 reviewing defect (bug)
get_adjacent_post() doesn't work with custom taxonomies
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (49)
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:5
billerickson — 2 years ago
- Cc billerickson added
billerickson — 23 months ago
comment:7
billerickson — 23 months 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' );
- 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.
billerickson — 23 months ago
comment:9
billerickson — 23 months 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
billerickson — 23 months ago
- Cc billerickson removed
comment:11
billerickson — 23 months ago
- Cc bill.erickson@… added
comment:12
billerickson — 22 months 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.
- Milestone changed from Awaiting Review to 3.3
comment:14
scribu — 22 months 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.
billerickson — 22 months ago
comment:15
billerickson — 22 months 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:16
billerickson — 22 months ago
Here's the plugin you can use to test the patch: http://wordpress.org/extend/plugins/previous-and-next-post-in-same-taxonomy/
comment:17
scribu — 21 months ago
Marked #13096 as duplicate.
comment:18
scribu — 21 months 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().
comment:19
scribu — 21 months ago
- Keywords dev-feedback removed
comment:20
scribu — 21 months ago
Related: #15959
SergeyBiryukov — 21 months ago
Updated the patch with Nacin's suggestions in ticket:13096:6 about $wpdb->prepare() and checking for taxonomy existence.
comment:22
scribu — 21 months ago
Sergey, if we were to use get_tax_sql(), we wouldn't need to worry about escaping, taxonomy existance etc.
comment:23
ethitter — 19 months 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
ethitter — 19 months 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
nacin — 19 months 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
ethitter — 19 months 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
sabreuse — 19 months ago
- Cc sabreuse@… added
comment:28
landwire — 18 months ago
- Cc sascha@… added
comment:29
ZaneMatthew — 17 months ago
- Cc zanematthew@… added
comment:30
stephenh1988 — 11 months ago
- Cc contact@… added
comment:31
ethitter — 9 months ago
- Keywords needs-testing added; 3.4-early removed
comment:32
SergeyBiryukov — 9 months ago
Related: #17302
comment:33
mikkelbreum — 9 months ago
- Cc mikkel@… added
comment:34
ryan — 9 months ago
Unit tests can be added here: http://unit-tests.trac.wordpress.org/browser/trunk/tests/url.php#L255
comment:35
husobj — 3 months ago
- Cc ben@… added
comment:36
jjeaton — 6 weeks ago
- Cc jjeaton added
comment:37
sporkme — 5 weeks ago
- Cc sporkme added
comment:38
follow-up:
↓ 41
sporkme — 5 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?
comment:39
SergeyBiryukov — 5 weeks ago
- Keywords needs-refresh added
comment:40
DrewAPicture — 4 weeks ago
- Keywords needs-refresh removed
comment:41
in reply to:
↑ 38
ethitter — 4 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.
comment:42
geminorum — 10 days 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.

As a workaround, see my Smarter Navigation plugin.