Fix - Sync product GTIN when available #2810
Conversation
includes/fbproduct.php
Outdated
| @@ -720,6 +720,11 @@ public function prepare_product( $retailer_id = null, $type_to_prepare_for = sel | |||
| $product_data['quantity_to_sell_on_facebook'] = (int) max( 0, $this->woo_product->get_stock_quantity() ); | |||
| } | |||
|
|
|||
| // Add GTIN (Global Trade Item Number) | |||
| if ( $gtin = $this->woo_product->get_global_unique_id() ) { | |||
There was a problem hiding this comment.
2 Issues I see here:
-
get_global_unique_id has only been available since WooCommerce version 9.2, we should check this before calling this method (Add unique_id field to product woocommerce/woocommerce#47364).
-
get_global_unique_id() could return values that are not GTIN such ISBN, EAN etc... If Meta is expecting a GTIN specifically, I think we should validate that the value returned is a valid GTIN. Since version 9.3.3, We added prepare_gtin to WC_Structured_Data for this (Validate and prepare GTIN in structured data woocommerce/woocommerce#50926)
There was a problem hiding this comment.
@rawdreeg thank you very much for reviewing! And sorry for late follow up.
1/ Make sense, will address this.
2/ Re "could return values that are not GTIN such ISBN, EAN etc...", on Meta side the GTIN is a super set of all types of identifiers, including:
UPC (North America / GTIN-12): 12-digit number.
EAN (Europe / GTIN-13): 13-digit number.
JAN (Japan / GTIN-13): 8 or 13-digit number.
ISBN (for books / ISBN-13): 13-digit number. Convert any 10-digit ISBN-10 numbers to ISBN-13.
ITF-14 (for multipacks / GTIN-14): 14-digit number.
Here is the API dev doc: https://developers.facebook.com/docs/commerce-platform/catalog/fields/
Plus, we have own validators of the GTIN on our side, so if it does it conform to any of the acceptable formats we would just not persist it.
So, whatever user input into Woo GTIN UI field, we can just pass to the Meta as raw value in my opinion.
There was a problem hiding this comment.
Plus, we have own validators of the GTIN on our side, so if it does it conform to any of the acceptable formats we would just not persist it.
If you have your validation, then it should be fine. Thanks for your response.
There was a problem hiding this comment.
@rawdreeg added plugin version check before calling the function.
Changes proposed in this Pull Request:
This change suggests to sync to Meta a product GTIN when it is available.
Screenshots:
N/A (no changes to UI/UX)
Detailed test instructions:
Run new tests
./vendor/bin/phpunit --filter test_gtin_for_simple_product_set
./vendor/bin/phpunit --filter test_gtin_for_simple_product_unset
./vendor/bin/phpunit --filter test_gtin_for_variable_product_set
./vendor/bin/phpunit --filter test_gtin_for_variable_product_unset
Run all tests
npm run test:php
Manual testing. I tested this in the locally hosted WP WooCommerce website and checked logs on Meta side to verify the request is being sent with the "gtin" populated correctly. Tested on both simple and variable product.
Changelog entry