Reduce get_post_meta calls in orders
#61148
Conversation
github-actions
bot
added
the
plugin: woocommerce
woocommercebot
requested review from
a team and
prettyboymp
and removed request for
a team
Testing GuidelinesHi @prettyboymp , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughConsolidates WooCommerce order/refund data loading to a single bulk get_post_meta call in CPT data stores, replacing multiple per-key lookups. Updates internal property assignment accordingly. Adds tests validating order and refund property persistence. Adds a changelog entry documenting the performance optimization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Caller (wc_get_order / load)
participant Store as WC_*_Data_Store_CPT
participant WP as WordPress Meta API
Client->>Store: read_order_data(order_id)
Store->>WP: get_post_meta(order_id) // bulk fetch
WP-->>Store: post_meta map
rect rgb(230,245,255)
note over Store: Assign props from post_meta[...] with defaults
Store->>Store: set_props({...})
end
Store-->>Client: Order/Refund object populated
note over Client,Store: Replaces many per-key get_post_meta calls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php (2)
180-181: Docblock nit (optional).Consider standardizing the
@paramannotations (types first) for consistency with surrounding docs.
426-451: Normalize prices_include_tax with wc_string_to_bool
Replace the raw'yes' ===checks inabstract-wc-order-data-store-cpt.phpto usewc_string_to_bool()for both the meta value and the option fallback:- $prices_include_tax = $meta_data['_prices_include_tax'][0] ?? ''; + $prices_include_tax_raw = $meta_data['_prices_include_tax'][0] ?? null; ... - 'prices_include_tax' => metadata_exists( 'post', $id, '_prices_include_tax' ) ? 'yes' === $prices_include_tax : 'yes' === get_option( 'woocommerce_prices_include_tax' ), + 'prices_include_tax' => null !== $prices_include_tax_raw + ? wc_string_to_bool( $prices_include_tax_raw ) + : wc_string_to_bool( get_option( 'woocommerce_prices_include_tax' ) ),No other per-key
get_post_meta( $id, … )usages remain in the order data stores.plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.php (1)
142-182: Normalize boolean-like meta on readA few operational flags are loaded as raw strings. Convert all boolean-like meta to bools on read to keep object state typed and consistent with update_post_meta behavior and DS getters.
Apply:
// Operational data props. - 'order_stock_reduced' => $post_meta['_order_stock_reduced'][0] ?? '', - 'download_permissions_granted' => $post_meta['_download_permissions_granted'][0] ?? '', - 'new_order_email_sent' => $post_meta['_new_order_email_sent'][0] ?? '', - 'recorded_sales' => wc_string_to_bool( $post_meta['_recorded_sales'][0] ?? '' ), - 'recorded_coupon_usage_counts' => $post_meta['_recorded_coupon_usage_counts'][0] ?? '', + 'order_stock_reduced' => wc_string_to_bool( $post_meta['_order_stock_reduced'][0] ?? '' ), + 'download_permissions_granted' => wc_string_to_bool( $post_meta['_download_permissions_granted'][0] ?? '' ), + 'new_order_email_sent' => wc_string_to_bool( $post_meta['_new_order_email_sent'][0] ?? '' ), // DB stores 'true'|'false'; normalize to bool. + 'recorded_sales' => wc_string_to_bool( $post_meta['_recorded_sales'][0] ?? '' ), + 'recorded_coupon_usage_counts' => wc_string_to_bool( $post_meta['_recorded_coupon_usage_counts'][0] ?? '' ),This avoids stringly-typed truthiness and matches DS getters (e.g., get_email_sent, get_stock_reduced). As per coding guidelines.
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php (2)
1-1: Add strict types declarationPlease add strict_types for consistency across tests.
<?php +declare( strict_types=1 );As per coding guidelines.
643-737: Comprehensive end‑to‑end read verificationThorough coverage of identity, addresses, payment, dates, notes, and permissions. One minor ask: dates are asserted via formatted strings; if test env timezone varies, this can be brittle. Consider asserting timestamps (getTimestamp()) to avoid TZ flakiness.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
plugins/woocommerce/changelog/performance-60706-get-post-meta(1 hunks)plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php(2 hunks)plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.php(1 hunks)plugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.php(1 hunks)plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.phpplugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.phpplugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.phpplugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit configuration file
**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.phpplugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.phpplugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.phpplugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php
plugins/woocommerce/tests/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/woo-phpunit.mdc)
plugins/woocommerce/tests/**/*.php: Ensure test classes define public setUp() and tearDown() methods (not protected)
All PHPUnit test methods must be public, not protected
Add declare(strict_types=1); at the top of each test file
Test classes should extend WC_Unit_Test_Case
Files:
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php
plugins/woocommerce/tests/php/**
📄 CodeRabbit inference engine (plugins/woocommerce/CLAUDE.md)
Store PHP test data and fixtures under tests/php/
Files:
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php
🧠 Learnings (5)
📓 Common learnings
Learnt from: chihsuan
PR: woocommerce/woocommerce#58744
File: plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.php:145-149
Timestamp: 2025-08-20T05:55:28.772Z
Learning: In WooCommerce, the add_refund_type_meta function in the Products DataStore is called by woocommerce_order_partially_refunded and woocommerce_order_fully_refunded hooks, so the refund order is guaranteed to exist when these methods are invoked, making null checks for wc_get_order() unnecessary in this context.
📚 Learning: 2025-08-20T05:55:28.772Z
Learnt from: chihsuan
PR: woocommerce/woocommerce#58744
File: plugins/woocommerce/src/Admin/API/Reports/Products/DataStore.php:145-149
Timestamp: 2025-08-20T05:55:28.772Z
Learning: In WooCommerce, the add_refund_type_meta function in the Products DataStore is called by woocommerce_order_partially_refunded and woocommerce_order_fully_refunded hooks, so the refund order is guaranteed to exist when these methods are invoked, making null checks for wc_get_order() unnecessary in this context.
Applied to files:
plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.phpplugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.phpplugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.phpplugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php
📚 Learning: 2025-08-18T06:11:48.768Z
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : Ensure test classes define public setUp() and tearDown() methods (not protected)
Applied to files:
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php
📚 Learning: 2025-08-18T06:11:48.768Z
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-08-18T06:11:48.768Z
Learning: Applies to plugins/woocommerce/tests/**/*.php : All PHPUnit test methods must be public, not protected
Applied to files:
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php
📚 Learning: 2025-09-11T07:18:56.840Z
Learnt from: CR
PR: woocommerce/woocommerce#0
File: plugins/woocommerce/CLAUDE.md:0-0
Timestamp: 2025-09-11T07:18:56.840Z
Learning: Applies to plugins/woocommerce/tests/php/src/Internal/Admin/Suggestions/**/*.php : Place unit tests for payment extension suggestions in tests/php/src/Internal/Admin/Suggestions/
Applied to files:
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php
🧬 Code graph analysis (4)
plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.php (1)
plugins/woocommerce/includes/wc-formatting-functions.php (1)
wc_string_to_bool(25-28)
plugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.php (1)
plugins/woocommerce/includes/wc-formatting-functions.php (1)
wc_string_to_bool(25-28)
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php (4)
plugins/woocommerce/includes/abstracts/abstract-wc-order.php (6)
set_currency(702-707)set_cart_tax(786-789)set_prices_include_tax(715-717)get_currency(332-334)get_version(342-344)get_prices_include_tax(352-354)plugins/woocommerce/includes/wc-order-functions.php (1)
wc_get_order(88-94)plugins/woocommerce/includes/class-wc-order.php (19)
set_order_key(1228-1230)set_customer_id(1238-1240)set_payment_method(1476-1486)set_payment_method_title(1494-1496)set_transaction_id(1504-1506)set_customer_ip_address(1514-1516)set_customer_user_agent(1524-1526)set_created_via(1534-1536)set_date_completed(1554-1556)set_date_paid(1564-1566)set_cart_hash(1574-1576)set_customer_note(1544-1546)get_payment_method(863-865)get_payment_method_title(873-875)get_transaction_id(883-885)get_customer_ip_address(893-895)get_customer_user_agent(903-905)get_created_via(913-915)get_customer_note(923-925)plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.php (3)
set_download_permissions_granted(695-701)get_order_key(403-409)get_download_permissions_granted(684-687)
plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php (1)
plugins/woocommerce/includes/abstracts/abstract-wc-data.php (1)
get_extra_data_keys(292-294)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
plugins/woocommerce/changelog/performance-60706-get-post-meta (1)
1-4: Changelog looks good.Entry is clear and correctly categorized.
plugins/woocommerce/includes/data-stores/class-wc-order-data-store-cpt.php (2)
125-131: Bulk meta fetch is a solid winSingle get_post_meta call reduces DB round-trips and aligns with the caching you prime elsewhere. LGTM.
129-138: Legacy date fallbacks handled correctlyGraceful fallback from date* to legacy *_date keys keeps BC while moving forward with timestamps. LGTM.
plugins/woocommerce/tests/php/includes/data-stores/class-wc-order-data-store-cpt-test.php (2)
611-638: Basic props read-path coverage looks goodGood round‑trip coverage for currency/totals/taxes/version/prices_include_tax. LGTM.
742-761: Refund read-path test is clear and focusedValidates amount, refunded_by, refunded_payment, and reason correctly. LGTM.
| $refunded_by = $post_meta['_refunded_by'][0] ?? null; | ||
| $reason = $post_meta['_refund_reason'][0] ?? ''; | ||
|
|
||
| $refund->set_props( | ||
| array( | ||
| 'amount' => get_post_meta( $id, '_refund_amount', true ), | ||
| 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? get_post_meta( $id, '_refunded_by', true ) : absint( $post_object->post_author ), | ||
| 'refunded_payment' => wc_string_to_bool( get_post_meta( $id, '_refunded_payment', true ) ), | ||
| 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? get_post_meta( $id, '_refund_reason', true ) : $post_object->post_excerpt, | ||
| 'amount' => $post_meta['_refund_amount'][0] ?? 0, | ||
| 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? $refunded_by : absint( $post_object->post_author ), | ||
| 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ), | ||
| 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? $reason : $post_object->post_excerpt, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid behavior change: fall back to post excerpt when refund_reason meta is empty.
Using metadata_exists() means an empty '_refund_reason' will no longer fall back to $post_object->post_excerpt. Preserve prior behavior by falling back when the meta is missing or empty. Also normalize refunded_by from meta with absint for symmetry with the fallback.
Apply:
- $refunded_by = $post_meta['_refunded_by'][0] ?? null;
- $reason = $post_meta['_refund_reason'][0] ?? '';
+ $refunded_by_raw = $post_meta['_refunded_by'][0] ?? null;
+ $reason_meta = $post_meta['_refund_reason'][0] ?? '';
...
- 'amount' => $post_meta['_refund_amount'][0] ?? 0,
- 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? $refunded_by : absint( $post_object->post_author ),
- 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ),
- 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? $reason : $post_object->post_excerpt,
+ 'amount' => $post_meta['_refund_amount'][0] ?? 0,
+ 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? absint( $refunded_by_raw ) : absint( $post_object->post_author ),
+ 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ),
+ 'reason' => ( '' !== $reason_meta ) ? $reason_meta : $post_object->post_excerpt,Please verify with this quick manual check:
- Create a refund where UI captures a note (post excerpt) but leaves
_refund_reasonempty. - Confirm
$refund->get_reason()returns the excerpt as before.
Let's inspect the relevant code to confirm behavior:
🏁 Script executed:
#!/bin/bash
# Locate and display lines 70-120 of the refund data store CPT file
file=$(fd --ignore-case --max-depth 4 class-wc-order-refund-data-store-cpt.php -p | head -n1)
echo "Inspecting $file"
sed -n '70,120p' "$file"Length of output: 55
Let's locate the file and inspect the relevant lines:
🏁 Script executed:
#!/bin/bash
# Find the refund data store CPT file and display lines 70-120
file=$(find . -type f -iname 'class-wc-order-refund-data-store-cpt.php' | head -n1)
if [ -z "$file" ]; then
echo "File not found"
exit 1
fi
echo "Inspecting $file"
sed -n '70,120p' "$file"Length of output: 1832
Preserve fallback for empty refund_reason and normalize refunded_by
The current check with metadata_exists() will use an empty _refund_reason value instead of falling back to the post excerpt, and the raw _refunded_by value isn’t cast to an integer. Update as follows:
- $refunded_by = $post_meta['_refunded_by'][0] ?? null;
- $reason = $post_meta['_refund_reason'][0] ?? '';
+ $refunded_by_raw = $post_meta['_refunded_by'][0] ?? '';
+ $reason_meta = $post_meta['_refund_reason'][0] ?? '';
$refund->set_props(
array(
- 'amount' => $post_meta['_refund_amount'][0] ?? 0,
- 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? $refunded_by : absint( $post_object->post_author ),
- 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ),
- 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? $reason : $post_object->post_excerpt,
+ 'amount' => $post_meta['_refund_amount'][0] ?? 0,
+ 'refunded_by' => ( '' !== $refunded_by_raw ) ? absint( $refunded_by_raw ) : absint( $post_object->post_author ),
+ 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ),
+ 'reason' => ( '' !== $reason_meta ) ? $reason_meta : $post_object->post_excerpt,
)
);Verify by creating a refund with an empty _refund_reason meta—$refund->get_reason() should return the post excerpt as before.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $refunded_by = $post_meta['_refunded_by'][0] ?? null; | |
| $reason = $post_meta['_refund_reason'][0] ?? ''; | |
| $refund->set_props( | |
| array( | |
| 'amount' => get_post_meta( $id, '_refund_amount', true ), | |
| 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? get_post_meta( $id, '_refunded_by', true ) : absint( $post_object->post_author ), | |
| 'refunded_payment' => wc_string_to_bool( get_post_meta( $id, '_refunded_payment', true ) ), | |
| 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? get_post_meta( $id, '_refund_reason', true ) : $post_object->post_excerpt, | |
| 'amount' => $post_meta['_refund_amount'][0] ?? 0, | |
| 'refunded_by' => metadata_exists( 'post', $id, '_refunded_by' ) ? $refunded_by : absint( $post_object->post_author ), | |
| 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ), | |
| 'reason' => metadata_exists( 'post', $id, '_refund_reason' ) ? $reason : $post_object->post_excerpt, | |
| ) | |
| $refunded_by_raw = $post_meta['_refunded_by'][0] ?? ''; | |
| $reason_meta = $post_meta['_refund_reason'][0] ?? ''; | |
| $refund->set_props( | |
| array( | |
| 'amount' => $post_meta['_refund_amount'][0] ?? 0, | |
| 'refunded_by' => ( '' !== $refunded_by_raw ) | |
| ? absint( $refunded_by_raw ) | |
| : absint( $post_object->post_author ), | |
| 'refunded_payment' => wc_string_to_bool( $post_meta['_refunded_payment'][0] ?? false ), | |
| 'reason' => ( '' !== $reason_meta ) | |
| ? $reason_meta | |
| : $post_object->post_excerpt, | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In
plugins/woocommerce/includes/data-stores/class-wc-order-refund-data-store-cpt.php
around lines 83 to 92, the code currently uses metadata_exists() which will pick
up an empty _refund_reason meta and it doesn't normalize _refunded_by to an
integer; change the logic so 'refunded_by' uses absint($refunded_by) when
present (otherwise fallback to absint($post_object->post_author)), and make
'reason' fall back to $post_object->post_excerpt when the meta is missing or an
empty string (i.e. check that the meta value is set and not an empty string
before using $reason).
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
albarin
changed the title
get_post_meta calls in orders
prettyboymp
left a comment
prettyboymp
left a comment
There was a problem hiding this comment.
Everything looks good. I double checked to make sure non of the prop data was stored as serialized data and didn't see any.
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR reduces the number of calls to
get_post_meta. It's a follow-up of #60614 (#37326)Instead of calling
get_post_metafor each field, it gets all the post meta in one call.trunk~3000~200~1300~80Closes #60706
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
/wp-admin/edit.php?post_type=shop_order).Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment