Skip to content

Add dates_are_gmt parameter to REST API #28132

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.

Sign up for GitHub

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

Jump to bottom
Merged
Konamiman merged 2 commits into from
May 12, 2021
Merged

Add dates_are_gmt parameter to REST API #28132

Konamiman merged 2 commits into from
May 12, 2021

Conversation

@TimBHowe
Copy link
Contributor

@TimBHowe TimBHowe commented Oct 28, 2020 •
edited by Konamiman
Loading

All Submissions:

Changes proposed in this Pull Request:

Added the dates_are_gmt parameters to allow the before and after parameters to use the post_date_gmt column if it is true.

Closes #28111

Related PR in documentation repository: woocommerce/woocommerce-rest-api-docs#203

How to test the changes in this Pull Request:

  1. Install WordPress/WooCommerce with testing data
  2. Generate some API Keys
  3. Make a REST API request to the 'https://localhost.com/wp-json/wc/v2/orders' end point and use the dates_are_gmt parameters to allow the before and after parameters to use the post_date_gmt column if it is true.
    Example: https://localhost.com/wp-json/wc/v3/orders?dates_are_gmt=true&after=2021-04-24T06:00:00&before=2021-04-24T14:00:00&consumer_key=xxx&consumer_secret=xxx

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Added the dates_are_gmt parameters to REST API to searched posts using the post_date_gmt column.

@TimBHowe TimBHowe marked this pull request as ready for review October 29, 2020 00:35
@TimBHowe TimBHowe changed the title … Added before_gmt and before_gmt parameters to REST API Oct 29, 2020
@TimBHowe TimBHowe changed the title Added before_gmt and before_gmt parameters to REST API Added before_gmt and after_gmt parameters to REST API Oct 30, 2020
Base automatically changed from master to trunk February 25, 2021 22:17
@vedanshujain vedanshujain requested review from a team and Konamiman and removed request for a team April 16, 2021 14:41
@Konamiman
Copy link
Contributor

Konamiman commented Apr 19, 2021

Hi @TimBHowe, thanks for your contribution. The problem I see with your approach is that it makes before/after and before/after_gmt arguments mutually exclusive, if one is applied then the other will necessarily be ignored and which one will "win" is accidental based on the order of their processing in the code.

What if instead you add a new bool dates_are_gmt argument? So that setting it to true will cause the existing before and after arguments to be interpreted as gmt dates.

Additionally, any addition or change in request arguments needs to be accompanied by the corresponding update in the get_collection_params method in the same class.

@Konamiman Konamiman added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Apr 27, 2021
@TimBHowe
Copy link
Contributor Author

TimBHowe commented May 6, 2021

Updated the pull requests as requested.

Added the dates_are_gmt parameters to allow the before and after parameters to use the post_date_gmt column if it is true.

Also added the new parameter to get_collection_params function in the class.

The new example request would look something like this
https://localhost.com/wp-json/wc/v3/orders?dates_are_gmt=true&after=2021-04-24T06:00:00&before=2021-04-24T14:00:00&consumer_key=xxx&consumer_secret=xxx

@github-actions github-actions bot added needs: triage feedback Issues for which we requested feedback from the author and received it. and removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. labels May 6, 2021
Konamiman
Konamiman approved these changes May 11, 2021
View reviewed changes
Copy link
Contributor

@Konamiman Konamiman left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm approving. Could you please update the pull request title and description? They're still referring to the old before_gmt and after_gmt parameters. I'll merge afterwards.

@TimBHowe
Copy link
Contributor Author

TimBHowe commented May 11, 2021

Updated the description of the pull request.

Let me know if you need any other changes.

@Konamiman
Copy link
Contributor

Konamiman commented May 12, 2021

It's all good now, merging.

@Konamiman Konamiman merged commit d2da61a into woocommerce:trunk May 12, 2021
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels May 12, 2021
@Konamiman Konamiman added this to the 5.4.0 milestone May 12, 2021
@Konamiman Konamiman added REST/Store API/Webhooks Issues related to WooCommerce REST API. and removed needs: triage feedback Issues for which we requested feedback from the author and received it. labels May 12, 2021
@Konamiman Konamiman changed the title Added before_gmt and after_gmt parameters to REST API Add dates_are_gmt parameter to REST API May 12, 2021
@Konamiman Konamiman mentioned this pull request May 12, 2021
Merged
@jonathansadowski jonathansadowski mentioned this pull request May 14, 2021
Merged
6 tasks
@ObliviousHarmony ObliviousHarmony added changelog added and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels May 14, 2021
@juliaamosova juliaamosova added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels May 16, 2021
@Konamiman Konamiman mentioned this pull request Aug 26, 2021
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Konamiman Konamiman Konamiman approved these changes

Assignees

No one assigned

Labels

REST/Store API/Webhooks Issues related to WooCommerce REST API.

Projects

None yet

Milestone

5.4.0

Development

Successfully merging this pull request may close these issues.

WooCommerce Fails to Recognize Timezone

5 participants

@TimBHowe @Konamiman @ObliviousHarmony @juliaamosova @woocommercebot