Add MPN (manufacturer part / product number) to core #60286
Conversation
📝 WalkthroughWalkthroughThis change adds support for a new product property, MPN (Manufacturer Part Number), across WooCommerce core. It introduces the MPN field to product data structures, admin UI, REST APIs (products and variations), database schema (lookup table), and update routines, enabling storage, retrieval, editing, and querying of MPN values for products and variations. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant UI as WooCommerce Admin UI
participant WC_Product as WC_Product
participant DataStore as Product Data Store
participant DB as Database
participant REST as REST API Client
Admin->>UI: Edit product/variation (enter MPN)
UI->>WC_Product: set_mpn($mpn)
WC_Product->>DataStore: Save product
DataStore->>DB: Store _mpn post meta and update lookup table
REST->>REST: Submit product/variation with MPN via API
REST->>WC_Product: set_mpn($mpn)
WC_Product->>DataStore: Save product
DataStore->>DB: Store _mpn post meta and update lookup table
WC_Product->>DataStore: Read product
DataStore->>DB: Fetch _mpn meta
DataStore->>WC_Product: set_mpn($mpn)
WC_Product->>REST: Return MPN in API response
WC_Product->>UI: Display MPN in admin UI
note over DB: Lookup table now includes 'mpn' column
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
plugins/woocommerce/includes/class-wc-install.php (1)
1792-1816: Schema: add an index for ‘mpn’ in wc_product_meta_lookupIf you plan to query/filter by MPN (REST, admin lists, etc.), adding an index mirrors
skuand avoids full scans at scale.Proposed schema change (add alongside the existing KEYs):
KEY `onsale` (`onsale`), KEY min_max_price (`min_price`, `max_price`), + KEY mpn (mpn(50)), KEY sku (sku(50))dbDelta will add the index on update since it’s in get_schema(). If you prefer a dedicated migration, add an ALTER in the 10.2.0 update callback.
plugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.php (1)
574-581: Bug: Wrong function name ‘wc_format_Decimal’ (capital D) will fatally errorWooCommerce helper is wc_format_decimal (lowercase d). This line will fatal on save.
- $variation->set_cogs_value( is_null( $cogs_value ) ? null : (float) wc_format_Decimal( $cogs_value ) ); + $variation->set_cogs_value( is_null( $cogs_value ) ? null : (float) wc_format_decimal( $cogs_value ) );plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (1)
462-467: Include search_mpn in post_type widening to catch variation MPNsParity with search_sku ensures partial MPN matches on variations are included.
- if ( ! empty( $request['sku'] ) || ! empty( $request['search_sku'] ) || ! empty( $request['mpn'] ) || $this->search_name_or_sku_tokens || $this->search_fields_tokens ) { + if ( ! empty( $request['sku'] ) || ! empty( $request['search_sku'] ) || ! empty( $request['mpn'] ) || ! empty( $request['search_mpn'] ) || $this->search_mpn_arg_value || $this->search_name_or_sku_tokens || $this->search_fields_tokens ) {
♻️ Duplicate comments (1)
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (1)
584-589: WHERE clause for partial MPN — contingent on JOIN fixThis is correct but depends on joining wc_product_meta_lookup when search_mpn is used (see earlier comment).
🧹 Nitpick comments (14)
plugins/woocommerce/includes/class-wc-structured-data.php (1)
232-236: MPN added to JSON-LD product markup — good; minor normalization suggestedLooks correct and aligns with Schema.org. Consider trimming to avoid accidental leading/trailing whitespace.
- $mpn = $product->get_mpn(); + $mpn = trim( (string) $product->get_mpn() );plugins/woocommerce/includes/admin/meta-boxes/views/html-product-data-inventory.php (1)
44-55: Use “Manufacturer Part Number” (MPN) for label copy consistencyIndustry and Schema.org use “Manufacturer Part Number”. Adjust label/title and description to match.
- 'label' => '<abbr title="' . esc_attr__( 'Manufacturer Product Number', 'woocommerce' ) . '">' . esc_html__( 'MPN', 'woocommerce' ) . '</abbr>', - 'description' => __( 'Enter the manufacturer product number for this product.', 'woocommerce' ), + 'label' => '<abbr title="' . esc_attr__( 'Manufacturer Part Number', 'woocommerce' ) . '">' . esc_html__( 'MPN', 'woocommerce' ) . '</abbr>', + 'description' => __( 'Enter the manufacturer part number for this product.', 'woocommerce' ),plugins/woocommerce/includes/admin/meta-boxes/views/html-variation-admin.php (1)
109-120: Variation MPN UI looks good; align label copy with “Manufacturer Part Number”Mirror the inventory tab’s wording for consistency.
- 'label' => '<abbr title="' . esc_attr__( 'Manufacturer Product Number', 'woocommerce' ) . '">' . esc_html__( 'MPN', 'woocommerce' ) . '</abbr>', - 'description' => __( 'Enter the manufacturer product number for this variation.', 'woocommerce' ), + 'label' => '<abbr title="' . esc_attr__( 'Manufacturer Part Number', 'woocommerce' ) . '">' . esc_html__( 'MPN', 'woocommerce' ) . '</abbr>', + 'description' => __( 'Enter the manufacturer part number for this variation.', 'woocommerce' ),plugins/woocommerce/includes/wc-product-functions.php (1)
1709-1716: Populate ‘mpn’ from post meta — LGTM; optional ergonomic helperThe switch handling reuses the existing meta mapping pattern. Consider adding a convenience helper similar to SKU/Global Unique ID:
Would you like a
wc_get_product_id_by_mpn( $mpn )helper (and corresponding data store method) for parity withwc_get_product_id_by_sku/wc_get_product_id_by_global_unique_id?plugins/woocommerce/includes/wc-update-functions.php (1)
3099-3106: Large-table UPDATE: acceptable once-off, but consider indexing and batching if queries will filter by MPNThe once-off UPDATE … LEFT JOIN is fine, but if MPN will be used for filtering, consider adding an index on wc_product_meta_lookup.mpn in the base schema (WC_Install::get_schema) to avoid future full scans. If stores are extremely large, a batched backfill (by product_id ranges) reduces lock time.
plugins/woocommerce/includes/abstracts/abstract-wc-product.php (1)
897-905: Setter is fine; consider documenting length expectationsSetter mirrors other text props and relies on upstream sanitization (admin UI/REST). Since the lookup column is varchar(100), consider documenting that values longer than 100 chars will be trimmed in the lookup table (or add trimming at lookup-update time to avoid MySQL truncation).
plugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.php (1)
375-378: UI improvement follow-up: cap MPN input length at 100 in the form fieldsThe lookup table column is varchar(100). To prevent user confusion, add maxlength="100" to the MPN inputs in:
- views/html-product-data-inventory.php
- views/html-variation-admin.php
plugins/woocommerce/includes/rest-api/Controllers/Version2/class-wc-rest-products-v2-controller.php (2)
1803-1808: Schema entry for MPN added — consider clarifying constraintsOptionally note in description (or via maxLength) that MPN may be stored up to 100 chars in the lookup table to set expectations for clients.
- 'mpn' => array( - 'description' => __( 'Manufacturer product number.', 'woocommerce' ), - 'type' => 'string', - 'context' => array( 'view', 'edit' ), - ), + 'mpn' => array( + 'description' => __( 'Manufacturer product number (up to 100 characters persisted in lookup).', 'woocommerce' ), + 'type' => 'string', + 'context' => array( 'view', 'edit' ), + ),
2332-2454: Optional: add collection filtering by MPN (parity with other identifiers)If you plan to allow filtering by MPN in V2 as well (V3 may already have it), consider adding an mpn param here and honoring it in prepare_objects_query via meta_query or lookup table.
plugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.php (2)
1875-2013: Optional: include MPN in product searchsearch_products currently searches title/excerpt/content and SKU (and parent SKU for variations). If you expect users to search by MPN, extend the LIKE predicates to check wc_product_meta_lookup.mpn (and parent for variations).
2378-2412: Performance: consider an index on wc_product_meta_lookup.mpn if used in queriesIf REST/Product queries will filter by MPN, adding an index on the new column in WC_Install::get_schema will avoid table scans.
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.php (2)
580-584: Schema addition for MPN — minor copy tweakConsider aligning the description text with products controller for consistency (either “Manufacturer Product Number.” or “Manufacturer product number.” everywhere).
1244-1250: Nit: wording says “products” in variations controllerChange description to “Limit result set to product variations with specified MPN(s).”
- 'description' => __( 'Limit result set to products with specified MPN(s).', 'woocommerce' ), + 'description' => __( 'Limit result set to product variations with specified MPN(s).', 'woocommerce' ),plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (1)
1200-1304: Schema: add mpn — LGTM (minor consistency nit)Consider matching capitalization with the variations controller.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
plugins/woocommerce/includes/abstracts/abstract-wc-product.php(3 hunks)plugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.php(2 hunks)plugins/woocommerce/includes/admin/meta-boxes/views/html-product-data-inventory.php(1 hunks)plugins/woocommerce/includes/admin/meta-boxes/views/html-variation-admin.php(1 hunks)plugins/woocommerce/includes/class-wc-install.php(2 hunks)plugins/woocommerce/includes/class-wc-structured-data.php(1 hunks)plugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.php(5 hunks)plugins/woocommerce/includes/rest-api/Controllers/Version2/class-wc-rest-products-v2-controller.php(3 hunks)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.php(5 hunks)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php(12 hunks)plugins/woocommerce/includes/wc-product-functions.php(2 hunks)plugins/woocommerce/includes/wc-update-functions.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/admin/meta-boxes/views/html-product-data-inventory.phpplugins/woocommerce/includes/admin/meta-boxes/views/html-variation-admin.phpplugins/woocommerce/includes/rest-api/Controllers/Version2/class-wc-rest-products-v2-controller.phpplugins/woocommerce/includes/wc-product-functions.phpplugins/woocommerce/includes/wc-update-functions.phpplugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.phpplugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.phpplugins/woocommerce/includes/abstracts/abstract-wc-product.phpplugins/woocommerce/includes/class-wc-structured-data.phpplugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.phpplugins/woocommerce/includes/class-wc-install.phpplugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.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/admin/meta-boxes/views/html-product-data-inventory.phpplugins/woocommerce/includes/admin/meta-boxes/views/html-variation-admin.phpplugins/woocommerce/includes/rest-api/Controllers/Version2/class-wc-rest-products-v2-controller.phpplugins/woocommerce/includes/wc-product-functions.phpplugins/woocommerce/includes/wc-update-functions.phpplugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.phpplugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.phpplugins/woocommerce/includes/abstracts/abstract-wc-product.phpplugins/woocommerce/includes/class-wc-structured-data.phpplugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.phpplugins/woocommerce/includes/class-wc-install.phpplugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php
🔇 Additional comments (24)
plugins/woocommerce/includes/wc-product-functions.php (1)
1607-1621: Including ‘mpn’ in lookup table update columns — LGTMThis ensures async population via the existing column update workflow. No issues found.
plugins/woocommerce/includes/class-wc-install.php (1)
297-299: DB update callback registration — verify idempotent upgrader existsEnsure
wc_update_1020_add_mpn_to_product_lookup_table()exists in wc-update-functions.php, is idempotent (safe on re-run), and schedules backfill of existing meta into the new column.plugins/woocommerce/includes/wc-update-functions.php (1)
3078-3107: Updater registration and base schema inclusion confirmed
- Updater callback
wc_update_1020_add_mpn_to_product_lookup_tableis registered for 10.2.0 (class-wc-install.php:298)- Base schema’s
wc_product_meta_lookuptable includes thempncolumn (class-wc-install.php:1795)No further changes required.
plugins/woocommerce/includes/abstracts/abstract-wc-product.php (2)
75-75: Adding 'mpn' to product data looks goodThe property is placed alongside other identifiers and defaults to empty string. No issues.
290-292: Getter implementation is consistent with the rest of the modelUsing get_prop keeps context semantics intact.
plugins/woocommerce/includes/admin/meta-boxes/class-wc-meta-box-product-data.php (2)
375-378: Persisting product MPN is correct and sanitizedMPN is read from POST and sanitized via wc_clean before set_props. Good.
558-566: Persisting variation MPN is correct and sanitizedVariation path mirrors the simple product path. Looks good.
plugins/woocommerce/includes/rest-api/Controllers/Version2/class-wc-rest-products-v2-controller.php (2)
730-733: Expose MPN in responses — looks goodUsing get_mpn($context) is consistent with other fields.
1031-1034: Accept MPN in requests — good; sanitizedSetting via wc_clean() matches conventions.
plugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.php (4)
38-39: Including '_mpn' in internal meta keys is correctThis aligns MPN with other first-class props.
446-449: Mapping _mpn ↔ mpn during read is correctKeeps model hydration consistent.
667-668: Mapping _mpn during write is correctEnsures the meta layer stays in sync.
888-894: Triggering lookup table update when MPN changes — goodIncludes ‘mpn’ in the set of props that update the lookup table. This is needed for search/sort/reporting parity.
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-product-variations-controller.php (3)
124-124: Expose MPN in variation responses — LGTMConsistent with existing fields; no concerns.
220-224: Set MPN on variation — LGTMInput is sanitized via wc_clean; matches established pattern.
1017-1029: Filter variations by MPN — LGTMMeta query matches existing patterns (sku/global_unique_id). Works with comma-separated values.
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php (8)
48-57: Introduce search_mpn arg cache — LGTMScoped, documented, and reset later; no issues.
411-422: Exact-match MPN filter — LGTMUses sanitized input and IN compare; consistent with global_unique_id handling.
500-500: Search criteria gating now includes search_mpn — LGTM
521-521: Reset search_mpn state — LGTM
856-859: Set MPN on product — LGTMSanitization via wc_clean; matches SKU/global_unique_id handling.
1924-1930: Collection param: search_mpn — LGTMGood precedence over mpn and sanitization.
To prevent regressions, consider adding API tests for:
- GET /wc/v3/products?search_mpn=partial to ensure lookup JOIN works (product and variation cases).
- GET /wc/v3/products?mpn=A,B exact matching and comma handling.
2013-2019: Collection param: mpn (exact) — LGTMAligned with existing sku/global_unique_id patterns.
2121-2123: Expose MPN in product responses when requested — LGTM
| if ( get_option( 'woocommerce_schema_version', 0 ) >= 1020 ) { | ||
| $product_data['mpn'] = get_post_meta( $id, '_mpn', true ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Trim MPN to 100 chars when populating lookup to avoid silent DB truncation
The lookup column is varchar(100). If meta _mpn exceeds this, MySQL will truncate. Trim proactively here to keep app-side behavior explicit and predictable.
- $product_data['mpn'] = get_post_meta( $id, '_mpn', true );
+ $raw_mpn = (string) get_post_meta( $id, '_mpn', true );
+ // Ensure we don't exceed lookup column size.
+ $product_data['mpn'] = function_exists('mb_substr') ? mb_substr( $raw_mpn, 0, 100 ) : substr( $raw_mpn, 0, 100 );📝 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.
| if ( get_option( 'woocommerce_schema_version', 0 ) >= 1020 ) { | |
| $product_data['mpn'] = get_post_meta( $id, '_mpn', true ); | |
| } | |
| if ( get_option( 'woocommerce_schema_version', 0 ) >= 1020 ) { | |
| - $product_data['mpn'] = get_post_meta( $id, '_mpn', true ); | |
| + $raw_mpn = (string) get_post_meta( $id, '_mpn', true ); | |
| + // Ensure we don't exceed lookup column size. | |
| + $product_data['mpn'] = function_exists( 'mb_substr' ) | |
| + ? mb_substr( $raw_mpn, 0, 100 ) | |
| + : substr( $raw_mpn, 0, 100 ); | |
| } |
🤖 Prompt for AI Agents
In plugins/woocommerce/includes/data-stores/class-wc-product-data-store-cpt.php
around lines 2408 to 2410, the MPN value fetched from post meta can exceed the
100 character limit of the database lookup column, causing silent truncation. To
fix this, trim the MPN string to a maximum of 100 characters before assigning it
to the product_data array. This ensures explicit and predictable behavior on the
application side.
| // Do a partial match for MPN. Supersedes the 'mpn' argument, that does exact matching. | ||
| if ( ! empty( $request['search_mpn'] ) ) { | ||
| // Store this for use in the query clause filters. | ||
| $this->search_mpn_arg_value = $request['search_mpn']; | ||
|
|
||
| unset( $request['mpn'] ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: search_mpn WHERE references lookup table without guaranteeing the JOIN
When using search_mpn, add_search_criteria_to_wp_query_where adds a wc_product_meta_lookup.mpn LIKE clause, but add_search_criteria_to_wp_query_join does not consider search_mpn for joining the lookup table. This can produce invalid SQL and/or miss results.
Apply these changes:
- Ensure the join runs for search_mpn:
public function add_search_criteria_to_wp_query_join( $join ) {
// Check if already joined to avoid duplicate joins.
if ( strstr( $join, 'wc_product_meta_lookup' ) ) {
return $join;
}
// Only join if we need meta table search.
- if ( ! $this->search_fields_tokens &&
- ! $this->search_sku_arg_value &&
- ! ( $this->search_name_or_sku_tokens && wc_product_sku_enabled() ) ) {
+ if ( ! $this->search_fields_tokens &&
+ ! $this->search_sku_arg_value &&
+ ! $this->search_mpn_arg_value &&
+ ! ( $this->search_name_or_sku_tokens && wc_product_sku_enabled() ) ) {
return $join;
}- Also include search_mpn in the post_type widening so variation-level MPNs are discoverable, mirroring search_sku:
- if ( ! empty( $request['sku'] ) || ! empty( $request['search_sku'] ) || ! empty( $request['mpn'] ) || $this->search_name_or_sku_tokens || $this->search_fields_tokens ) {
+ if ( ! empty( $request['sku'] ) || ! empty( $request['search_sku'] ) || ! empty( $request['mpn'] ) || ! empty( $request['search_mpn'] ) || $this->search_mpn_arg_value || $this->search_name_or_sku_tokens || $this->search_fields_tokens ) {
$args['post_type'] = array( 'product', 'product_variation' );
}Optional: update the join method’s docblock to reflect it now covers SKU and MPN.
📝 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.
| // Do a partial match for MPN. Supersedes the 'mpn' argument, that does exact matching. | |
| if ( ! empty( $request['search_mpn'] ) ) { | |
| // Store this for use in the query clause filters. | |
| $this->search_mpn_arg_value = $request['search_mpn']; | |
| unset( $request['mpn'] ); | |
| } | |
| // In plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php | |
| public function add_search_criteria_to_wp_query_join( $join ) { | |
| // Check if already joined to avoid duplicate joins. | |
| if ( strstr( $join, 'wc_product_meta_lookup' ) ) { | |
| return $join; | |
| } | |
| // Only join if we need meta table search. | |
| if ( ! $this->search_fields_tokens && | |
| ! $this->search_sku_arg_value && | |
| ! $this->search_mpn_arg_value && // <-- added | |
| ! ( $this->search_name_or_sku_tokens && wc_product_sku_enabled() ) ) { | |
| return $join; | |
| } | |
| // existing join logic continues… | |
| } | |
| # …later in the same file, where post_type is widened: | |
| // Before: | |
| if ( ! empty( $request['sku'] ) || | |
| ! empty( $request['search_sku'] ) || | |
| ! empty( $request['mpn'] ) || | |
| $this->search_name_or_sku_tokens || | |
| $this->search_fields_tokens ) { | |
| $args['post_type'] = [ 'product', 'product_variation' ]; | |
| } | |
| // After: | |
| if ( ! empty( $request['sku'] ) || | |
| ! empty( $request['search_sku'] ) || | |
| ! empty( $request['mpn'] ) || | |
| ! empty( $request['search_mpn'] ) || // <-- added | |
| $this->search_mpn_arg_value || // <-- added | |
| $this->search_name_or_sku_tokens || | |
| $this->search_fields_tokens ) { | |
| $args['post_type'] = [ 'product', 'product_variation' ]; | |
| } |
🤖 Prompt for AI Agents
In
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-products-controller.php
around lines 392 to 399, the search_mpn filter adds a WHERE clause referencing
the wc_product_meta_lookup table without ensuring the necessary JOIN is added,
causing potential SQL errors or missed results. Update the
add_search_criteria_to_wp_query_join method to include a JOIN for search_mpn
similar to search_sku, and modify the post_type widening logic to include
search_mpn so variation-level MPNs are searchable. Optionally, update the join
method's docblock to mention it now handles both SKU and MPN joins.
| // Check if the column already exists. | ||
| $column_exists = $wpdb->get_var( | ||
| $wpdb->prepare( | ||
| "SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = %s AND TABLE_NAME = %s AND COLUMN_NAME = 'mpn'", | ||
| $wpdb->dbname, | ||
| $table_name | ||
| ) | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer SHOW COLUMNS over INFORMATION_SCHEMA for portability and consistency
Using INFORMATION_SCHEMA can fail on some managed hosts with restricted permissions and is inconsistent with other checks in this file that use SHOW COLUMNS. Recommend switching to SHOW COLUMNS LIKE to detect the column.
- $column_exists = $wpdb->get_var(
- $wpdb->prepare(
- "SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = %s AND TABLE_NAME = %s AND COLUMN_NAME = 'mpn'",
- $wpdb->dbname,
- $table_name
- )
- );
+ $column_exists = $wpdb->get_var(
+ $wpdb->prepare(
+ "SHOW COLUMNS FROM `{$table_name}` LIKE %s",
+ 'mpn'
+ )
+ );🤖 Prompt for AI Agents
In plugins/woocommerce/includes/wc-update-functions.php around lines 3083 to
3091, replace the query using INFORMATION_SCHEMA.COLUMNS with a SHOW COLUMNS
LIKE query to check for the 'mpn' column. This change improves compatibility on
managed hosts with restricted permissions and aligns with other column existence
checks in the file. Use $wpdb->get_var with a prepared SHOW COLUMNS LIKE
statement targeting the table and column name.
| $wpdb->query( | ||
| "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NULL default '' AFTER `global_unique_id`" | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align column nullability with existing lookup string columns
Current DDL defines varchar(100) NULL default ''. To avoid three-states and to match typical lookup table semantics (e.g., sku), use NOT NULL DEFAULT ''. This also simplifies queries.
- "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NULL default '' AFTER `global_unique_id`"
+ "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NOT NULL DEFAULT '' AFTER `global_unique_id`"📝 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.
| $wpdb->query( | |
| "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NULL default '' AFTER `global_unique_id`" | |
| ); | |
| $wpdb->query( | |
| - "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NULL default '' AFTER `global_unique_id`" | |
| + "ALTER TABLE {$table_name} ADD COLUMN `mpn` varchar(100) NOT NULL DEFAULT '' AFTER `global_unique_id`" | |
| ); |
🤖 Prompt for AI Agents
In plugins/woocommerce/includes/wc-update-functions.php around lines 3094 to
3096, the SQL statement adds the 'mpn' column as varchar(100) NULL with a
default empty string, which creates a three-state scenario. Modify the column
definition to be NOT NULL with DEFAULT '' to align with existing lookup string
columns and simplify query logic.
|
Thanks for working on this, Brian! |
Submission Review Guidelines:
Changes proposed in this Pull Request:
Add MPN (Manufacturer Product Number to Core. This is part of a bigger move to support the full product schema-
This PR adds the following changes:
Closes #52332 .
(For Bug Fixes) Bug introduced in PR # .
Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment