-
Notifications
You must be signed in to change notification settings - Fork 3.2k
get_adjacent_post: adjacent post query plugin compatibility #10620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
get_adjacent_post: adjacent post query plugin compatibility #10620
Conversation
…ery hasn't been modified. This update introduces a deterministic fallback for the SQL `WHERE` and `ORDER BY` clauses in the `get_adjacent_post` function when posts have identical dates. The fallback is applied only if the respective clauses have not been modified by filters, ensuring consistent behavior. Unit tests have been added to verify the correct application of this fallback under various conditions, including scenarios where filters are applied or not. See Trac ticket https://core.trac.wordpress.org/ticket/64390.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a few notes inline.
|
|
||
| remove_all_filters( 'get_next_post_where' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test suite does this automatically so it can be removed.
🔢 Applies in the tests below so I won't repeat myself.
| remove_all_filters( 'get_next_post_where' ); |
| // Next post should be the 4th post (higher ID, same date) - deterministic. | ||
| $next = get_adjacent_post( false, '', false ); | ||
| $this->assertInstanceOf( 'WP_Post', $next ); | ||
| $this->assertEquals( $post_ids[3], $next->ID ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $this->assertEquals( $post_ids[3], $next->ID ); | |
| $this->assertSame( $post_ids[3], $next->ID ); |
🔢 Looks to apply elsewhere so I won't repeat myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
|
|
||
| // Only force deterministic fallback if the where clause has not been modified by a filter. | ||
| if ( $where === $where_prepared ) { | ||
| $where = $where_prepared_with_deterministic_fallback; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason you're defining $where_prepared_with_deterministic_fallback above rather than here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to reduce my carbon footprint by limiting my Ctrl+V and Ctrl+P ![]()
No reason at all actually. I can move.
| * @param string $order Sort order. 'DESC' for previous post, 'ASC' for next. | ||
| */ | ||
| $sort = apply_filters( "get_{$adjacent}_post_sort", "ORDER BY p.post_date $order, p.ID $order LIMIT 1", $post, $order ); | ||
| $sort_prepared = "ORDER BY p.post_date $order LIMIT 1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be done above the docblock so docs-parsers can figure out the filter it applies to.
| * @since 4.4.0 Added the `$taxonomy` and `$post` parameters. | ||
| * @since 6.9.0 Adds ID-based fallback for posts with identical dates in adjacent post queries. | ||
| * | ||
| * @param string $where The `WHERE` clause in the SQL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yours truly missed this last time.
| * @param string $where The `WHERE` clause in the SQL. | |
| * @param string $where_prepared The `WHERE` clause in the SQL. |
| * Can either be next or previous post. | ||
| * | ||
| * @since 2.5.0 | ||
| * @since 6.9.1 Adds deterministic fallback for sort clause if not modified by a filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably include the since changes for 6.0.0 that yours truly also missed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are they? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this?
| * @since 6.9.1 Adds deterministic fallback for sort clause if not modified by a filter. | |
| * @since 6.9.0 Introduce deterministic fallback based in IDs to account for date collisions. | |
| * @since 6.9.1 Remove deterministic fallback for sites modifying the WHERE clause via a filter. | |
| * See https://core.trac.wordpress.org/ticket/64390 |
| * @since 2.5.0 | ||
| * @since 4.4.0 Added the `$post` parameter. | ||
| * @since 4.9.0 Added the `$order` parameter. | ||
| * @since 6.9.0 Adds ID sort to ensure deterministic ordering for posts with identical dates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it best to keep this and then not that it was removed in 6.9.1 and applied later if changed by a filter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah great thanks, I was wondering about that. Cheers
| * Can either be next or previous post. | ||
| * | ||
| * @since 2.5.0 | ||
| * @since 6.9.1 Adds deterministic fallback for sort clause if not modified by a filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure whether to also add a 6.9.0 entry for historical purposes
| * | ||
| * @since 2.5.0 | ||
| * @since 4.4.0 Added the `$taxonomy` and `$post` parameters. | ||
| * @since 6.9.0 Adds ID-based fallback for posts with identical dates in adjacent post queries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This okay to remove, or should we have some ref to historical changes?
|
Thanks @peterwilsoncc I'll fix those things up next week. |
Follow up to #10394
Tests out an idea suggested in https://core.trac.wordpress.org/ticket/64390#comment:10 Props @azaozz
Note
This is mainly to discuss whether it's worth the change, if so good, if not, I can close.
Description
This PR is a follow-up to #10394 that makes the deterministic ordering changes more defensive and plugin-friendly. It ensures that when plugins modify the
get_{$adjacent}_post_whereorget_{$adjacent}_post_sortfilters, the deterministic logic is not applied on top of their modifications.Problem
In PR #10394, deterministic ID-based ordering was added to fix adjacent post navigation for posts with identical dates. However, the implementation applied the deterministic logic before the filters ran, which meant:
Solution
This PR implements a defensive approach that:
Implementation:
$where === $where_prepared(filter didn't modify it)$sort === $sort_prepared(filter didn't modify it)Test Steps
Automated Tests
Run the PHPUnit tests:
Manual Testing
1. Verify Default Behavior (No Filters)
Create test scenario:
post_datevalues (bulk publish drafts)Expected Results:
2. Verify Filter Modifications Are Respected
Test WHERE Filter:
functions.phpor a test plugin:AND p.IDin WHERE clause)Test SORT Filter:
3. Verify Unmodified Filters Still Get Deterministic Logic
Trac ticket: https://core.trac.wordpress.org/ticket/64390