#50617 closed enhancement (fixed)
REST API: Add modified_before and modified_after to WP_REST_Request attributes
Reported by: | claytoncollie | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | REST API | Keywords: | has-patch has-dev-note |
Focuses: | Cc: |
Description
Right now there are two attributes in WP_REST_Request that allow requests to be made by the Post's published date; before
and after
.
I am proposing that modified_before
and modified_after
be added to allow for querying posts by their modified date.
Here is the list of the current attributes.
https://developer.wordpress.org/rest-api/requests/#attributes
before and after are on lines 64 and 44 of this gist.
Change History (24)
This ticket was mentioned in PR #501 on WordPress/wordpress-develop by claytoncollie.
4 years ago
#4
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
Code and tests for adding an additional query parameter to the REST API request.
Added two new arguments, modified_before
and modified_after
which request data based on the post_modified
field from the WP_Post object.
Trac ticket: https://core.trac.wordpress.org/ticket/50617
#5
@
4 years ago
- Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests removed
@SergeyBiryukov I wrote a patch for this ticket. Happy to make changes as you see fit. I do have a couple of questions.
- I am using the
post_modified
field for the request. Should I usepost_modified_gmt
instead?
https://core.trac.wordpress.org/browser/tags/5.5/src/wp-includes/class-wp-post.php#L151
WP_Query date_query defaults to post_date
so I think post_modified
is correct.
https://developer.wordpress.org/reference/classes/wp_query/#date-parameters
- I ported my changes to the Comments controller as well since it uses the same request arguments. Is that alright for the scope of this ticket?
- I wrote tests for Posts, Pages, and Attachments since they all used similar request/test methods. Is that alright for the scope of this ticket?
- Do I need to open a PR to update the documentation at https://github.com/WP-API/docs/blob/master/requests.md
#7
@
4 years ago
@TimothyBlynJacobs Hi, I was waiting for open floor call on the slack meeting on 9/10/2020, but might have missed it. I was on another work meeting at the same time. Can you and I go over this ticket at some point?
TimothyBJacobs commented on PR #501:
4 years ago
#9
Sorry for the back and forth on this @claytoncollie. WP_Date_Query
does support an array of clauses, so why can't we support searching by both columns at once actually?
claytoncollie commented on PR #501:
4 years ago
#10
@TimothyBJacobs Not a problem at all. I want to get the right so I appreciate you looking into it. I looked at WP_Date_Query
and the related tests where a person is querying by post_date and post_modified date at the same time. The first date array is built with the post_date data then a second array is created to house the post_modified data. The reason I put the conditional in my PR is that we are setting the first array $args['date_query'][0]
in both instances.
If you like, I can write some more code to support where they work individually and where they work together. This is the first pass but it feels weird. It also doesn't support when somebody is querying by after
and modified_before
at the same time. Only after and
modified_after and then
before and
modified_before`. The combinations start to get out of hand.
{{{php
Set modified_before into date query. Date query must be specified as an array of an array.
if ( isset( $registeredmodified_before?, $requestmodified_before? ) ) {
Check if we are using modified_before and before in the same query.
if ( isset( $registeredbefore?, $requestbefore? ) ) {
$argsdate_query?[1]before? = $requestmodified_before?;
$argsdate_query?[1]column? = 'post_modified';
} else {
$argsdate_query?[0]before? = $requestmodified_before?;
$argsdate_query?[0]column? = 'post_modified';
}
}
}}}
and
{{{php
Set modified_after into date query. Date query must be specified as an array of an array.
if ( isset( $registeredmodified_after?, $requestmodified_after? ) ) {
Check if we are using modified_after and after in the same query.
if ( isset( $registeredafter?, $requestafter? ) ) {
$argsdate_query?[1]after? = $requestmodified_after?;
$argsdate_query?[1]column? = 'post_modified';
} else {
$argsdate_query?[0]after? = $requestmodified_after?;
$argsdate_query?[0]column? = 'post_modified';
}
}
}}}
TimothyBJacobs commented on PR #501:
4 years ago
#11
Sorry for the delay in responding to your comment @claytoncollie.
Instead of trying to implement it in one clause, can we make each date check its own clause? Something like this?
{{{php
$date_query = array();
if ( isset( $registeredbefore?, $requestbefore? ) ) {
$date_query[] = array(
'before' => $requestbefore?,
'column' => 'post_date',
);
}
if ( isset( $registeredmodified_before?, $requestmodified_before? ) ) {
$date_query[] = array(
'before' => $requestmodified_before?,
'column' => 'post_modified',
);
}
if ( isset( $registeredafter?, $requestafter? ) ) {
$date_query[] = array(
'after' => $requestafter?,
'column' => 'post_date',
);
}
if ( isset( $registeredmodified_after?, $requestmodified_after? ) ) {
$date_query[] = array(
'after' => $requestmodified_after?,
'column' => 'post_modified',
);
}
}}}
claytoncollie commented on PR #501:
4 years ago
#12
Thanks for getting back to me @TimothyBJacobs
Your code is much more simple and builds the array properly. I updated the Post Controller with this code and it works perfectly.
I also removed the code I had for the Comments Controller as the WP_Comment class does not have a modified date as a class parameter.
The last thing I did with this PR is to add the modified_after
and modified_before
array keys to the test fixtures for testing the query params on Post, Page, and Attachments.
This ticket was mentioned in Slack in #core by timothybjacobs. View the logs.
4 years ago
#15
@
4 years ago
Capturing notes from Timothy in today's core scrub:
It has a solid patch I think, could use additional eyes from people familiar with the Query component to see if there would be any unintended ramifications from splitting one Date Query clause to 2+
cc @johnbillion @SergeyBiryukov @boonebgorges for your eyes on this one.
#16
@
4 years ago
For anybody who might be wondering, the post modified date is considered public information so it's safe to add these as new REST API query parameters. The modified date is exposed in the modified
and modified_gmt
fields in an unauthenticated REST API request, and in the <updated>
element in an Atom feed.
#17
@
4 years ago
- Keywords dev-feedback removed
The post_modified
and post_modified_gmt
fields in the database are not indexed. I've run into performance problems in the past when querying by these fields, and ended up adding a new index similar to type_status_date
that used post_modified
in place of post_date
. Worth mentioning in the dev note at least so users are aware.
With the change to the date query structure there's a slight change to the resulting SQL when querying for both before
and after
. It does not functionally alter the query. The >
and <
order is reversed and a surrounding bracket pair is removed.
Before:
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND ( ( wp_posts.post_date > '2020-01-01 00:00:00' AND wp_posts.post_date < '2020-12-31 23:59:59' ) ) AND wp_posts.post_type = 'post' AND ((wp_posts.post_status = 'publish')) ORDER BY wp_posts.post_date DESC LIMIT 0, 10
After:
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND ( wp_posts.post_date < '2020-12-31 23:59:59' AND wp_posts.post_date > '2020-01-01 00:00:00' ) AND wp_posts.post_type = 'post' AND ((wp_posts.post_status = 'publish')) ORDER BY wp_posts.post_date DESC LIMIT 0, 10
I don't think this is a problem. WordPress doesn't guarantee that its SQL queries never change.
Going to look at why the CI is failing on the PR.
#18
@
4 years ago
There were 3 failures: 1) WP_Test_REST_Attachments_Controller::test_get_items_valid_modified_date Failed asserting that actual size 0 matches expected size 1. /var/www/tests/phpunit/tests/rest-api/rest-attachments-controller.php:618 2) WP_Test_REST_Pages_Controller::test_get_items_valid_modified_date Failed asserting that actual size 0 matches expected size 1. /var/www/tests/phpunit/tests/rest-api/rest-pages-controller.php:404 3) WP_Test_REST_Posts_Controller::test_get_items_valid_modified_date Failed asserting that actual size 0 matches expected size 1. /var/www/tests/phpunit/tests/rest-api/rest-posts-controller.php:1519
@claytoncollie Wanna look into these? These look like sporadic test failures that only fail because of indeterminate sort order, but as far as I can tell those tests use specific and distinct dates.
#19
@
4 years ago
Thanks for looking at this ticket @johnbillion
I found that if I created the post with the same post_date
and post_modified
date, the test would pass. I also found that if the post_date
was some amount of time in the past compared to the post_modified
date, then my tests would fail. For this reason, I split the post creation with the post_date
value apart from the post_modified
being updated. The post_date
and post_modified
dates are now 15 days apart. After doing it this way, all my tests pass.
Probably cannot have a post_modified
value without a post_date
value in the past. Does that sound correct?
TimothyBJacobs commented on PR #501:
4 years ago
#23
#24
@
4 years ago
- Keywords has-dev-note added; needs-dev-note removed
Dev note published: https://make.wordpress.org/core/2021/02/23/rest-api-changes-in-wordpress-5-7/
Hi there, welcome to WordPress Trac! Thanks for the ticket.
This seems like a good idea, would you be interested in working on a patch?