Add date query to posts endpoints #2266
Conversation
Currently I exposed everything for date parameters. Whether we want just date query or just top level or make date query params top level and then parse a date query, is up for discussion. I am up for improving this, this is a start and something that works.
|
This is for #2144, new to GitHub should have referenced that above. |
Testing to see if this fixes failed test on array assertion.
Attempt to fix testing failures by adding new params into the test files themselves. If I am not supposed to do this let me know.
Coolest part was I found out that this year is a leap year from this code and testing! Do /wp/v2/posts/?date_query[0][year]=2015&date_query[0][month]=2&date_query[0][day]=29. If it doesn't fail it's a leap year! Hooray. Added validation. This is still not error proof as I need to handle weird cases with how the top level params work, like when monthnum is specified without year. Maybe chopping top level parameter support would be nice, but would leave this incomplete so I will continue to add error handling for those strange cases.
joehoyle
changed the title
|
This is looking like a good first step @BE-Webdesign, I think we'll likely want to look at the param names, for example I think we're unlikely to match the wp_query names like Code wise, this is very neat and clear - I'd typically steer away from fixing whitespace / yoda conditions at the same time though, unless it's that piece of code you are changing. This is a good PoC and we should look to iterate on it after discussing at the next meeting in slack 👍 |
|
@joehoyle As my schedule is right now I should be able to attend the meeting and am up for helping out with any changes the team needs me to make. Thanks, for the tips I will not change the yoda conditions, makes sense. I did that in my other tax_query support so I will remove that at some point today in both of them. |
Revert yoday conditions and white spacing.
|
Per #2144 (comment) I think we're best off only supporting |
|
(Agreed with @joehoyle on the code quality though, this is a super neat PR 👍 ) |
Supporting only before and after top level now and filter[date_query] as suggested in WP-API#2144. Food for thought: if both filter[date_query] and before or after are specified filter[date_query] will override. Do we want that or do we want before after to override filter[date_query]? Let me know if I interpreted your feedback correctly or if I went in the wrong direction.
|
@rachelbaker , @joehoyle, @rmccue, @danielbachhuber , is this latest revision on the right track? |
| $params['context']['default'] = 'view'; | ||
|
|
||
| $params['after'] = array( | ||
| 'description' => __( 'The after parameter in a date query, send as an ISO8601 or strtotime() compliant date string. See wp-includes/date.php' ), |
There was a problem hiding this comment.
Accepting strtotime()-compatible values seems a bit sketchy to me, given we don't accept them anywhere else in the API and they're very much a PHP-ism.
If we change format=>date-time, then we get validation for ISO8601 for free and don't have to introduce a validation callback.
There was a problem hiding this comment.
@danielbachhuber - Excellent suggestion, will get on that.
I think so, aside from my one comment. |
Changing after and before parameter to date-time format.
|
@danielbachhuber Let me know if I need to tweak this anymore. Thanks for the feedback! I am learning a lot faster so big thanks to all of the WP-API team for helping me out! |
Can you add some tests covering both valid and invalid date parameters supplied? |
I have to learn how to do that first, but I believe I can get that done. |
First attempt at adding tests for valid and invalid before after queries.
Second attempt at adding test.
Fixed the tests not sure if they are good enough though. Finally installed PHPUnit and stuff so my PRs should be cleaner in the future! Need feedback if the tests are good enough.
|
@danielbachhuber I think this is ready to go, maybe? Before and after would need to be added to posts, pages, and media on #924. |
|
|
||
| // Set before into date query. Date query must be specified as an array of an array. | ||
| if ( isset( $request['before'] ) ) { | ||
| $args['date_query'][0]['before'] = $request['before']; |
There was a problem hiding this comment.
To prevent a PHP notice, can we create the date_query array before trying to assign the value?
| $params['context']['default'] = 'view'; | ||
|
|
||
| $params['after'] = array( | ||
| 'description' => __( 'The after parameter in a date query, send as an ISO8601 compliant date string.' ), |
There was a problem hiding this comment.
Clients don't necessarily know what a "date query" is, given this is a WordPress-ism. Also, "after parameter" and "send" are implied in the context. What about something like:
"Limit response to resources published after a given ISO8601 compliant date."
Tests look pretty good. I left a few small follow-up comments to address.
Yep, I've got a script to update that. |
|
Okay, thanks for the comments. I will make those changes now! |
Changed before, after description to be less WordPress oriented. Added empty array for $args['date_query'], to prevent PHP Notice
Add date query to posts endpoints
4 participants
Currently I exposed everything for date parameters. Whether we want
just date query or just top level or make date query params top level
and then parse a date query, is up for discussion. I am up for improving
this, this is a start and something that works.
If you want to use the top level just pass them as arguments.
GET wp/v2/posts/?year=2016&monthnum=1
For posts from January, 2016.
If you want to use date query do something like:
GET /wp/v2/posts/?date_query[0][after][year]=2001&date_query[0][before][2020]
This will return posts after the year 2001 and before 2020.