Skip to content
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

Fix: update REST API v3 reports/orders/totals endpoint to be compatible with HPOS #46715

Merged
merged 5 commits into from Apr 18, 2024

Conversation

lsinger
Copy link
Contributor

@lsinger lsinger commented Apr 18, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

This PR updates the REST API v3 reports/orders/totals endpoint to be compatible with HPOS.

Closes woocommerce/woocommerce-ios#12468.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. checkout the branch locally and then move HEAD to the first commit that adds the failing test
  2. run the test, observe that it fails: pnpm test:php:env --filter=WC_Tests_API_Reports_Orders_Totals
  3. move HEAD to the tip of this branch again
  4. run the test, observe that it passes: pnpm test:php:env --filter=WC_Tests_API_Reports_Orders_Totals
  5. ensure CI checks pass

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@lsinger lsinger added focus: rest api Issues related to WooCommerce REST API. focus: custom order tables / HPOS Issues related to High-Performance Order Storage (HPOS) née Custom Order Tables. plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Apr 18, 2024
@lsinger lsinger self-assigned this Apr 18, 2024
Copy link
Contributor

Hi @jorgeatorres,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@lsinger lsinger force-pushed the fix/update-rest-api-order-totals-hpos branch from 4d7bbf7 to 97fd247 Compare April 18, 2024 09:56
@jorgeatorres
Copy link
Member

Just for completeness, some instructions to test outside of the test suite:

  1. Select "WordPress post storage" as datastore in WC > Settings > Advanced > Features. Compatibility mode should be disabled.
  2. Populate the site with some orders (i.e. via wp wc generate orders).
  3. Use the API client of your choice to make a GET request to https://<yoursite>/wp-json/wc/v3/reports/orders/totals.
  4. Take note of the results.
  5. Run wp cot sync to sync orders to HPOS.
  6. Select "High-performance order storage" as datastore in WC > Settings > Advanced > Features. Keep compatibility mode disabled.
  7. Repeat step 3 and confirm the results are identical to those from step 4.

Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

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

LGTM. Works as described. Thank you @lsinger!

@jorgeatorres jorgeatorres force-pushed the fix/update-rest-api-order-totals-hpos branch from 97fd247 to 3810071 Compare April 18, 2024 12:09
@lsinger lsinger force-pushed the fix/update-rest-api-order-totals-hpos branch from 3810071 to 1fd92d3 Compare April 18, 2024 15:15
@lsinger lsinger enabled auto-merge (squash) April 18, 2024 15:23
@lsinger lsinger merged commit 2d837ac into trunk Apr 18, 2024
22 of 23 checks passed
@lsinger lsinger deleted the fix/update-rest-api-order-totals-hpos branch April 18, 2024 15:49
@github-actions github-actions bot added this to the 8.9.0 milestone Apr 18, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Apr 18, 2024
nielslange pushed a commit that referenced this pull request Apr 20, 2024
…le with HPOS (#46715)

* add a failing test for order totals with HPOS enabled but sync disabled

* fix comment

* ensure the wc/v3/reports/orders/totals endpoint is compatible with HPOS

* add changelog file

* address linter issues
@rodelgc rodelgc added status: analysis complete Indicates if a PR has been analysed by Solaris contains: rest api change Indicates if the PR contains a REST API change. needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. impact: high Indicates if the PR has been determined as high impact. needs: internal testing Indicates if the PR requires further testing conducted by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Apr 22, 2024
samueljseay pushed a commit that referenced this pull request Apr 29, 2024
…le with HPOS (#46715)

* add a failing test for order totals with HPOS enabled but sync disabled

* fix comment

* ensure the wc/v3/reports/orders/totals endpoint is compatible with HPOS

* add changelog file

* address linter issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contains: rest api change Indicates if the PR contains a REST API change. focus: custom order tables / HPOS Issues related to High-Performance Order Storage (HPOS) née Custom Order Tables. focus: rest api Issues related to WooCommerce REST API. impact: high Indicates if the PR has been determined as high impact. needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. needs: internal testing Indicates if the PR requires further testing conducted by Solaris plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orders Badge does not sync on iOS when HPOS is enabled
3 participants