Fix: Attribute terms REST API writes menu_order to wrong meta key#63390
Conversation
Testing GuidelinesHi , 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:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR updates the WooCommerce REST API controller for product attribute terms to read and write Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/woocommerce/includes/rest-api/Controllers/Version1/class-wc-rest-product-attribute-terms-v1-controller.php`:
- Around line 176-182: The update_term_meta_fields function currently calls
update_term_meta( $id, 'order', $request['menu_order'] ) without checking if
$request['menu_order'] is present, which allows null to overwrite existing meta;
modify update_term_meta_fields (referencing update_term_meta_fields,
$term->term_id, and $request['menu_order']) to first check that menu_order
exists and is not null (e.g. isset($request['menu_order']) or array_key_exists
and !== null) and only call update_term_meta when that guard passes, leaving
existing term meta unchanged for partial/PATCH requests.
- Line 138: prepare_item_for_response now reads term meta 'order' while
update_term_meta_fields writes 'order', but existing installations still have
ordering stored under legacy keys like 'order_{taxonomy}' (e.g. order_pa_size),
causing silent data loss on upgrade; add a one-time migration (run during a WC
upgrade routine or activation hook) that scans product attribute taxonomies and
copies meta from each legacy key ('order_' . $taxonomy) into the unified 'order'
meta for the same term_id when 'order' does not already exist, then remove or
ignore the old keys; implement this migration near the plugin/theme upgrade
entrypoint and reference prepare_item_for_response and update_term_meta_fields
to ensure reads/writes use the new 'order' meta consistently.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
(1)
1-1: Add strict types declaration to align with WooCommerce test conventions.Please add
declare( strict_types = 1 );after the opening PHP tag.Proposed patch
<?php +declare( strict_types = 1 ); /** * Tests for WC_REST_Product_Attribute_Terms_Controller menu_order meta key. */Based on learnings: Applies to plugins/woocommerce/tests/**/*.php : Add
declare( strict_types = 1 );at the top of WooCommerce PHPUnit test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @ at line 1, Add a strict types declaration immediately after the opening PHP tag: where you find files starting with "<?php" in the WooCommerce PHPUnit test files, insert the line declare( strict_types = 1 ); on the next line so the file begins with "<?php" followed by "declare( strict_types = 1 );" to align with test conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @ :
- Line 1: Add a strict types declaration immediately after the opening PHP tag:
where you find files starting with "<?php" in the WooCommerce PHPUnit test
files, insert the line declare( strict_types = 1 ); on the next line so the file
begins with "<?php" followed by "declare( strict_types = 1 );" to align with
test conventions.
- Remove garbled test file (space character filename) and add tests to the existing V1 controller test class at the correct path. - Add isset($request['menu_order']) guard to prevent overwriting order meta when updating unrelated term fields (matches categories controller). - Add tests for create, update, read, old-key regression, and the isset guard preserving existing order. - Add missing changelog entry.
|
Thanks for the contribution — this is a great catch! The meta key mismatch has been there since 3.6 and your fix is spot on. I pushed a couple of small tweaks: added an |
- Move file docblock before declare(strict_types) per WooCommerce standards - Add short descriptions to all test method docblocks - Fix equals sign alignment
Changes Proposed
Fixes #28158
The attribute terms REST API controller saves
menu_orderto meta keyorder_{taxonomy}(e.g.order_pa_size), but everywhere else in WooCommerce the meta key is justorder:wc_set_term_order()→orderwc_change_get_terms_defaults()→orderwc_change_pre_get_terms()→orderorderorderSo
menu_orderset via the API is silently ignored by the admin and frontend because they look up a different key.The fix aligns
class-wc-rest-product-attribute-terms-v1-controller.phpwith the categories controller by usingorderin bothupdate_term_meta_fields()andprepare_item_for_response().Testing Instructions
order_byset to "Custom ordering"menu_ordervalues:PUT /wc/v3/products/attributes/{id}/terms/{term_id}with{ "menu_order": 0 },{ "menu_order": 1 }, etc.Before: Terms appear in alphabetical order regardless of
menu_ordervalues.After: Terms appear in the order set via the API.
Changelog Entry
Significance: Patch
Type: Fix
Message: Fix attribute terms REST API writing menu_order to wrong meta key, preventing custom ordering from working via the API.